Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix XUnit Log Fixer Irregularities #85183

Merged
merged 6 commits into from
Apr 28, 2023
Merged

Fix XUnit Log Fixer Irregularities #85183

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5bcc000
Fixed the log tags regular expressions.
ivdiazsa Apr 19, 2023
ff08687
Added test exit code to the XUnit Log Checker, and made it return it …
ivdiazsa Apr 19, 2023
dbd3fe5
Finished fixes.
ivdiazsa Apr 21, 2023
172da68
Decoupled the test's exit code from the XUnit Log Checker.
ivdiazsa Apr 24, 2023
2419b04
Merge branch 'main' into xml-in-check
ivdiazsa Apr 25, 2023
ba7fb10
Merge branch 'main' into xml-in-check
ivdiazsa Apr 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 31 additions & 36 deletions src/tests/Common/XUnitLogChecker/XUnitLogChecker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ public class XUnitLogChecker
{
private static class Patterns
{
public const string OpenTag = @"(\B<\w+)|(\B<!\[CDATA\[)";
public const string CloseTag = @"(\B</\w+>)|(\]\]>)";
public const string OpenTag = @"(\B<\w+[-]?(\w+)?)|(\B<!\[CDATA\[)";
public const string CloseTag = @"(\B</\w+[-]?(\w+)?)|(\]\]>)";
}

private readonly struct TagResult
Expand Down Expand Up @@ -59,6 +59,16 @@ static int Main(string[] args)
string finalLogPath = Path.Combine(resultsDir, finalLogName);
string statsCsvPath = Path.Combine(resultsDir, statsCsvName);

// If the final results log file is present, then we can assume everything
// went fine, and it's ready to go without any further processing.

if (File.Exists(finalLogPath))
{
Console.WriteLine($"[XUnitLogChecker]: Item '{wrapperName}' did"
+ " complete successfully!");
return SUCCESS;
}

// If there are no logs, then this work item was probably entirely skipped.
// This can happen under certain specific circumstances, such as with the
// JIT Hardware Intrinsics tests with DOTNET_GCStress enabled. See Github
Expand All @@ -78,16 +88,6 @@ static int Main(string[] args)
return SUCCESS;
}

// If the final results log file is present, then we can assume everything
// went fine, and it's ready to go without any further processing.

if (File.Exists(finalLogPath))
{
Console.WriteLine($"[XUnitLogChecker]: Item '{wrapperName}' did"
+ " complete successfully!");
return SUCCESS;
}

// If we're here, then that means we've got something to fix.
// First, read the stats csv file. If it doesn't exist, then we can
// assume something went very badly and will likely cause more issues
Expand Down Expand Up @@ -197,18 +197,6 @@ static IEnumerable<string> TryReadFile(string filePath)
return fileContents;
}

static void PrintMissingCrashPath(string wrapperName,
string crashFileType,
string crashFilePath)
{
Console.WriteLine($"[XUnitLogChecker]: Item '{wrapperName}' did not complete"
+ $" successfully, but there was no {crashFileType} found."
+ " The XML log was fixed successfully though.");

Console.WriteLine($"[XUnitLogChecker]: Expected {crashFileType} path"
+ $" was '{crashFilePath}'");
}

static void PrintWorkItemSummary(int numExpectedTests, int[] workItemEndStatus)
{
Console.WriteLine($"\n{workItemEndStatus[0]}/{numExpectedTests} tests run.");
Expand Down Expand Up @@ -255,12 +243,21 @@ static bool FixTheXml(string xFile)
// We are beginning to process a test's output. Set the flag to
// treat everything as such, until we get the closing output tag.
if (tagText.Equals("output") && !inOutput && !inCData)
{
inOutput = true;
}
else if (tagText.Equals("CDATA") && !inCData)
{
inCData = true;
tags.Push(tagText);
continue;
}

tags.Push(tagText);
continue;
// CDATA tags store plain output, which can include tag-like
// looking strings. So, we skip those until we're done processing
// the current CDATA tag.
if (!inCData)
tags.Push(tagText);
}

// Found a closing tag. If we're currently in an output state, then
Expand All @@ -280,28 +277,26 @@ static bool FixTheXml(string xFile)

if (inCData)
{
if (tagText.Equals("CDATA"))
if (tagText.Equals("CDATA") && tagText.Equals(tags.Peek()))
{
tags.Pop();
inCData = false;
}
else continue;
continue;
}

if (inOutput)
{
if (tagText.Equals("output"))
{
tags.Pop();
inOutput = false;
}
else continue;
if (tagText.Equals("output") && tagText.Equals(tags.Peek()))
{
tags.Pop();
inOutput = false;
}
continue;
}

if (tagText.Equals(tags.Peek()))
{
tags.Pop();
}
}
}
}
Expand Down
25 changes: 25 additions & 0 deletions src/tests/Common/helixpublishwitharcade.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,10 +415,35 @@
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' == 'true'" Include="set TEST_HARNESS_STRIPE_TO_EXECUTE=.0.1" />
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' != 'true'" Include="export TEST_HARNESS_STRIPE_TO_EXECUTE=.0.1" />
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' != 'true'" Include="chmod +x $(_MergedWrapperRunScriptRelative)" />

<!-- Force assemblies to lazy-load for LLVM AOT test runs to enable using tests that fail at AOT time (and as a result can't be AOTd) -->
<HelixCommandLines Condition="'$(RuntimeVariant)' == 'llvmfullaot'" Include="$(_MergedWrapperRunScriptPrefix)$(_MergedWrapperRunScriptRelative) -usewatcher --aot-lazy-assembly-load" />
<HelixCommandLines Condition="'$(RuntimeVariant)' != 'llvmfullaot'" Include="$(_MergedWrapperRunScriptPrefix)$(_MergedWrapperRunScriptRelative) -usewatcher" />

<!--
Bug Fix: GH Issue #85056 - Helix takes the exit code of the last ran executable. Since that spot has now been
taken by the log fixer, its exit code gets reported to Helix, and thus eclipsing the test's one. This might
lead to test failures being identified as passed if the log checker finishes successfully. To fix this, we
store the test's exit code, and if it's not successful, then we return it after the log checker finishes.
-->
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' == 'true'" Include="set test_exit_code=%25ERRORLEVEL%25" />
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' != 'true'" Include="test_exit_code=%24%3F" />

<!-- Add the XUnitLogChecker's running command and store its exit code. -->
<HelixCommandLines Include="$(XUnitLogCheckerCommand)" />
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' == 'true'" Include="set xunitlogchecker_exit_code=%25ERRORLEVEL%25" />
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' != 'true'" Include="xunitlogchecker_exit_code=%24%3F" />

<!--
Check both exit codes, the test's and XUnitLogChecker's. If any of them failed, then we return that one, with the
test's taking priority (i.e. if the test failed, we return that one regardless of how things went with the log checker.
Full description in the reasoning of this is in the comment above about GH issue #85056.
-->
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' == 'true'" Include="if %25test_exit_code%25 NEQ 0 exit %25test_exit_code%25" />
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' != 'true'" Include="if [ %24test_exit_code -ne 0 ]%3B then exit %24test_exit_code%3B fi" />
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' == 'true'" Include="if %25xunitlogchecker_exit_code%25 NEQ 0 exit %25xunitlogchecker_exit_code%25" />
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' != 'true'" Include="if [ %24xunitlogchecker_exit_code -ne 0 ]%3B then exit %24xunitlogchecker_exit_code%3B fi" />
<HelixCommandLines Include="exit 0" />
</ItemGroup>

<Copy SourceFiles="@(ReducedMergedPayloadFilesFinal)" DestinationFiles="@(ReducedMergedPayloadFilesFinal->'$(MergedPayloadsRootDirectory)\$(_MergedWrapperName)\%(FileRelativeToPayloadsRootDirectory)')" Condition="'@(ReducedMergedPayloadFilesFinal)' != ''" />
Expand Down