Unify quarantine and outerloop CI with the main test pipeline#16448
Unify quarantine and outerloop CI with the main test pipeline#16448radical wants to merge 17 commits into
Conversation
Replace the older bespoke test matrix pipeline used by quarantine and outerloop workflows with the same metadata-driven pipeline as tests.yml: - TestEnumerationRunsheetBuilder generates .tests-metadata.json per project (deterministic, no assembly execution) - build-test-matrix.ps1 merges into canonical platform-agnostic matrix - expand-test-matrix-github.ps1 expands to per-OS entries - split-test-matrix-by-deps.ps1 splits into 6 dependency-based buckets Key changes: - specialized-test-runner.yml: Major rewrite to inline enumerate-tests steps, derive MSBuild props from attributeName, use 6 split test jobs with proper empty-bucket handling in results gate - split-test-projects-for-ci.ps1: Add -IncludeTraitFilter parameter for positive trait filtering in class discovery (enables finding classes that ONLY contain quarantined/outerloop tests) - tests/Directory.Build.targets: Wire DiscoveryTraitFilter MSBuild property to the -IncludeTraitFilter script parameter - Caller workflows simplified (removed testRunnerName, extraRunSheetBuilderArgs) - Delete old infrastructure: SpecializedTestRunsheetBuilderBase.targets, QuarantinedTestRunsheetBuilder/, OuterloopTestRunsheetBuilder/, _GenerateTestMatrix target from AfterSolutionBuild.targets Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move the 6 dependency-split test job definitions + results gate into
a new reusable workflow (run-tests-core.yml) called by both tests.yml
and specialized-test-runner.yml. This eliminates duplication of the
test bucket pattern across the two pipelines.
Changes:
- Create .github/workflows/run-tests-core.yml with 6 test bucket
jobs and an internal results gate that handles empty-bucket skips
- Update specialized-test-runner.yml to call run-tests-core.yml
instead of defining 6 inline test jobs
- Update tests.yml to call run-tests-core.yml instead of defining
6 inline test jobs; simplify its results gate accordingly
- Add run-tests-core.yml to path triggers in quarantine/outerloop
caller workflows
Trade-off: in tests.yml, tests_no_nugets now waits for builds to
complete (~2 min regression) since all buckets share a single
reusable workflow call with unified dependencies. This was accepted
for the maintenance win of having a single test job definition.
Nesting depth: 4 levels (max allowed by GitHub Actions):
ci.yml -> tests.yml -> run-tests-core.yml -> run-tests.yml
tests-quarantine.yml -> specialized-test-runner.yml ->
run-tests-core.yml -> run-tests.yml
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-ci.ps1 Test the new IncludeTraitFilter parameter added to split-test-projects-for-ci.ps1: - IncludeTraitFilterGracefullySkipsWhenNoClassesFound: verifies that when IncludeTraitFilter is set and no matching classes are found, the script succeeds and writes valid JSON with empty testPartitions (graceful skip) - WithoutIncludeTraitFilterFailsWhenNoClassesFound: verifies the existing behavior that zero discovered classes without IncludeTraitFilter is an error Also updates the RunScript helper to accept the optional includeTraitFilter parameter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16448Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16448" |
…kage GitHub Actions' pwsh wrapper appends 'exit $LASTEXITCODE' after the user script. When ignoreTestFailures is true, the Windows test steps skipped the explicit exit but left $LASTEXITCODE set to the non-zero dotnet test exit code, causing the step to fail despite the intent to ignore failures. Also add always() to the 'Verify test results exist' step condition so it runs even if a prior step failed unexpectedly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The TestEnumerationRunsheetBuilder.targets has explicit skip gates for Aspire.Templates.Tests and Aspire.Cli.EndToEnd.Tests that require these props to be set. Without them, these projects are excluded from the canonical matrix even when the BeforeBuildProps lists them. Projects with their own SkipTests conditions (e.g., Templates skips quarantine runs) will still be correctly excluded by the downstream SkipTests check in the runsheet builder. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The splitting is an implementation detail; the parent workflows provide the context of what kind of tests are running. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
|
There was a problem hiding this comment.
Pull request overview
This PR migrates the quarantine/outerloop “specialized test runner” off the legacy MSBuild runsheet generation and onto the same metadata-driven test matrix pipeline used by the main CI, while extracting a shared reusable workflow to eliminate duplicated test-bucket job definitions.
Changes:
- Added a new reusable workflow (
run-tests-core.yml) that executes the 6 dependency-based test buckets plus a results gate. - Refactored
specialized-test-runner.ymlto generate/split matrices via the canonical pipeline and then callrun-tests-core.yml(including a new positive trait discovery mode for class splitting). - Simplified
tests.ymlto callrun-tests-core.yml, and removed legacy MSBuild runsheet targets/logic.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Infrastructure.Tests/PowerShellScripts/SplitTestProjectsTests.cs | Adds tests for new IncludeTraitFilter “graceful skip” behavior. |
| tests/Directory.Build.targets | Plumbs DiscoveryTraitFilter through to the split script as -IncludeTraitFilter. |
| eng/scripts/split-test-projects-for-ci.ps1 | Adds IncludeTraitFilter support and “skip instead of fail” behavior when no matching classes are found. |
| eng/SpecializedTestRunsheetBuilderBase.targets | Deletes legacy specialized runsheet generator base targets. |
| eng/QuarantinedTestRunsheetBuilder/QuarantinedTestRunsheetBuilder.targets | Deletes quarantined runsheet builder targets. |
| eng/OuterloopTestRunsheetBuilder/OuterloopTestRunsheetBuilder.targets | Deletes outerloop runsheet builder targets. |
| eng/AfterSolutionBuild.targets | Removes legacy combined runsheet generation target (keeps canonical matrix generation). |
| .github/workflows/tests.yml | Replaces 6 inline bucket jobs with a single call to run-tests-core.yml. |
| .github/workflows/tests-quarantine.yml | Updates paths: triggers and switches to new specialized runner inputs. |
| .github/workflows/tests-outerloop.yml | Updates paths: triggers and switches to new specialized runner inputs. |
| .github/workflows/specialized-test-runner.yml | Rebuilds specialized test execution around canonical matrices + run-tests-core.yml. |
| .github/workflows/run-tests.yml | Ensures ignoreTestFailures exits 0 reliably and always verifies TRX presence when enabled. |
| .github/workflows/run-tests-core.yml | New shared workflow implementing the 6 bucket jobs + results gate. |
Comments suppressed due to low confidence (1)
eng/scripts/split-test-projects-for-ci.ps1:50
- The script header still states it "Fails fast if zero test classes discovered when in class mode", but with IncludeTraitFilter set the script now intentionally succeeds and writes an empty partitions file. Update the .NOTES (and/or description) to reflect the conditional behavior so callers understand when zero classes is an error vs a skip.
.NOTES
PowerShell 7+
Fails fast if ExtractTestPartitions cannot be built or run.
Fails fast if zero test classes discovered when in class mode.
Only runs --list-tests when no partitions are found in the assembly.
Prevents fork runs from failing on empty matrix inputs when generate_tests_matrix is skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…st-runner Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JamesNK
left a comment
There was a problem hiding this comment.
Review: 2 issues flagged (1 correctness, 1 CI infrastructure). Both relate to the unification trade-offs — one about cross-OS build failure isolation regression, one about the auto-rerun script not accounting for the new results-gate job name.
| run_tests: | ||
| needs: | ||
| - setup_for_tests | ||
| - build_packages | ||
| - build_cli_archive_linux | ||
| - build_cli_archive_linux_arm64 | ||
| - build_cli_archive_windows | ||
| - build_cli_archive_windows_arm64 | ||
| - build_cli_archive_macos | ||
| if: ${{ !cancelled() && !failure() }} |
There was a problem hiding this comment.
The unified run_tests job depends on all CLI archive build jobs with if: ${{ !cancelled() && !failure() }}. The !failure() condition means if any needed job fails (e.g., build_cli_archive_macos), all 6 test buckets across all platforms are blocked — including tests_no_nugets which never needed build artifacts.
Previously, each test bucket had isolated dependencies:
tests_no_nugetsdepended only onsetup_for_tests(never blocked by builds)tests_requires_nugets_linuxdepended onbuild_packages+build_cli_archive_linuxonly
This contradicts the existing comment at line 67:
Split by OS so that test jobs can depend on just their platform's archive, allowing Linux tests to start as soon as the Linux archive completes without waiting for the slower Windows/macOS builds.
The specialized-test-runner.yml uses a more permissive condition that doesn't check build_cli_archives result, but here !failure() fails if any needed job fails. A single macOS infra issue would block all Linux and Windows tests too.
There was a problem hiding this comment.
[automated] This was fixed in eb6bf7e; the reusable workflow now still starts when setup succeeds, and bucket-level gating handles missing artifacts instead of suppressing all test buckets.
| - tests_requires_cli_archive | ||
| steps: | ||
| - name: Fail if any test job failed or was unexpectedly skipped | ||
| if: >- | ||
| ${{ always() && | ||
| (contains(needs.*.result, 'failure') || | ||
| contains(needs.*.result, 'cancelled') || | ||
| (fromJson(inputs.tests_matrix_no_nugets).include[0] != null && | ||
| needs.tests_no_nugets.result == 'skipped') || | ||
| (fromJson(inputs.tests_matrix_no_nugets_overflow).include[0] != null && | ||
| needs.tests_no_nugets_overflow.result == 'skipped') || | ||
| (fromJson(inputs.tests_matrix_requires_nugets_linux).include[0] != null && | ||
| needs.tests_requires_nugets_linux.result == 'skipped') || | ||
| (fromJson(inputs.tests_matrix_requires_nugets_windows).include[0] != null && | ||
| needs.tests_requires_nugets_windows.result == 'skipped') || | ||
| (fromJson(inputs.tests_matrix_requires_nugets_macos).include[0] != null && | ||
| needs.tests_requires_nugets_macos.result == 'skipped') || | ||
| (fromJson(inputs.tests_matrix_requires_cli_archive).include[0] != null && | ||
| needs.tests_requires_cli_archive.result == 'skipped')) }} | ||
| run: | | ||
| echo "One or more test jobs failed or were unexpectedly skipped." | ||
| exit 1 |
There was a problem hiding this comment.
The new results gate job appears as Test Results (or run_tests / Test Results in the nested workflow context). The auto-rerun transient CI failures script in auto-rerun-transient-ci-failures.js has an ignoredJobs set that currently includes 'Final Results' and 'Tests / Final Test Results' but not this new gate job name. When a test job fails, the gate also fails, which may consume a slot in the maxRetryableJobs budget (default 5) instead of being ignored as a derived failure.
Per AGENTS.md: "When changing CI workflow job or step names that are referenced by automation or tests, update the corresponding workflow helpers."
Worth checking whether the new job name needs to be added to ignoredJobs.
There was a problem hiding this comment.
[automated] Fixed in eb6bf7e: added Tests / run_tests / Test Results to the transient rerun aggregator ignore list, with matching workflow-script test coverage and docs updates.
Resolve workflow conflicts by keeping the shared run-tests-core.yml test execution path while preserving origin/main's new CLI E2E image, macOS x64 archive, installer artifact, TypeScript API compatibility, and final-gate wiring. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The shared run_tests job now waits for all artifact producers, but it should not require every producer to succeed before starting. A failed archive or image build already fails the final results gate, while independent test buckets can still run and provide useful diagnostics. Gate run_tests on successful matrix generation instead of the aggregate needs failure state so no-nugets and other unaffected buckets still execute after unrelated artifact producer failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve workflow conflicts by keeping the shared run-tests-core.yml test execution path while preserving origin/main's newer selective CI gates, Nix package validation, installer artifact gates, and MTP exit-code classification for quarantined runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The new IncludeTraitFilter tests used _tempDir, but this test class only owns the TemporaryWorkspace stored in _workspace. That left the targeted Infrastructure.Tests project unable to compile after the merge. Use the existing workspace for the mock assembly and output paths so the new tests exercise the split-test-projects-for-ci.ps1 IncludeTraitFilter behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The specialized test matrix probes projects with a positive trait filter. When a project has no matching tests, Microsoft.Testing.Platform can exit with code 8 before the script reaches its empty-partition handling. Treat exit code 8 as expected only for positive trait discovery, while keeping other list-test failures fatal. Update the IncludeTraitFilter test so it exercises the non-zero no-tests path instead of an exit-0 echo path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
The shared test workflow adds a new `Tests / run_tests / Test Results` aggregator job in CI. The transient rerun classifier ignored the old aggregator job names, but not the new reusable-workflow gate, so a derived gate failure could count against the retryable job budget. Add the new CI gate name to the ignored job set, update the matching workflow-script test and docs, and clarify that specialized positive trait discovery writes an empty partitions file when no matching test classes are found. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| # NOTE: This workflow is at nesting level 3 (caller → this → run-tests.yml). | ||
| # GitHub Actions allows max 4 levels. Do NOT add reusable workflow calls inside | ||
| # run-tests.yml or this chain will break. |
The old specialized runsheet builder skipped Playground tests because they still need setup that is not wired through the specialized workflows. The new metadata-driven project discovery was including Aspire.Playground.Tests, which changed Outerloop coverage from only splitting existing projects to adding a new effective project. Filter Playground out when generating the specialized BeforeBuildProps file so Outerloop and Quarantine preserve the old project set while still using the new partitioned test matrix for supported projects. Refs microsoft#18710 Refs microsoft#18711 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| rel_path="${file#$REPO_ROOT/}" | ||
| echo "$rel_path" | cut -d'/' -f1-2 | ||
| done \ | ||
| | grep -v '^tests/Aspire\.Playground\.Tests$' \ |
| ${{ steps.derive_props.outputs.msbuild_prop }} | ||
| /p:DiscoveryTraitFilter=${{ steps.derive_props.outputs.discovery_filter }} |
| paths: | ||
| - '.github/workflows/tests-quarantine.yml' | ||
| - '.github/workflows/specialized-test-runner.yml' | ||
| - '.github/workflows/run-tests-core.yml' | ||
| - '.github/workflows/run-tests.yml' | ||
| - '.github/actions/enumerate-tests/**' | ||
| - '.github/workflows/build-cli-e2e-image.yml' |
| paths: | ||
| - '.github/workflows/tests-outerloop.yml' | ||
| - '.github/workflows/specialized-test-runner.yml' | ||
| - '.github/workflows/run-tests-core.yml' | ||
| - '.github/workflows/run-tests.yml' | ||
| - '.github/actions/enumerate-tests/**' | ||
| - '.github/workflows/build-cli-e2e-image.yml' |
| // Arrange - assembly with no partition attributes → falls to class mode. | ||
| // The fake list-tests command exits with MTP's no-tests exit code. With | ||
| // IncludeTraitFilter set, the script should succeed gracefully instead of | ||
| // erroring before it can write the empty partitions file. |
Outerloop and quarantine shard entries previously relied on the workflow-level specialized trait filter plus per-shard filters generated by the matrix. xUnit/MTP treats repeated positive trait filters as OR, so a shard such as outerloop plus Partition=1 selected normal Partition 1 tests too. Generate the specialized trait filter into each matrix entry instead. Regular and class entries use simple filters where categories intersect, while partitioned entries use an xUnit query that intersects the specialized trait, category!=failing, and the partition trait. Suppress the default failing-test simple filter when a query filter is present so MTP does not mix filter syntaxes. Add infrastructure coverage and CI docs for the specialized-filter contract. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| rel_path="${file#$REPO_ROOT/}" | ||
| echo "$rel_path" | cut -d'/' -f1-2 | ||
| done \ | ||
| | grep -v '^tests/Aspire\.Playground\.Tests$' \ |
| /p:GenerateCIPartitions=true | ||
| /p:InstallBrowsersForPlaywright=false | ||
| /p:BeforeBuildPropsPath=${{ github.workspace }}/artifacts/BeforeBuildProps.props | ||
| /bl:${{ github.workspace }}/artifacts/log/Debug/runsheet.binlog | ||
| /p:IncludeTemplateTests=true | ||
| /p:IncludeCliE2ETests=true | ||
| ${{ steps.derive_props.outputs.msbuild_prop }} | ||
| /p:DiscoveryTraitFilter=${{ steps.derive_props.outputs.discovery_filter }} | ||
| /p:SpecializedTraitFilter=${{ steps.derive_props.outputs.discovery_filter }} |
|
|
||
| Each job invokes `.github/workflows/run-tests.yml` with matrix parameters including `extraTestArgs` for filtering (e.g., `--filter-trait "Partition=X"`). | ||
|
|
||
| Specialized workflows (`tests-outerloop.yml` and `tests-quarantine.yml`) generate their specialized trait filter into each matrix entry instead of appending one global filter at execution time. Regular and class-split entries use simple filters such as `--filter-trait "outerloop=true"` and `--filter-class "..." --filter-trait "outerloop=true"`. Partition-split entries use `--filter-query` so the specialized trait and `Partition` trait are intersected in one xUnit query filter; repeated positive `--filter-trait` arguments are ORed by xUnit.net and would broaden the run. |
Specialized outerloop and quarantine shards now use xUnit query filters when a shard needs trait intersection. On Windows those queries include cmd metacharacters such as `&` and `!`, and the repo-local dotnet.cmd shim reparses `%*` through cmd.exe before the arguments reach MTP. The Windows test step now reads the restored SDK path from artifacts/toolset/sdk.txt and invokes dotnet.exe directly for test execution and heartbeat startup. That preserves the query filter as a PowerShell argument instead of letting cmd.exe reinterpret it. Add a workflow guard so this path does not regress back to dotnet.cmd for the Windows test command. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| # Reusable workflow that runs test jobs split into 6 dependency-based buckets. | ||
| # | ||
| # Shared by tests.yml and specialized-test-runner.yml to avoid duplicating the | ||
| # 6 test job definitions. The calling workflow is responsible for: |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Description
Migrates the specialized test runner (quarantine/outerloop) from the old MSBuild-based runsheet generation to the new metadata-driven pipeline, and extracts a shared
run-tests-core.ymlworkflow to eliminate duplicated test job definitions.Key changes
New
run-tests-core.yml— reusable workflow containing the 6 dependency-based test bucket jobs + results gate. Shared by bothtests.ymlandspecialized-test-runner.ymlto avoid duplicating job definitions.Refactored
specialized-test-runner.yml— now uses the same pipeline astests.yml:TestEnumerationRunsheetBuilder→build-test-matrix→expand-test-matrix-github→split-test-matrix-by-deps→run-tests-core. Added grep-based project scoping and positive trait filtering (DiscoveryTraitFilter) to build a lean matrix of only relevant test classes.Simplified
tests.yml— replaced 6 inline test job definitions with a single call torun-tests-core.yml.Updated
tests-outerloop.yml/tests-quarantine.yml— addedrun-tests-core.ymltopaths:triggers so workflow changes are validated on PRs.Deleted old MSBuild targets — removed
SpecializedTestRunsheetBuilderBase.targets,OuterloopTestRunsheetBuilder.targets,QuarantinedTestRunsheetBuilder.targets, and cleared the old runsheet generation fromAfterSolutionBuild.targets.split-test-projects-for-ci.ps1— addedIncludeTraitFilterparameter for positive trait filtering in class discovery mode. When set and zero classes are found, the script gracefully skips instead of erroring.tests/Directory.Build.targets— passesDiscoveryTraitFilterthrough to the split script as-IncludeTraitFilter.Tests added
IncludeTraitFilterGracefullySkipsWhenNoClassesFound— verifies graceful skip with empty output JSONWithoutIncludeTraitFilterFailsWhenNoClassesFound— verifies error behavior is preserved without the filterChecklist