From 9b91db5007a7720e5f461f6f868b75d71c353642 Mon Sep 17 00:00:00 2001 From: Vladimir Chirikov Date: Fri, 28 Oct 2022 16:15:02 +0300 Subject: [PATCH] fix: multiple stack trace annotations --- src/dotnet/Logger/Logger.cs | 47 +++++++-------- src/dotnet/Logger/StackTraceParser.cs | 59 +++++++++++++++++++ src/dotnet/Logger/TestRunSummaryGenerator.cs | 32 +++------- .../Logger.UnitTests/StackTraceParserTests.cs | 29 +++++++++ 4 files changed, 119 insertions(+), 48 deletions(-) create mode 100644 src/dotnet/Logger/StackTraceParser.cs create mode 100644 tests/Logger.UnitTests/StackTraceParserTests.cs diff --git a/src/dotnet/Logger/Logger.cs b/src/dotnet/Logger/Logger.cs index ebdf086..e31b224 100644 --- a/src/dotnet/Logger/Logger.cs +++ b/src/dotnet/Logger/Logger.cs @@ -18,6 +18,7 @@ public class GitHubLogger : ITestLoggerWithParameters private LoggerParameters _params = null!; private GitHubApi _gh = null!; + private StackTraceParser _stackTraceParser = null!; private TestRunSummaryGenerator _summaryGenerator = null!; private TestRunSummaryWriter _summaryWriter = null!; private string? _testRunName; @@ -51,13 +52,19 @@ public void Initialize(TestLoggerEvents events, Dictionary param _gh.Output.Warning("GITHUB_TOKEN is an unsupported token (length != 40). Logger won't do anything."); return; } + var workspace = !string.IsNullOrWhiteSpace(_params.GITHUB_WORKSPACE) + ? _params.GITHUB_WORKSPACE + : Environment.CurrentDirectory; + + _stackTraceParser = new StackTraceParser(workspace); _summaryGenerator = new TestRunSummaryGenerator( + _stackTraceParser, _params.GITHUB_SERVER_URL, _params.GITHUB_REPOSITORY, - _params.GITHUB_WORKSPACE, _params.GITHUB_SHA ); + var summaryFile = !string.IsNullOrWhiteSpace(_params.GITHUB_STEP_SUMMARY) ? _params.GITHUB_STEP_SUMMARY : "test-run.md"; @@ -158,45 +165,35 @@ async Task OnTestResultInternalAsync(TestResult result) if (_annotationWriter == null) throw new($"annotationWriter must not be null, test run status: {_status}"); - var stackTraces = StackTraceParser.Parse(result.ErrorStackTrace, (f, t, m, pl, ps, fn, ln) => new - { - Frame = f, - Type = t, - Method = m, - ParameterList = pl, - Parameters = ps, - File = fn, - Line = ln, - }); - var sb = new StringBuilder(1024); - if (!await _locker.WaitAsync(_timeout).ConfigureAwait(false)) - throw new TimeoutException($"{nameof(OnTestResult)}: Waiting for the lock is too long"); - try + var stackTraces = _stackTraceParser.ParseAndNormalize(result.ErrorStackTrace); + var stackTrace = stackTraces.FirstOrDefault(x => !string.IsNullOrWhiteSpace(x.File) && File.Exists(x.File)) + ?? stackTraces.FirstOrDefault(); + + if (stackTrace != null) { - foreach (var st in stackTraces) + if (!await _locker.WaitAsync(_timeout).ConfigureAwait(false)) + throw new TimeoutException($"{nameof(OnTestResult)}: Waiting for the lock is too long"); + try { - if (!int.TryParse(st.Line, NumberStyles.Integer, CultureInfo.InvariantCulture, out int line)) + if (!int.TryParse(stackTrace.Line, NumberStyles.Integer, CultureInfo.InvariantCulture, out int line)) _gh.Output.Warning("Can't parse line in stacktrace"); await _annotationWriter.ErrorAsync( message: GetDetailsMessage(result, sb), title: $"{result.TestCase.DisplayName}", - file: WorkspaceRelativePath(st.File), + file: stackTrace.File, line: line > 5 ? line - 5 : line, endLine: line + 1 ).ConfigureAwait(false); } - } - finally - { - _locker.Release(); + finally + { + _locker.Release(); + } } } - string WorkspaceRelativePath(string fullPath) => string.IsNullOrEmpty(_params.GITHUB_WORKSPACE) - ? fullPath - : fullPath.Replace(_params.GITHUB_WORKSPACE, "", StringComparison.Ordinal).TrimStart('\\', '/'); static string GetDetailsMessage(TestResult result, StringBuilder sb) { diff --git a/src/dotnet/Logger/StackTraceParser.cs b/src/dotnet/Logger/StackTraceParser.cs new file mode 100644 index 0000000..b0d7ca3 --- /dev/null +++ b/src/dotnet/Logger/StackTraceParser.cs @@ -0,0 +1,59 @@ + +namespace GitHub.VsTest.Logger; + +internal record class StackTraceInfo +{ + public string Frame { get; set; } = ""; + public string Type { get; set; } = ""; + public string Method { get; set; } = ""; + public string ParameterList { get; set; } = ""; + public IEnumerable> Parameters { get; set; } = new Dictionary(StringComparer.Ordinal); + public string File { get; set; } = ""; + public string Line { get; set; } = ""; +} + +internal partial class StackTraceParser +{ + private readonly string _workspacePath; + + /// ctor + /// Full path to the current dir / workspace folder + public StackTraceParser(string workspacePath) => _workspacePath = workspacePath; + + public static IEnumerable Parse(string stackTrace) + => Parse(stackTrace, (f, t, m, pl, ps, fn, ln) => new StackTraceInfo { + Frame = f, + Type = t, + Method = m, + ParameterList = pl, + Parameters = ps, + File = fn, + Line = ln, + }); + + private static readonly StringComparison _pathStringComparison = + RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + + + /// Returns first parsed stack traces with relative file path + public IEnumerable ParseAndNormalize(string stackTrace) + { + var directorySeparators = new char[]{'/', '\\'}; + foreach (var st in Parse(stackTrace)) + { + // won't work on old msbuilds (requires netstandard2.1, but we are already there, so) + if (!string.IsNullOrWhiteSpace(st.File)) + { + // file in stacktraces must use full paths, so we can check it before make them relative + // to exclude files out from our workspace + if (st.File.StartsWith(_workspacePath, _pathStringComparison)) + st.File = Path.GetRelativePath(_workspacePath, st.File).Trim(directorySeparators); + // normalize paths to use '/' as directory separators + st.File = st.File.Replace('\\', '/'); + } + yield return st; + } + } +} diff --git a/src/dotnet/Logger/TestRunSummaryGenerator.cs b/src/dotnet/Logger/TestRunSummaryGenerator.cs index 9e1d77e..db243d2 100644 --- a/src/dotnet/Logger/TestRunSummaryGenerator.cs +++ b/src/dotnet/Logger/TestRunSummaryGenerator.cs @@ -6,14 +6,14 @@ internal sealed class TestRunSummaryGenerator { private readonly string _serverUrl; private readonly string _repository; - private readonly string _workspace; private readonly string _sha; + private readonly StackTraceParser _stackTraceParser; - public TestRunSummaryGenerator(string serverUrl, string repository, string workspace, string sha) + public TestRunSummaryGenerator(StackTraceParser stackTraceParser, string serverUrl, string repository, string sha) { + _stackTraceParser = stackTraceParser; _serverUrl = serverUrl; _repository = repository; - _workspace = workspace.Replace('\\', '/').TrimEnd('/'); _sha = sha; } @@ -86,18 +86,11 @@ public string Generate( foreach (var testResult in testResults.Where(r => r.Outcome == TestOutcome.Failed)) { - var stackTraces = StackTraceParser.Parse(testResult.ErrorStackTrace, (f, t, m, pl, ps, fn, ln) => new - { - Frame = f, - Type = t, - Method = m, - ParameterList = pl, - Parameters = ps, - File = fn, - Line = ln, - }); - - foreach (var stackTrace in stackTraces) + var stackTraces = _stackTraceParser.ParseAndNormalize(testResult.ErrorStackTrace); + var stackTrace = stackTraces.FirstOrDefault(x => !string.IsNullOrWhiteSpace(x.File) && File.Exists(x.File)) + ?? stackTraces.FirstOrDefault(); + + if (stackTrace != null) { var url = !string.IsNullOrWhiteSpace(stackTrace.File) ? TryGenerateFilePermalink(stackTrace.File, stackTrace.Line) @@ -126,20 +119,13 @@ static string FormatTimeSpan(TimeSpan timeSpan) { if (string.IsNullOrWhiteSpace(_serverUrl) || string.IsNullOrWhiteSpace(_repository) || - string.IsNullOrWhiteSpace(_workspace) || string.IsNullOrWhiteSpace(_sha)) { return null; } - var filePathNormalized = filePath - .Replace('\\', '/') - .TrimEnd('/') - .Replace(_workspace, "", StringComparison.Ordinal) - .Trim('/'); - line = string.IsNullOrWhiteSpace(line) ? "" : $"#L{line}"; - return $"{_serverUrl}/{_repository}/blob/{_sha}/{filePathNormalized}{line}"; + return $"{_serverUrl}/{_repository}/blob/{_sha}/{filePath}{line}"; } } diff --git a/tests/Logger.UnitTests/StackTraceParserTests.cs b/tests/Logger.UnitTests/StackTraceParserTests.cs new file mode 100644 index 0000000..fc0782e --- /dev/null +++ b/tests/Logger.UnitTests/StackTraceParserTests.cs @@ -0,0 +1,29 @@ + +namespace GitHub.VsTest.Logger.UnitTests; +public class StackTraceParserTests +{ + [Fact] + public void ParseAndNormalize_Should_Return_RelativePath() + { + const string stackTrace = @"System.Collections.Generic.KeyNotFoundException : Unable to find price for ServiceId=1, Quantity=1 + at SomeCompany.SomeNamespace.Tests.AccountTests.BillForService_Works_ForPlatformServices() in /__NOT_FROM_WORKSPACE__/src/tests/Domain.Tests/AccountTests.cs:line 87 + at SomeCompany.SomeNamespace.Plan.GetPriceTier(Service service, Int64 quantity) in /src/src/Domain/Plan.cs:line 31 + at SomeCompany.SomeNamespace.BillingPeriod.BillForPlatformService(PriceList priceList, ServiceRecord serviceRecord)+MoveNext() in /src/src/Domain/BillingPeriod.cs:line 71 + at SomeCompany.SomeNamespace.Tests.AccountTests.BillForService_Works_ForPlatformServices() in /__NOT_FROM_WORKSPACE__/src/tests/Domain.Tests/AccountTests.cs:line 87 + at System.Collections.Generic.LargeArrayBuilder`1.AddRange(IEnumerable`1 items) + at System.Collections.Generic.EnumerableHelpers.ToArray[T](IEnumerable`1 source) + at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source) + at SomeCompany.SomeNamespace.Account.BillForService(PriceList priceList, BillingPeriod period, ServiceRecord serviceRecord) in /src/src/Domain/Account.cs:line 79 + at SomeCompany.SomeNamespace.Tests.AccountTests.BillForService_Works_ForPlatformServices() in /src/tests/Domain.Tests/AccountTests.cs:line 87 + +"; + var parser = new StackTraceParser(workspacePath: "/src"); + var result = parser.ParseAndNormalize(stackTrace); + + var outFromWorkspace = result.FirstOrDefault(); + var projectFile = result.FirstOrDefault(x => x.File.StartsWith("src/", StringComparison.Ordinal)); + + Assert.Equal("/__NOT_FROM_WORKSPACE__/src/tests/Domain.Tests/AccountTests.cs", outFromWorkspace?.File); + Assert.Equal("src/Domain/Plan.cs", projectFile?.File); + } +} \ No newline at end of file