Assert every VS Code extension E2E spec has a CI matrix row by adamint · Pull Request #19146 · microsoft/aspire

Adam Ratzman (adamint) · 2026-08-07T18:13:06Z

Description

Adds a unit test asserting that the set of VS Code extension E2E spec files and the set of spec: entries in the extension-e2e-tests.yml CI matrix are identical, in both directions.

Today a new src/test-e2e/*.e2e.test.ts file with no matching matrix row is silently never run, and a matrix row pointing at a spec that was renamed or deleted is a shard that runs nothing. Neither produces any signal. This test enumerates the spec directory, parses the spec: values out of the workflow, and asserts the difference is empty each way.

It is a unit test, so it runs on every PR through the existing extension unit test job rather than through the E2E workflow.

Split out of #19133, where it was introduced alongside the harness it validates.

Validation

From extension/:

yarn run compile-tests   clean
yarn run lint            clean
yarn run compile         webpack succeeded
yarn run unit-test       1455 passing, 4 pending, 0 failing

Negative-tested by adding a throwaway zzUnmatched.e2e.test.ts with no matrix row: the run produced 1 failing test naming that file. The throwaway file was then removed.

Checklist

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

github-actions · 2026-08-07T18:13:26Z

🚀 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 -- 19146

Or

Run remotely in PowerShell:

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

Copilot

Pull request overview

Adds unit coverage to keep VS Code E2E specs synchronized with CI matrix entries.

Changes:

Enumerates E2E spec files and workflow matrix paths.
Checks for missing and stale matrix entries.

Show a summary per file

File	Description
`extension/src/test/e2eShardMatrix.test.ts`	Adds E2E shard-matrix consistency tests.

Review details

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

github-actions · 2026-08-07T18:48:22Z

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 · 2026-08-07T18:55:09Z

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 · 2026-08-07T19:02:44Z

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 (1)

extension/src/test/e2eShardMatrix.test.ts:28

readdirSync(specDirectory) only scans immediate children, while both tsconfig.e2e.json and .mocharc.e2e.js use recursive ** patterns. A valid spec under a subdirectory would therefore compile and match the E2E runner, but be absent from this canonical set, allowing it to remain unsharded without this test failing. Enumerate recursively and normalize separators for Windows.

        return fs.readdirSync(specDirectory)
            .filter(file => file.endsWith('.e2e.test.ts'))
            .map(file => `out/test-e2e/test-e2e/${file.replace(/\.ts$/, '.js')}`)

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

The E2E suite is sharded one spec per matrix row in `extension-e2e-tests.yml`, and `run-e2e.js` is pointed at a single compiled spec through `ASPIRE_EXTENSION_E2E_SPEC`. Nothing enumerates `src/test-e2e` at runtime, so a spec that never gets a matrix row does not fail - it never runs, and the workflow stays green while the coverage it was written for is silently gone. The reverse holds too: a renamed or deleted spec leaves a row whose glob matches nothing. Both directions are now asserted as a set difference in each direction. This is deliberately a unit test rather than part of the E2E suite. The failure it detects is "an E2E spec did not run", which a suite that did not run cannot report on itself, and it means the check keeps working while `extension_e2e_tests` is disabled in `tests.yml`. Verified by adding a spec file with no matrix row: the first test fails and names the orphaned spec. Both tests pass as-is on main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The reverse check asserted that every matrix `spec:` value mapped to some existing TypeScript file under `src/test-e2e`, which is not the claimed set equality. `out/test-e2e/test-e2e/helpers/fixtures.js` resolves to `src/test-e2e/helpers/fixtures.ts`, so a row pointing at a helper passed - and that row is a shard whose runner glob matches no tests, so it reports success while covering nothing. Both directions are now one `deepStrictEqual` between the deduplicated, sorted matrix values and the compiled paths derived from the `.e2e.test.ts` files, so a matrix value is correct only if it is a spec. Verified by injecting the helper row: the old pair of tests both passed, the new one fails with `+ 'out/test-e2e/test-e2e/helpers/fixtures.js'`. Dropping the `azureFunctions` rows fails it in the other direction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Review details

Suppressed comments (1)

extension/src/test/e2eShardMatrix.test.ts:55

This comment no longer matches the runner: nonexistent/renamed paths are already rejected by the workflow's Test-Path check and assertSpecMatches, rather than producing a green zero-test shard. The reverse comparison still matters for existing non-spec files such as helpers, so please describe that remaining blind spot accurately.

        // helper module, or a spec that was renamed or deleted) means the runner's glob matches
        // nothing and the shard reports success while running zero tests. Both are invisible in a
        // green workflow, so the matrix has to equal the spec set exactly.

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

The set equality comparison was correct but only ever ran against the real workflow and the real spec directory, so nothing demonstrated that it closes the hole the previous existence-based reverse check left open. Verified directly: a row of `out/test-e2e/test-e2e/helpers/fixtures.js` maps to `src/test-e2e/helpers/fixtures.ts`, which exists, so the old check returned no offenders and accepted a shard that runs zero tests. Take the workflow text and the spec file names as parameters so the comparison can be exercised against synthetic matrices, and add the three cases that pin the behaviour: a row pointing at a real helper is rejected, a spec with no row is rejected, and one spec on several platform rows is still accepted. The helper case asserts the file it names really exists, so it cannot decay into a vacuous test if that helper is ever renamed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Review details

Suppressed comments (1)

extension/src/test/e2eShardMatrix.test.ts:63

readdirSync(specDirectory) only examines immediate children, so a nested spec such as src/test-e2e/debug/node.e2e.test.ts is omitted from the canonical set and can still be left without a matrix row. This is a supported layout: tsconfig.e2e.json includes src/test-e2e/**/*.ts, and the E2E runner uses out/test-e2e/**/*.e2e.test.js. Recursively enumerate specs and preserve each relative subdirectory when deriving the compiled path.

        const specFiles = canonicalSpecPaths(fs.readdirSync(specDirectory));

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

The two negative cases asserted `notDeepStrictEqual` between `matrixSpecPaths` and `canonicalSpecPaths` directly. That compares two helper outputs and never reaches the production comparison, which was inlined in the real-workflow test, so both cases passed no matter what the check did - the commit that added them did not prove what it claimed. The comparison moves into `assertMatrixMatchesSpecs`, which all four cases now call, and the negative cases become `assert.throws(..., assert.AssertionError)` so they fail when the check stops rejecting their input. Verified by weakening `assertMatrixMatchesSpecs` in both directions. Reverse inclusion only (the pre-fix check): `rejects a matrix row pointing at a real file that is not an E2E spec` fails with `Missing expected exception (AssertionError)`. Forward inclusion only: `rejects a spec that has no matrix row` fails the same way. Before this change both weakenings left all four tests green. 1457 unit tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Review details

Suppressed comments (1)

extension/src/test/e2eShardMatrix.test.ts:85

This only enumerates direct children of src/test-e2e. Both the E2E compiler (tsconfig.e2e.json:12) and Mocha (.mocharc.e2e.js:24) include specs recursively, so a valid nested subdir/foo.e2e.test.ts would be omitted from the canonical set and could still have no CI matrix row. Enumerate recursively and normalize path separators for the workflow paths.

        const specFileNames = fs.readdirSync(specDirectory);

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

github-actions · 2026-08-07T22:34:05Z

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

Adam Ratzman (adamint) requested a review from Ella Hathaway (ellahathaway) as a code owner August 7, 2026 18:13

Copilot AI balanced review requested due to automatic review settings August 7, 2026 18:13

github-actions Bot added the area-vscode-extension label Aug 7, 2026

Copilot started reviewing on behalf of Adam Ratzman (adamint) August 7, 2026 18:14 View session