Make vacuous CI guards capable of failing

[Adam Ratzman (adamint)]

Description

Five CI guards in this repo could not fail. Each is individually plausible, which is why they survived review, and each reported success on input it never actually examined.

Verify test results exist only ran when failures were already being ignored. Its condition was if: ${{ inputs.ignoreTestFailures }}, so on every normal CI run — the runs whose results we act on — the step was skipped entirely. It now runs whenever a test step actually executed.

A shard reporting zero tests passed. .trx files were counted but the tests inside them were not, so a run that executed nothing was indistinguishable from a run that executed everything and passed. The step now sums Counters/@total across the shard's .trx files and fails when the sum is zero. An allowZeroTests input exists for generated shards that are legitimately empty, and it defaults to false so the failure is opt-out rather than opt-in. The success line now prints the test count as well as the file count.

The hang dump detectors matched a filename MTP never writes. Both the classification path and the final reporting path globbed *hangdump*, but MTP writes <process>_<pid>_hang.dmp. The pattern never matched, so the step printed ✓ No hang dump files detected seconds after dumps were written to the directory it was searching. Both now match *_hang.dmp, which is deliberately narrow enough to keep crash dumps — which can appear during cleanup on a fully passing run — out of a timeout-only classification.

The transient-rerun classifier could rerun a real hang. Check for hang dump files was not in testExecutionFailureStepPatterns, so an unrelated feed blip elsewhere in the log was enough to retry a job that had genuinely timed out. It is now classified as a test-execution failure.

The lockfile registry guard passed on an empty lockfile. validate-lockfile-registry.cjs filtered yarn.lock for resolved entries and then checked that none pointed outside the internal feed. With zero resolved entries the filter is empty, the check is vacuous, and the script exits 0 — including when the file is deleted. It now requires at least one resolved entry before validating them, and the E2E lockfile test asserts the same precondition.

The shared shape is asserting over a filtered collection without first asserting the collection is non-empty. That is worth naming, because it will recur.

Related: the E2E workflow guard now compares the full expected matrix rows against the committed src/test-e2e/*.e2e.test.ts specs, so deleting a shard row fails immediately instead of silently dropping coverage.

No issue.

Testing

Every guard was tested by deleting or emptying the input it is supposed to protect and confirming it now fails. A guard fix that cannot itself fail would be the joke writing itself.

• Red: deleted both edge-cases rows from .github/workflows/extension-e2e-tests.yml; runs every E2E spec from the workflow matrix failed on the missing Linux/Windows rows.
• Red: deleted all 955 resolved lines from extension/yarn.lock; installs the E2E runner dependencies from the internal npm feed failed with Expected extension/yarn.lock to contain resolved registry entries.
• Red: with those same 955 lines deleted, node scripts/validate-lockfile-registry.cjs exited 1 with extension/yarn.lock does not contain any resolved entries.
• Red: with the old *hangdump* detector, RunTestsWorkflowTests.HangDumpDetectorsMatchMtpHangDumpFilesAndIgnoreOtherDumps matched only not-a-dump-hangdump.txt and missed dotnet_6079_hang.dmp and docker_6110_hang.dmp.
• Red: with the old rerun classifier, DoesNotApplyBroadNetworkOverrideWhenHangDumpDetectionFailed retried Check for hang dump files when the log also contained a dnceng feed error.
• Red: TestResultValidationFailsWhenTrxFilesContainNoTests saw exit 0 against a .trx reporting total: 0.
• Green: restored files; targeted E2E guard tests passed (3 passing); node scripts/validate-lockfile-registry.cjs exited 0; corepack yarn lint passed.
• Green: restored detector finds docker_6110_hang.dmp and dotnet_6079_hang.dmp, and an empty results directory still reports clean (empty_count=0).
• Green: RunTestsWorkflowTests passes 5/5, including the zero-test case failing validation and a total="3" case passing; AutoRerunTransientCiFailuresTests passed (100 succeeded); node --check .github/workflows/auto-rerun-transient-ci-failures.js 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
• Does the change require an update in our Aspire docs?
  • 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

[Copilot]

Review details

Suppressed comments (2)

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

• allowZeroTests also accepts a count-less or partially written TRX. A file containing only a valid <TestRun> increments validFileCount, leaves totalTestCount at zero, and then reaches the opt-out branch as though it were a legitimate empty shard. Only treat a TRX as valid for this guard after its Counters/@total value has been found and parsed, so the opt-out cannot hide truncated result files.

                $countersNode = $trxContent.TestRun.ResultSummary.Counters

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

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

• This only checks the physical line containing IgnoreZeroTestResult; a multiline Condition attribute or a condition on the containing PropertyGroup can make the setting conditional while this test remains green. Since the blanket workflow opt-out depends on every generated row receiving this property, parse the targets XML and verify the command element and its relevant ancestors cannot conditionally omit it.

        int commandLineIndex = Array.FindIndex(lines, line => line.Contains("/p:IgnoreZeroTestResult=true", StringComparison.Ordinal));
        Assert.True(commandLineIndex >= 0, $"Could not find the /p:IgnoreZeroTestResult=true line in {s_specializedTestRunsheetBuilderTargetsPath}.");
        Assert.DoesNotContain("Condition", lines[commandLineIndex], StringComparison.Ordinal);

• 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.

[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.