Add the concurrency option to node tests - #64379
Conversation
typescript-automation
Bot
added
Author: Team
For Uncommitted Bug
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Concurrent execution changes ordering across many large API suites and warrants final human validation for platform-specific races.
Review effort: Balanced
Findings: None
What changed in this PR
Adds selective concurrency to Node.js test suites to accelerate filtered test runs while avoiding full API-suite slowdown.
Changes:
- Enables concurrency for filtered TypeScript API tests.
- Runs suitable VS Code extension tests concurrently.
- Groups standalone tests into concurrent suites.
| File | Description |
|---|---|
packages/vscode-typescript/test/tsdkPackage.test.ts |
Runs isolated package-resolution tests concurrently. |
packages/vscode-typescript/test/contentMapperContributions.test.ts |
Runs contribution tests concurrently. |
packages/typescript/test/wtf8.test.ts |
Adds conditional concurrency. |
packages/typescript/test/sync/astnav.test.ts |
Adds conditional concurrency. |
packages/typescript/test/sync/ast.test.ts |
Adds conditional concurrency across AST suites. |
packages/typescript/test/sync/api.test.ts |
Adds conditional concurrency across synchronous API suites. |
packages/typescript/test/sync/api-generators.test.ts |
Adds conditional generator-test concurrency. |
packages/typescript/test/spanMap.test.ts |
Adds conditional concurrency. |
packages/typescript/test/encoder.test.ts |
Adds conditional encoder-test concurrency. |
packages/typescript/test/diagnosticFormatter.test.ts |
Adds conditional formatter-test concurrency. |
packages/typescript/test/async/astnav.test.ts |
Adds conditional concurrency. |
packages/typescript/test/async/api.test.ts |
Adds conditional concurrency across asynchronous API suites. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| } from "../src/api/node/protocol.ts"; | ||
| import { Wtf8Decoder } from "../src/api/node/wtf8.ts"; | ||
|
|
||
| const concurrency = process.execArgv.some(arg => arg === "--test-name-pattern" || arg.startsWith("--test-name-pattern=")); |
There was a problem hiding this comment.
On one hand, I'm thinking "we should yoink this into a helper, or describeConcurrent", on the other, "a little copying is better than a little dependency"
There was a problem hiding this comment.
Yeah, I thought the same, but since the tests are within a few different packages (npm package, vscode extension), either I'd need a common test utils package (....eh?) or at least 2 duplicates, and neither actually has a dedicated test utils file either, so it's a new file in both, too. It's simple enough, so I just shrugged and left it at once per-file it was actually used in.
There was a problem hiding this comment.
Actually, because the slowdown-when-fully-concurrent was limited to the API, this is only used in the API in the end, so I guess it's just one test util file 🤷♂️
...When it helps. Running the full API suite with it unconditionally enabled is actually 25% slower - something about running a lot of sync tests at the same time slows it down a lot. So
concurrencyis only enabled when tests are filtered in some way - but enabling it like that can makes those tests finish in like half the wall-clock time, which is pretty nice.