Make vacuous CI guards capable of failing

[Adam Ratzman (adamint)]

Summary

Makes the .NET test-result guards fail when CI did not produce trustworthy test execution evidence.

Changes

• Runs result validation after any test runner step that actually succeeded, not only quarantine runs.
• Fails shards whose TRX files report zero tests unless the generated shard explicitly allows that result.
• Treats missing, malformed, unparseable, and negative TRX counters as invalid evidence.
• In quarantine mode, allows only known test-outcome exit codes and fails closed on every other MTP exit code.
• Matches MTP's real *_hang.dmp filenames and classifies result-validation/hang-dump failures as test execution failures for transient reruns.
• Carries the zero-test opt-out only to generated class/specialized shards where an empty OS row is expected.

The extension E2E matrix and npm lockfile registry guards are intentionally not part of this PR.

Validation

dotnet test --project tests/Infrastructure.Tests/Infrastructure.Tests.csproj --no-launch-profile -- \
  --filter-class '*.RunTestsWorkflowTests' \
  --filter-class '*.AutoRerunTransientCiFailuresTests' \
  --filter-class '*.BuildTestMatrixTests' \
  --filter-class '*.ExpandTestMatrixGitHubTests' \
  --filter-not-trait 'quarantined=true' \
  --filter-not-trait 'outerloop=true'

162 passed

Checklist

• Is this feature complete?
  • Yes. Ready to ship.
  • No. Follow-up changes expected.
• Are you including unit tests for the changes and scenario tests if relevant?
  • Yes
  • No
• Did you add public API?
  • Yes
  • No
• Does the change make any security assumptions or guarantees?
  • Yes
  • No

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19177

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19177"

[Copilot]

Pull request overview

Strengthens CI guards so missing tests, hang dumps, incomplete E2E matrices, and empty lockfiles fail reliably.

Changes:

• Validates nonzero TRX test counts and correct hang-dump filenames.
• Prevents retrying genuine test hangs.
• Adds lockfile and E2E matrix safeguards.

Show a summary per file

File	Description
.github/workflows/run-tests.yml	Strengthens test-result and hang-dump validation.
.github/workflows/auto-rerun-transient-ci-failures.js	Classifies hang detection as test failure.
tests/Infrastructure.Tests/WorkflowScripts/RunTestsWorkflowTests.cs	Adds workflow guard tests.
tests/Infrastructure.Tests/WorkflowScripts/AutoRerunTransientCiFailuresTests.cs	Tests hang retry classification.
extension/scripts/validate-lockfile-registry.cjs	Rejects lockfiles without resolved entries.
extension/src/test/e2eLaunchProfile.test.ts	Tests lockfile and E2E matrix completeness.

Review details

Tip

Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 6/6 changed files
• Comments generated: 2
• Review effort level: Balanced

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

[Copilot]

Review details

Suppressed comments (4)

.github/workflows/run-tests.yml:730

• This still cannot detect hangs from the repository's actual MTP invocation: eng/Testing.props:41 passes --hangdump-type none to every generated mtpBaseArgs, and docs/ci/mtp-args-pipeline.md:56 documents that this disables hang-dump file creation. The regression test only creates synthetic _hang.dmp files, so it does not show that CI produces anything this glob can match. Either enable dump creation or classify hangs from evidence that exists with the current configuration (for example, the persisted MTP timeout exit code), with a regression test covering that production path.

          # MTP's default hang dump file name is "<process>_<pid>_hang.dmp"; use the suffix so
          # crash dumps and unrelated .dmp files do not fail otherwise-successful runs.
          $hangDumpFiles = Get-ChildItem -Path $testResultsDir -Filter *_hang.dmp -Recurse -ErrorAction SilentlyContinue

.github/workflows/run-tests.yml:685

• validFileCount is incremented before Counters/@total is validated, so a TRX with a missing or non-numeric count leaves $totalTestCount at zero and is accepted whenever allowZeroTests is true. The opt-out should permit only a successfully parsed total="0"; count the file as valid only after its total has been parsed.

                if ($countersNode -and $countersNode.total) {
                  $testCount = [int]$countersNode.total
                  $totalTestCount += $testCount

.github/workflows/run-tests.yml:47

• The generated test matrix always includes an uncollected:* backstop (eng/scripts/scan-test-partitions-from-source.ps1:123-124), which runs with --filter-not-trait "Partition=*" (eng/scripts/build-test-matrix.ps1:206-210) and can legitimately match zero tests; eng/Testing.props:41 therefore already ignores MTP exit code 8. None of the reusable-workflow callers pass this new input, so those empty generated shards now fail despite the opt-out. Propagate an explicit matrix flag for the uncollected entry and pass it as allowZeroTests from tests.yml (and any generated-shard caller that needs it).

This issue also appears on line 728 of the same file.

      # Only set this for generated shards that are expected to be empty. Normal CI should
      # fail when a named shard produces .trx files that report zero tests.
      allowZeroTests:
        required: false
        type: boolean
        default: false

.github/workflows/run-tests.yml:660

• Because the configured --hangdump-type none produces no dump file, a tolerated MTP timeout (exit code 3) with no TRX or recording still falls through to “Tests may not have run” and fails quarantine mode. Include the already-read MTP exit code in this classification (or enable dump creation) so the intended timeout path does not depend on an artifact CI suppresses.

            # MTP's default hang dump file name is "<process>_<pid>_hang.dmp"; use the suffix so
            # crash dumps and unrelated .dmp files stay out of this timeout-only classification.
            $hasHangDumps = Get-ChildItem -Path "${{ github.workspace }}/testresults" -Filter *_hang.dmp -Recurse -ErrorAction SilentlyContinue
            if ($hasRecordings -or $hasHangDumps) {

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (1)

tests/Infrastructure.Tests/WorkflowScripts/RunTestsWorkflowTests.cs:334

• Infrastructure.Tests already provides TemporaryWorkspace (used by BuildTestMatrixTests), but this recreates scratch-directory allocation and cleanup manually. That bypasses the shared workspace cleanup/preservation behavior and the repository temp-directory abstraction. Use TemporaryWorkspace.Create(_output) and dispose it rather than maintaining this helper.

        string scratchRoot = Path.Combine(RepoRoot.Path, "artifacts", "tmp", nameof(RunTestsWorkflowTests));
        Directory.CreateDirectory(scratchRoot);

        string scratchDirectory = Path.Combine(scratchRoot, Guid.NewGuid().ToString("N"));
        Directory.CreateDirectory(scratchDirectory);

• Files reviewed: 11/11 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (2)

.github/workflows/run-tests.yml:685

• allowZeroTests currently treats a missing or unparseable Counters/@total as an explicitly reported zero. validFileCount is incremented before this cast, and the catch only warns, so a TRX with total="invalid" (or no Counters) exits successfully when the opt-out is enabled. Track whether a test count was successfully parsed and fail if none was; the opt-out should cover a reported total="0", not malformed result data.

                if ($countersNode -and $countersNode.total) {
                  $testCount = [int]$countersNode.total
                  $totalTestCount += $testCount

.github/workflows/auto-rerun-transient-ci-failures.js:60

• The new Verify test results exist step can now fail for a deterministic zero-test run, but it is absent from testExecutionFailureStepPatterns. Consequently, an unrelated feed signature anywhere in the job log still enables the broad network override and reruns that zero-test job—the same false-rerun path this change closes for hang-dump detection. Classify this step as a test-execution failure and add the corresponding regression case.

    /^Check for hang dump files$/i,

• Files reviewed: 11/11 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

Review details

• Files reviewed: 11/11 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

[Copilot]

Review details

• Files reviewed: 15/15 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (2)

tests/Infrastructure.Tests/WorkflowScripts/RunTestsWorkflowTests.cs:324

• This second PowerShell-backed theory also omits RequiresTools(["pwsh"]), unlike the neighboring cases at lines 270-272 and 348-350. Add the tool requirement so unsupported environments skip consistently instead of failing to start pwsh.

    [Theory]
    [InlineData(3)]
    [InlineData(7)]

tests/Infrastructure.Tests/WorkflowScripts/RunTestsWorkflowTests.cs:297

• This theory launches pwsh but lacks the RequiresTools(["pwsh"]) guard used by every other PowerShell-backed test in this file (for example, lines 270-272). Without PowerShell installed, the tests intended to skip instead fail while the surrounding cases skip.

This issue also appears on line 322 of the same file.

    [Theory]
    [InlineData(3)]
    [InlineData(7)]

• Files reviewed: 15/15 changed files
• Comments generated: 1
• Review effort level: Balanced

[Adam Ratzman (adamint)]

Fixed in 59140d2304. Confirmed the hole: with ignoreTestFailures, exit code 1 (unknown error) or 9 (minimum execution policy violated) matched neither $fatalInfraCodes nor $toleratedTestOutcomeExitCodes, so it fell through and the job went green whenever a parseable .trx existed.

Quarantine mode now fails closed on any nonzero code outside the tolerated allow-list (2, 3, 7, 13), so an unrecognized code can no longer be silently tolerated. The fatal-infrastructure check is retained so normal CI keeps its existing behavior.

Regression: TestResultValidationFailsIgnoredRunWithUntoleratedExitCode for codes 1 and 9, with a fully valid .trx present so it isolates the exit-code path. Verified both fail against the previous workflow and pass with the change; full RunTestsWorkflowTests is 20/20.

[Copilot]

Review details

Suppressed comments (1)

.github/actions/enumerate-tests/action.yml:46

• This output-contract comment says every class entry tolerates zero tests, but expand-test-matrix-github.ps1:111-115 explicitly changes allowZeroTests to false for Linux rows. Document that discovery-OS exception here; otherwise callers are told the opposite of the emitted matrix behavior.

#   - allowZeroTests: (class type only) True. Class shards are generated mechanically from the
#       discovered class names, so a class that is real on one OS can be legitimately empty on
#       another once CI applies its environment trait filters (for example a class whose tests all
#       require containers, filtered out on Windows). Discovering zero tests is therefore not a
#       failure for these entries, unlike regular and collection shards.

• Files reviewed: 15/15 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

[Copilot]

Review details

Suppressed comments (6)

.github/workflows/run-tests.yml:736

• This Write-Error is also inside the XML parsing try. GitHub Actions sets $ErrorActionPreference = 'Stop', so a negative counter jumps to the catch and is ignored when quarantine mode has a tolerated exit code. Keep the diagnostic non-terminating so negative counters always reach exit 1.

                if ($testCount -lt 0) {
                  Write-Error "  $($trxFile.Name) has a negative ResultSummary/Counters/@total value '$testCount'."
                  exit 1

tests/Infrastructure.Tests/WorkflowScripts/RunTestsWorkflowTests.cs:281

• This PowerShell-backed theory is missing RequiresTools(["pwsh"]), so the test fails rather than skips when pwsh is unavailable.

    [Theory]

tests/Infrastructure.Tests/WorkflowScripts/RunTestsWorkflowTests.cs:307

• Add the same pwsh tool requirement as the surrounding process-based tests; otherwise this theory fails on hosts where PowerShell is not installed.

    [Theory]

.github/workflows/run-tests.yml:732

• In GitHub Actions, the generated pwsh script runs with $ErrorActionPreference = 'Stop', so this Write-Error throws into the surrounding catch before exit 1 executes. For quarantine runs with exit code 2/3/7/13, that catch treats the error as an ignorable incomplete TRX, allowing an unparseable counter to pass even though this branch is intended to fail closed. Force this diagnostic to remain non-terminating so the explicit exit is reached.

This issue also appears on line 734 of the same file.

                if (-not [int]::TryParse([string]$countersNode.total, [ref]$testCount)) {
                  Write-Error "  $($trxFile.Name) has an unparseable ResultSummary/Counters/@total value '$($countersNode.total)'."
                  exit 1

tests/Infrastructure.Tests/WorkflowScripts/RunTestsWorkflowTests.cs:256

• This theory launches pwsh but lacks the RequiresTools guard used by the other PowerShell tests in this file. On environments without PowerShell, it will fail during process startup instead of being skipped.

This issue also appears in the following locations of the same file:

• line 281
• line 307

    [Theory]

.github/actions/enumerate-tests/action.yml:46

• This output documentation says every class row has allowZeroTests: true, but expand-test-matrix-github.ps1 explicitly changes the Linux discovery row to false. Document the per-OS behavior so consumers do not assume the zero-test opt-out applies to the discovery OS.

#   - allowZeroTests: (class type only) True. Class shards are generated mechanically from the
#       discovered class names, so a class that is real on one OS can be legitimately empty on
#       another once CI applies its environment trait filters (for example a class whose tests all
#       require containers, filtered out on Windows). Discovering zero tests is therefore not a
#       failure for these entries, unlike regular and collection shards.

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

[Copilot]

Review details

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.