Skip to content

Isolate run-all default-concurrency tests from ambient env - #591

Merged
TheGreatAxios merged 1 commit into
mainfrom
cl-7240-stop-run-alls-own-test-from-breaking-under
Sep 4, 2026
Merged

TheGreatAxios merged 1 commit into
mainfrom
cl-7240-stop-run-alls-own-test-from-breaking-under

Conversation

@TheGreatAxios

@TheGreatAxios TheGreatAxios commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

scripts/run-all.test.ts runProbe spread process.env into the child, so the default-sequential assertion (peakOverlap == 1) failed when the parent had WORKBENCH_CHECK_CONCURRENCY set (for example while running the gate itself). Tests that intend an override already inject the var; the default path now clears it unless a test sets it. A new test sets the ambient var and still expects sequential default.

CL-7240

Test plan

  • bun test scripts/run-all.test.ts (15 pass, including the new ambient-env isolation test and existing override tests)
  • Red: ambient WORKBENCH_CHECK_CONCURRENCY=4 made the new test fail with peakOverlap 4 vs 1
  • Green after runProbe stopped inheriting the var unless extraEnv sets it

Fixes CL-7240

@TheGreatAxios

Copy link
Copy Markdown
Contributor Author

Fixes CL-7240

@TheGreatAxios

Copy link
Copy Markdown
Contributor Author

Self-review

Verdict: minimal, correctly targeted fix — runProbe no longer leaks an ambient WORKBENCH_CHECK_CONCURRENCY into default-concurrency probes, and the new test proves it with a real red/green cycle. No defects found.

  • CONFIRMED-OPEN: none
  • DISMISSED: checked for exactOptionalPropertyTypes hazards, bare catches, as T on untrusted input, fallbacks left beside new paths, and product logic leaking into apps/* — none apply, this is test-harness-only. The Object.hasOwn(extraEnv, ...) check correctly distinguishes "override intentionally set" from "not set", including the WORKBENCH_CHECK_CONCURRENCY: "0" rejection test.
  • Verified the fix actually addresses the stated bug: ran the full suite locally with WORKBENCH_CHECK_CONCURRENCY=4 set on the parent process before either test existed in isolation — passes with the fix, and the PR description documents the red state without it.

Targeted tests run:

  • bun test scripts/run-all.test.ts — 15 pass, 0 fail
  • WORKBENCH_CHECK_CONCURRENCY=4 bun test scripts/run-all.test.ts — 15 pass, 0 fail (ambient-leak repro confirms the fix holds)

@linear-code

linear-code Bot commented Sep 4, 2026

Copy link
Copy Markdown

CL-7240

The default sequential probe spread process.env into the child, so
WORKBENCH_CHECK_CONCURRENCY set by the parent gate made peakOverlap == 1
fail. Override tests already inject the var; the default path now clears
it unless a test sets it.
@TheGreatAxios
TheGreatAxios force-pushed the cl-7240-stop-run-alls-own-test-from-breaking-under branch from 094cc78 to d77304f Compare September 4, 2026 05:07
@TheGreatAxios
TheGreatAxios merged commit 7f407ca into main Sep 4, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant