From 24e71c8c455db6a1b3de4731dbc0b95c1d16feda Mon Sep 17 00:00:00 2001 From: John Lambert Date: Wed, 9 Sep 2026 16:26:10 -0400 Subject: [PATCH 1/4] Gate render baselines on size, pixel count and difference magnitude The verifier passed a snapshot when at most 4 pixels differed from the baseline. Font-smoothing drift between machines touches up to 50 pixels per scenario, each off by about 8 levels on one channel, so every RenderVerifyTests scenario failed on a box other than the one that captured the baselines. Pass only when the image size matches, fewer than 100 pixels differ, and the summed difference stays under 10 full-pixel equivalents, where a channel-saturated change on one pixel scores 1. Measured drift peaks at 50 pixels and magnitude 1.57; a shifted glyph touches thousands of pixels at a magnitude near 1 each, so it still fails. A size change fails regardless of tolerance so a layout regression cannot hide inside the pixel budget. The failure message now reports both measures, and the diff report records the magnitude limit. VerifyScenario drops an await of Task.CompletedTask that bought nothing under [Apartment(STA)]. Co-Authored-By: Claude Fable 5.1 --- .../RenderSnapshotVerifier.cs | 63 +++++++++++++++++-- .../RootSiteTests/RenderVerifyTests.cs | 7 +-- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/Src/Common/RenderVerification/RenderSnapshotVerifier.cs b/Src/Common/RenderVerification/RenderSnapshotVerifier.cs index 7cf86e6588..0bb9c34e97 100644 --- a/Src/Common/RenderVerification/RenderSnapshotVerifier.cs +++ b/Src/Common/RenderVerification/RenderSnapshotVerifier.cs @@ -16,7 +16,11 @@ public static class RenderSnapshotVerifier { private const string UpdateBaselinesEnvVar = "FW_UPDATE_RENDER_BASELINES"; private const string FontQualityEnvVar = "FW_FONT_QUALITY"; - private const int MaxAllowedPixelDifferences = 4; + // Font-smoothing drift between machines peaks at 50 touched pixels and magnitude + // 1.57, three channels off about 8 levels each; a shifted glyph touches thousands + // at a magnitude near 1. + private const int MaxAllowedPixelDifferences = 100; + private const double MaxAllowedDifferenceMagnitude = 10.0; private const string DeterministicRenderFontFamily = "Segoe UI"; private const int DpiAwarenessInvalid = -1; private const int DpiAwarenessUnaware = 0; @@ -81,7 +85,11 @@ public static RenderBaselineVerificationResult Verify(Bitmap actualBitmap, strin { var savedArtifact = LoadSavedArtifact(expectedBitmap, verifiedPath, verifiedMetadataPath); var diffSummary = CompareBitmaps(expectedBitmap, actualBitmap); - if (diffSummary.DifferentPixelCount <= MaxAllowedPixelDifferences) + // A size change is a layout regression, so it fails whatever the pixel tolerance + // allows. + bool sizeMatches = expectedBitmap.Width == actualBitmap.Width + && expectedBitmap.Height == actualBitmap.Height; + if (sizeMatches && IsWithinTolerance(diffSummary)) { DeleteIfPresent(receivedPath); DeleteIfPresent(receivedMetadataPath); @@ -113,6 +121,7 @@ public static RenderBaselineVerificationResult Verify(Bitmap actualBitmap, strin ScenarioId = scenarioId, SnapshotName = name, AllowedDifferentPixelCount = MaxAllowedPixelDifferences, + AllowedDifferenceMagnitude = MaxAllowedDifferenceMagnitude, SavedBaseline = savedArtifact, CurrentRun = currentArtifact, Diff = diffSummary, @@ -299,11 +308,18 @@ private static string BuildFailureMessage( { var builder = new StringBuilder(); builder.AppendFormat(CultureInfo.InvariantCulture, - "Render output for '{0}' differed from baseline by {1} pixels; {2} or fewer differences are allowed.", + "Render output for '{0}' differed from baseline by {1} pixels ({2} full-pixel equivalents); fewer than {3} pixels and fewer than {4} full-pixel equivalents are allowed.", scenarioId, report.Diff.DifferentPixelCount, - report.AllowedDifferentPixelCount); + FormatMagnitude(report.Diff.DifferenceMagnitude), + report.AllowedDifferentPixelCount, + FormatMagnitude(report.AllowedDifferenceMagnitude)); builder.AppendLine(); + if (report.SavedBaseline.ImageWidth != report.CurrentRun.ImageWidth + || report.SavedBaseline.ImageHeight != report.CurrentRun.ImageHeight) + { + builder.AppendLine("Image size changed, which fails regardless of the pixel tolerance."); + } builder.AppendLine(FormatArtifactLine("Saved baseline", report.SavedBaseline)); builder.AppendLine(FormatArtifactLine("Current run", report.CurrentRun)); builder.AppendFormat(CultureInfo.InvariantCulture, @@ -530,6 +546,35 @@ private static void DeleteIfPresent(string path) File.Delete(path); } + /// + /// Answers whether a diff is small enough to be environment drift rather than a render + /// change. The touched-pixel count and the summed magnitude must both stay under their + /// limit. + /// + private static bool IsWithinTolerance(RenderPixelDiffSummary summary) + { + return summary.DifferentPixelCount < MaxAllowedPixelDifferences + && summary.DifferenceMagnitude < MaxAllowedDifferenceMagnitude; + } + + /// + /// Scores one differing pixel. All three channels fully inverted score 1, so a single + /// saturated channel scores a third and the magnitude limit admits three times as many + /// single-channel changes as whole-pixel ones. Alpha is not scored. + /// + private static double PixelDifferenceMagnitude(Color expected, Color actual) + { + int channelDelta = Math.Abs(expected.R - actual.R) + + Math.Abs(expected.G - actual.G) + + Math.Abs(expected.B - actual.B); + return channelDelta / (3.0 * 255.0); + } + + private static string FormatMagnitude(double magnitude) + { + return magnitude.ToString("0.##", CultureInfo.InvariantCulture); + } + private static RenderPixelDiffSummary CompareBitmaps(Bitmap expectedBitmap, Bitmap actualBitmap) { int maxWidth = Math.Max(expectedBitmap.Width, actualBitmap.Width); @@ -546,6 +591,7 @@ private static RenderPixelDiffSummary CompareBitmaps(Bitmap expectedBitmap, Bitm if (!expectedInBounds || !actualInBounds) { summary.DifferentPixelCount++; + summary.DifferenceMagnitude += 1.0; if (expectedInBounds) summary.ExpectedOnlyPixelDifferences++; else if (actualInBounds) @@ -554,11 +600,14 @@ private static RenderPixelDiffSummary CompareBitmaps(Bitmap expectedBitmap, Bitm continue; } - if (expectedBitmap.GetPixel(x, y) == actualBitmap.GetPixel(x, y)) + Color expectedPixel = expectedBitmap.GetPixel(x, y); + Color actualPixel = actualBitmap.GetPixel(x, y); + if (expectedPixel == actualPixel) continue; summary.DifferentPixelCount++; summary.InBoundsPixelDifferences++; + summary.DifferenceMagnitude += PixelDifferenceMagnitude(expectedPixel, actualPixel); UpdateDiffBounds(summary, x, y); } } @@ -670,6 +719,7 @@ public sealed class RenderSnapshotComparisonReport public string ScenarioId { get; set; } public string SnapshotName { get; set; } public int AllowedDifferentPixelCount { get; set; } + public double AllowedDifferenceMagnitude { get; set; } public RenderSnapshotArtifact SavedBaseline { get; set; } public RenderSnapshotArtifact CurrentRun { get; set; } public RenderPixelDiffSummary Diff { get; set; } @@ -679,6 +729,7 @@ public sealed class RenderSnapshotComparisonReport public sealed class RenderPixelDiffSummary { public int DifferentPixelCount { get; set; } + public double DifferenceMagnitude { get; set; } public int InBoundsPixelDifferences { get; set; } public int ExpectedOnlyPixelDifferences { get; set; } public int ActualOnlyPixelDifferences { get; set; } @@ -689,4 +740,4 @@ public sealed class RenderPixelDiffSummary public int DiffRegionWidth { get; set; } public int DiffRegionHeight { get; set; } } -} \ No newline at end of file +} diff --git a/Src/Common/RootSite/RootSiteTests/RenderVerifyTests.cs b/Src/Common/RootSite/RootSiteTests/RenderVerifyTests.cs index 893f1bef07..78279ced18 100644 --- a/Src/Common/RootSite/RootSiteTests/RenderVerifyTests.cs +++ b/Src/Common/RootSite/RootSiteTests/RenderVerifyTests.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.Threading; -using System.Threading.Tasks; using NUnit.Framework; using SIL.FieldWorks.Common.RootSites.RenderBenchmark; @@ -16,7 +15,7 @@ namespace SIL.FieldWorks.Common.RootSites /// Each run saves a .received.png and compares it against the committed /// .verified.png baseline by decoded pixel values, not by the PNG file bytes. /// Small encoder-level differences are therefore ignored as long as the rendered - /// image differs by fewer than five pixels. + /// image stays within the verifier's pixel-count and difference-magnitude tolerance. /// /// Each scenario is set up inside its own UndoableUnitOfWork, matching the pattern /// used by RenderTimingSuiteTests. @@ -40,13 +39,11 @@ protected override void CreateTestData() /// compares decoded pixels against the committed .verified.png baseline. /// [Test, TestCaseSource(nameof(GetVerifyScenarios))] - public async Task VerifyScenario(string scenarioId) + public void VerifyScenario(string scenarioId) { var execution = ExecuteScenarioAndCapture(scenarioId, includeWarmRender: false); if (!execution.Verification.Passed) Assert.Fail(execution.Verification.FailureMessage); - - await Task.CompletedTask; } /// From 26d268d51f7cd8483e88b18bcb7102c410361445 Mon Sep 17 00:00:00 2001 From: John Lambert Date: Wed, 9 Sep 2026 16:26:42 -0400 Subject: [PATCH 2/4] End unit++ test processes explicitly after GlobalTeardown on Windows TestViews reported [309-0-0] and retval=0, yet the runner returned -1. The Uniscribe shaping path loads the OS text-input stack, which connects to TextInputHost.exe over ALPC. The console harness has an STA apartment but no message pump, so after main returns those threads never finish tearing down and the process hangs. The runner kills it after its grace period, and that TerminateProcess is where the -1 came from. Call TerminateProcess with retval once every test and GlobalTeardown have run and stdout is flushed. Static destructors no longer run, which is a deliberate trade: everything meaningful has already completed. main.cc already carries repo-specific Windows patches (SuppressInteractiveCrashUi, TerminateOnSigAbrt), so this follows existing practice for the vendored harness. Co-Authored-By: Claude Fable 5.1 --- Lib/src/unit++/main.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Lib/src/unit++/main.cc b/Lib/src/unit++/main.cc index 37c47eea46..742f92c14e 100644 --- a/Lib/src/unit++/main.cc +++ b/Lib/src/unit++/main.cc @@ -160,6 +160,13 @@ int main(int argc, const char* argv[]) retval = 1; } printf("DEBUG: unit++ main end (retval=%d)\n", retval); fflush(stdout); +#if defined(UNITPP_WINDOWS) + // The OS text-input stack loaded by text shaping never finishes teardown without a + // message pump, so end the process once tests and GlobalTeardown have run. Static + // destructors are skipped. + fflush(NULL); + ::TerminateProcess(::GetCurrentProcess(), static_cast(retval)); +#endif return retval; } From d52ec38eba5b469e4f58a9cfd067252595d67fa8 Mon Sep 17 00:00:00 2001 From: John Lambert Date: Wed, 9 Sep 2026 16:27:18 -0400 Subject: [PATCH 3/4] Retry crashed test hosts in test.ps1 and report the run as flaky A crashed test host aborts the vstest run, so every assembly after it never reports. The existing per-assembly fallback only fired on exit code -1, while a host crash returns 1, so about 1,600 tests vanished silently from a run that still ended with a summary. Detect the crash line in the vstest output and fall back to per-assembly runs. Each assembly, and a single-assembly run, is retried up to -MaxCrashAttempts (default 5) only when its host crashed; a reported test failure is an answer and is never retried. Match only "Test host process crashed", because vstest also prints the aborted line on Ctrl+C and CI cancellation. A run that passed only after a retry prints a [FLAKY] banner, writes TestResults/crash-retries.json, appends to the GitHub step summary, and exits with code 2 so a gate that reads only the exit code still sees it. CI uploads the report with the TRX artifacts. -Blame passes /Blame to vstest so a crash leaves a Sequence_*.xml naming the running test. Co-Authored-By: Claude Fable 5.1 --- .github/workflows/CI.yml | 1 + test.ps1 | 177 ++++++++++++++++++++++++++++++++++----- 2 files changed, 155 insertions(+), 23 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 04bc8f74b3..ab3116a508 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -185,6 +185,7 @@ jobs: name: trx-results path: | **/*.trx + **/TestResults/crash-retries.json if-no-files-found: warn - uses: actions/upload-artifact@v7 diff --git a/test.ps1 b/test.ps1 index 4695d7f2d3..27c61f0f5d 100644 --- a/test.ps1 +++ b/test.ps1 @@ -69,6 +69,15 @@ (when installed; otherwise they run bare with a warning), writing native..cobertura.xml to the same TestResults folder. +.PARAMETER MaxCrashAttempts + How many times one managed test assembly is run when its test host crashes (default 5). + Only a crash is retried, never a reported test failure. A run that passed only after a + retry prints [FLAKY], writes TestResults/crash-retries.json, and exits with code 2. + +.PARAMETER Blame + Passes /Blame to vstest.console.exe so a host crash leaves a Sequence_*.xml naming the + test that was running. + .EXAMPLE .\test.ps1 Runs all tests in Debug configuration (builds first if needed). @@ -120,7 +129,10 @@ param( [string]$StartedBy = 'unknown', [switch]$CommentHygiene, [switch]$TokenHygiene, - [switch]$LocalLibraryTests + [switch]$LocalLibraryTests, + [ValidateRange(1, 20)] + [int]$MaxCrashAttempts = 5, + [switch]$Blame ) $ErrorActionPreference = 'Stop' @@ -277,6 +289,59 @@ function Get-CentralPackageVersion { return $null } +function Test-HostCrashed { + param([string]$OutputText) + + # vstest also prints "The active test run was aborted" on Ctrl+C and CI cancellation, so only + # the crash line marks a run whose results are missing rather than failed. + return $OutputText -match 'Test host process crashed' +} + +function Invoke-VsTestWithCrashRetry { + param( + [string]$VsTestPath, + [string[]]$Arguments, + [string]$CoverageOutputPath, + [int]$MaxAttempts, + [string]$Label + ) + + # Only a crash is retried. A run that reported test failures is already an answer, and + # retrying it would turn a genuine failure into a false pass. + $attempt = 0 + $exitCode = 0 + $output = $null + $previousEap = $ErrorActionPreference + $ErrorActionPreference = 'Continue' + try { + while ($true) { + $attempt++ + if ($CoverageOutputPath) { + & dotnet tool run dotnet-coverage collect -f cobertura -o $CoverageOutputPath --nologo $VsTestPath @Arguments 2>&1 | Tee-Object -Variable output | Out-Host + } + else { + & $VsTestPath $Arguments 2>&1 | Tee-Object -Variable output | Out-Host + } + $exitCode = $LASTEXITCODE + + if ($exitCode -eq 0 -or $attempt -ge $MaxAttempts -or -not (Test-HostCrashed ($output | Out-String))) { + break + } + + Write-Host "[WARN] $Label crashed its test host (attempt $attempt of $MaxAttempts). Retrying; results so far are incomplete, not a pass." -ForegroundColor Yellow + } + } + finally { + $ErrorActionPreference = $previousEap + } + + return [pscustomobject]@{ + ExitCode = $exitCode + Output = $output + Attempts = $attempt + } +} + function Get-NUnitTestAdapterPaths { param( [string]$RepoRoot, @@ -340,6 +405,7 @@ $cleanupArgs = @{ } $testExitCode = 0 +$script:crashRetryReport = @() $script:coverageFailed = $false try { @@ -716,6 +782,10 @@ try { $vstestVerbosity = $verbosityMap[$Verbosity] $vstestArgs += "/Logger:trx" $vstestArgs += "/Logger:console;verbosity=$vstestVerbosity" + if ($Blame) { + # Records the test that was running when a host crashed, as Sequence_*.xml. + $vstestArgs += "/Blame" + } if ($TestFilter) { $vstestArgs += "/TestCaseFilter:$TestFilter" @@ -769,22 +839,22 @@ try { } Write-Host "" - $previousEap = $ErrorActionPreference - $ErrorActionPreference = 'Continue' - try { - if ($coverageOutputPath) { - & dotnet tool run dotnet-coverage collect -f cobertura -o $coverageOutputPath --nologo $vstestPath @vstestArgs 2>&1 | Tee-Object -Variable testOutput - } - else { - & $vstestPath $vstestArgs 2>&1 | Tee-Object -Variable testOutput - } - # Don't overwrite a non-zero exit code from native tests with a zero exit code from these tests. - if ($LASTEXITCODE -ne 0) { - $script:testExitCode = $LASTEXITCODE + # A multi-assembly run falls back to per-assembly runs below when a host crashes, so only + # a single-assembly run is retried here. + $mainAttempts = if ($testDlls.Count -eq 1) { $MaxCrashAttempts } else { 1 } + $mainRun = Invoke-VsTestWithCrashRetry -VsTestPath $vstestPath -Arguments $vstestArgs ` + -CoverageOutputPath $coverageOutputPath -MaxAttempts $mainAttempts -Label 'The test run' + $testOutput = $mainRun.Output + if ($mainRun.Attempts -gt 1) { + $script:crashRetryReport += [pscustomobject]@{ + Assembly = [System.IO.Path]::GetFileNameWithoutExtension($testDlls[0]) + Attempts = $mainRun.Attempts + Recovered = ($mainRun.ExitCode -eq 0) } } - finally { - $ErrorActionPreference = $previousEap + # Keep a non-zero exit code from native tests; a passing managed run must not clear it. + if ($mainRun.ExitCode -ne 0) { + $script:testExitCode = $mainRun.ExitCode } $vstestLogPath = Join-Path $resultsDir "vstest.console.log" @@ -796,19 +866,27 @@ try { Write-Host "[WARN] Failed to write VSTest output log to $vstestLogPath" -ForegroundColor Yellow } + $outputText = ($testOutput | Out-String) if ($script:testExitCode -ne 0) { - $outputText = ($testOutput | Out-String) if ($outputText -match 'used by another process|file is locked|cannot access the file') { throw "Detected possible file is locked during vstest execution." } } + # A crashed test host aborts the whole run, so the assemblies after it never report. + $hostCrashAborted = $script:testExitCode -ne 0 -and (Test-HostCrashed $outputText) + # ============================================================================= # Workaround: multi-assembly VSTest may fail with exit code -1 and minimal output # ============================================================================= - if (-not $ListTests -and $testDlls.Count -gt 1 -and $script:testExitCode -eq -1) { - Write-Host "[WARN] vstest.console.exe returned exit code -1 with multiple test assemblies. Retrying per-assembly to isolate failures." -ForegroundColor Yellow + if (-not $ListTests -and $testDlls.Count -gt 1 -and ($script:testExitCode -eq -1 -or $hostCrashAborted)) { + if ($hostCrashAborted) { + Write-Host "[WARN] A test host crashed and aborted the run. Retrying per-assembly so every assembly reports." -ForegroundColor Yellow + } + else { + Write-Host "[WARN] vstest.console.exe returned exit code -1 with multiple test assemblies. Retrying per-assembly to isolate failures." -ForegroundColor Yellow + } $timestamp = Get-Date -Format 'yyyyMMdd_HHmmss' $overallExitCode = 0 @@ -828,19 +906,30 @@ try { } $singleArgs += "/Logger:trx;LogFileName=${dllName}_${timestamp}.trx" $singleArgs += "/Logger:console;verbosity=$vstestVerbosity" + if ($Blame) { + $singleArgs += "/Blame" + } if ($TestFilter) { $singleArgs += "/TestCaseFilter:$TestFilter" } + $singleCoverageOutput = $null if ($coverageOutputPath) { $singleCoverageOutput = Join-Path $resultsDir "coverage.${dllName}.cobertura.xml" - & dotnet tool run dotnet-coverage collect -f cobertura -o $singleCoverageOutput --nologo $vstestPath @singleArgs 2>&1 | Tee-Object -Variable singleTestOutput } - else { - & $vstestPath $singleArgs 2>&1 | Tee-Object -Variable singleTestOutput + $singleRun = Invoke-VsTestWithCrashRetry -VsTestPath $vstestPath -Arguments $singleArgs ` + -CoverageOutputPath $singleCoverageOutput -MaxAttempts $MaxCrashAttempts -Label $dllName + $singleExitCode = $singleRun.ExitCode + $singleTestOutput = $singleRun.Output + if ($singleRun.Attempts -gt 1) { + $script:crashRetryReport += [pscustomobject]@{ + Assembly = $dllName + Attempts = $singleRun.Attempts + Recovered = ($singleExitCode -eq 0) + } } - $singleExitCode = $LASTEXITCODE + if ($singleExitCode -ne 0 -and $overallExitCode -eq 0) { $overallExitCode = $singleExitCode } @@ -975,7 +1064,49 @@ if ($script:coverageFailed -and $testExitCode -eq 0) { Write-Host "[FAIL] Tests passed but code coverage collection failed (see [ERROR] above)." -ForegroundColor Red } -if ($testExitCode -eq 0) { +# A crash that only passed on a retry is not a clean run, so it never reports as one. +if ($script:crashRetryReport.Count -gt 0) { + # $resultsDir was assigned inside the scriptblock that ran the tests, so it is out of + # scope here; rebuild the path as the failure summary above rebuilds its log path. + $crashReportPath = Join-Path $PSScriptRoot "Output/$Configuration/TestResults/crash-retries.json" + try { + $script:crashRetryReport | ConvertTo-Json -Depth 3 | Out-File -FilePath $crashReportPath -Encoding UTF8 + } + catch { + Write-Host "[WARN] Failed to write crash retry report to $crashReportPath" -ForegroundColor Yellow + } + + $crashLines = foreach ($entry in $script:crashRetryReport) { + $verdict = if ($entry.Recovered) { "passed on attempt $($entry.Attempts)" } else { "still failing after $($entry.Attempts) attempts" } + " {0}: {1}" -f $entry.Assembly, $verdict + } + + Write-Host "" + Write-Host "========== FLAKY: TEST HOST CRASHED ==========" -ForegroundColor Magenta + $crashLines | ForEach-Object { Write-Host $_ -ForegroundColor Magenta } + Write-Host " This is a real defect. Retrying only recovered the results; it did not fix the crash." -ForegroundColor Magenta + Write-Host " Report: $crashReportPath" -ForegroundColor Magenta + Write-Host "==============================================" -ForegroundColor Magenta + + # GitHub Actions shows the step summary on the run page, where a retried crash stays visible. + if ($env:GITHUB_STEP_SUMMARY) { + try { + @("### Flaky: a test host crashed and was retried", "") + ($crashLines | ForEach-Object { "- " + $_.Trim() }) | + Out-File -FilePath $env:GITHUB_STEP_SUMMARY -Append -Encoding UTF8 + } + catch { + Write-Host "[WARN] Failed to append the crash retry report to the GitHub step summary" -ForegroundColor Yellow + } + } +} + +if ($testExitCode -eq 0 -and $script:crashRetryReport.Count -gt 0) { + # Exit code 2 keeps a recovered crash visible to gates that only read the exit code. + $testExitCode = 2 + Write-Host "" + Write-Host "[FLAKY] All tests passed, but a test host crashed and had to be retried (exit code: 2)" -ForegroundColor Magenta +} +elseif ($testExitCode -eq 0) { Write-Host "" Write-Host "[PASS] All tests passed" -ForegroundColor Green } From e56d7e991392417a0cb7fd2e87376ed7681fc257 Mon Sep 17 00:00:00 2001 From: Hasso <4933670+papeh@users.noreply.github.com> Date: Fri, 11 Sep 2026 15:32:29 -0500 Subject: [PATCH 4/4] tweak a wording and an elseif block --- .../RenderSnapshotVerifier.cs | 4 ++-- test.ps1 | 20 ++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/Src/Common/RenderVerification/RenderSnapshotVerifier.cs b/Src/Common/RenderVerification/RenderSnapshotVerifier.cs index 0bb9c34e97..23a5a798b5 100644 --- a/Src/Common/RenderVerification/RenderSnapshotVerifier.cs +++ b/Src/Common/RenderVerification/RenderSnapshotVerifier.cs @@ -85,8 +85,8 @@ public static RenderBaselineVerificationResult Verify(Bitmap actualBitmap, strin { var savedArtifact = LoadSavedArtifact(expectedBitmap, verifiedPath, verifiedMetadataPath); var diffSummary = CompareBitmaps(expectedBitmap, actualBitmap); - // A size change is a layout regression, so it fails whatever the pixel tolerance - // allows. + // A size change is a layout regression, so it fails regardless of the pixel + // tolerance. bool sizeMatches = expectedBitmap.Width == actualBitmap.Width && expectedBitmap.Height == actualBitmap.Height; if (sizeMatches && IsWithinTolerance(diffSummary)) diff --git a/test.ps1 b/test.ps1 index 27c61f0f5d..552e00577c 100644 --- a/test.ps1 +++ b/test.ps1 @@ -1100,15 +1100,17 @@ if ($script:crashRetryReport.Count -gt 0) { } } -if ($testExitCode -eq 0 -and $script:crashRetryReport.Count -gt 0) { - # Exit code 2 keeps a recovered crash visible to gates that only read the exit code. - $testExitCode = 2 - Write-Host "" - Write-Host "[FLAKY] All tests passed, but a test host crashed and had to be retried (exit code: 2)" -ForegroundColor Magenta -} -elseif ($testExitCode -eq 0) { - Write-Host "" - Write-Host "[PASS] All tests passed" -ForegroundColor Green +if ($testExitCode -eq 0) { + if ($script:crashRetryReport.Count -gt 0) { + # Exit code 2 keeps a recovered crash visible to gates that only read the exit code. + $testExitCode = 2 + Write-Host "" + Write-Host "[FLAKY] All tests passed, but a test host crashed and had to be retried (exit code: 2)" -ForegroundColor Magenta + } + else { + Write-Host "" + Write-Host "[PASS] All tests passed" -ForegroundColor Green + } } else { Write-Host ""