test: isolate cwd-mutating unit tests - #3540
Conversation
Pre-push intermittently failed after thousands of parallel tests when AJV loaded during cli/commands/styles/command.test.ts and Node could no longer read the process cwd. The filesystem cwd lock serializes test writers, but arbitrary Deno and Node module loaders do not acquire that lock before resolving modules. The unit task now partitions discovery into a broad parallel phase that excludes cwd-mutating files, a serial cwd phase, and a separate parallel two-file mutex probe. A regression test pins the task partition so future changes do not reintroduce cwd mutation into broad parallel discovery. Constraint: Deno.chdir is process-global, so writer serialization alone cannot protect unrelated module loaders in the same process Rejected: Deno --ignore for this partition | it did not reliably exclude every explicit test path, so discovery-time find ! -path filters are used Confidence: high Scope-risk: narrow Related: #3538 Related: #3494 Tested: focused regression RED then GREEN Tested: deno task test:unit passed with 3773 parallel tests / 27898 steps, 10 serial cwd tests / 193 steps, and 2 cwd-exclusion probes Tested: deno fmt --check checked 4977 files Tested: deno lint checked 4902 files Tested: deno check src/index.ts Tested: git diff --check Not-tested: No additional manual verification was run during git packaging before push hooks
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe unit-test configuration now separates parallel, cwd-sensitive, and cwd-exclusion tasks. A configuration test defines expected test groups and verifies task commands, parallel flags, and path inclusion or exclusion. ChangesUnit Test Orchestration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/testing/cwd.test.ts`:
- Around line 53-79: Update the assertions in the CWD task-generation test to
verify the groups are disjoint: each CWD_EXCLUSION_TESTS path must be absent
from cwdTask, and each CWD_SCOPED_TESTS path must be absent from exclusionTask,
while preserving the existing positive membership checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e6b2d873-b7a6-4ba0-9a47-6e5a9b60be29
📒 Files selected for processing (2)
deno.jsonsrc/testing/cwd.test.ts
Ships the build fixes for veryfront-issue-inbox#456. `veryfront build` has been broken for every project since 0.1.1206, and 5 of 7 scaffold templates could not build at all. - #3538 prime host contracts in the build extension path - #3541 resolve server-side esm.sh modules with a server target - #3542 keep optional Deno imports optional - #3540 isolate cwd-mutating unit tests The version is embedded in hydration-runtime.generated.ts, so a bump must be followed by `deno task generate`; the manifests check fails otherwise. There is no earlier version to pin to: 0.1.1205 fails the same templates with the CSSProcessor error that #3417 was fixing.
Summary
Pre-push intermittently failed after thousands of parallel unit tests with
NotFound: Unable to get CWDwhile AJV loaded duringcli/commands/styles/command.test.ts. This was observed while pushing PR #3538, but the failure is separate from that build-extension fix.This changes
test:unitinto three phases:The regression test in
src/testing/cwd.test.tspins that partition so future task changes do not put cwd mutation back into broad parallel discovery.Root cause
Deno.chdiris process-global. The filesystem lock serializes the tests that intentionally change cwd, but arbitrary Deno and Node module loaders do not acquire that lock before resolving modules. A parallel module loader can therefore observe the process while another test has moved or removed cwd, producingUnable to get CWD.Implementation notes
A first attempt using Deno
--ignorewas rejected because it did not reliably exclude every explicit test path. The final task uses discovery-timefind ! -pathfilters so the broad parallel phase never receives those files.This builds on the prior writer-lock work in PR #3494.
Validation
deno task test:unitvalidation passed with 3773 parallel tests / 27898 steps, 10 serial cwd tests / 193 steps, and 2 cwd-exclusion probes.origin/mainpasseddeno task test:unitwith 3774 parallel tests / 27905 steps, 10 serial cwd tests / 193 steps, and 2 cwd-exclusion probes.deno fmt --checkpassed. Earlier manual run checked 4977 files; current pre-push checked 4980 files afterorigin/mainadvanced.deno lintpassed. Earlier manual run checked 4902 files; current pre-push checked 4905 files afterorigin/mainadvanced.deno check src/index.tspassed.git diff --checkpassed.Related
Summary by CodeRabbit
Tests
Chores