Skip to content

Commit

Permalink
fix: multiple stack trace annotations
Browse files Browse the repository at this point in the history
  • Loading branch information
vchirikov committed Oct 28, 2022
1 parent bf969c1 commit 9b91db5
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 48 deletions.
47 changes: 22 additions & 25 deletions src/dotnet/Logger/Logger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -51,13 +52,19 @@ public void Initialize(TestLoggerEvents events, Dictionary<string, string> 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";
Expand Down Expand Up @@ -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)
{
Expand Down
59 changes: 59 additions & 0 deletions src/dotnet/Logger/StackTraceParser.cs
Original file line number Diff line number Diff line change
@@ -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<KeyValuePair<string, string>> Parameters { get; set; } = new Dictionary<string, string>(StringComparer.Ordinal);
public string File { get; set; } = "";
public string Line { get; set; } = "";
}

internal partial class StackTraceParser
{
private readonly string _workspacePath;

/// <summary>ctor</summary>
/// <param name="workspacePath">Full path to the current dir / workspace folder</param>
public StackTraceParser(string workspacePath) => _workspacePath = workspacePath;

public static IEnumerable<StackTraceInfo> 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;


/// <summary> Returns first parsed stack traces with relative file path </summary>
public IEnumerable<StackTraceInfo> 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;
}
}
}
32 changes: 9 additions & 23 deletions src/dotnet/Logger/TestRunSummaryGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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}";
}
}
29 changes: 29 additions & 0 deletions tests/Logger.UnitTests/StackTraceParserTests.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}

0 comments on commit 9b91db5

Please sign in to comment.