Skip to content

test: isolate process-heavy upstream lifecycle contracts (#255) - #293

Merged
mohanagy merged 1 commit into
developmentfrom
fix/issue-255-same-file-lifecycle-groups
Jul 28, 2026
Merged

test: isolate process-heavy upstream lifecycle contracts (#255)#293
mohanagy merged 1 commit into
developmentfrom
fix/issue-255-same-file-lifecycle-groups

Conversation

@mohanagy

@mohanagy mohanagy commented Jul 28, 2026

Copy link
Copy Markdown
Owner

What changed

  • moved the existing upstream-manager lifecycle contracts into a shared registration helper
  • split the 37 contracts across basics, recovery, and teardown test entry files so Vitest gives each group a fresh fork
  • added a focused suite-layout contract that prevents the process-heavy tests from collapsing back into one file

Root cause

The prior fresh-fork fix isolated test files, but all 37 real-child upstream lifecycle contracts still shared one worker process because they remained in a single file. During aggregate runs, startup timeout failures migrated between otherwise healthy tests; the exact failing test and the complete original file both passed independently. Splitting by lifecycle group removes the remaining same-file process accumulation without increasing the 1,000 ms startup timeout.

Impact

Production code and public APIs are unchanged. Existing test bodies, timeouts, process containment, platform coverage, and error behavior are preserved.

Closes #255

Validation

  • focused TDD: the suite-layout contract failed before the split, then 37 passed / 1 platform skip
  • npm test: 1,770 passed / 33 skipped
  • npm run test:core: 425 passed / 26 skipped
  • npm run test:coverage: 1,770 passed / 33 skipped; 95.65% statements, 91.92% branches, 98.66% functions, 95.65% lines
  • npm run lint
  • npm run typecheck
  • npm run build
  • npm run smoke:cli
  • npm run check:pack
  • npm run test:package: 26 passed

Summary by CodeRabbit

  • Tests
    • Reorganized upstream manager contract tests into separate basics, recovery, and teardown suites.
    • Added validation for the expected test suite structure and contract registration.
    • Added coverage for delayed shutdown behavior and preventing idle shutdown during active upstream requests.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1e9f1200-a0ce-449d-9213-6e4bc2bfedc1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The upstream-manager contract helper now dispatches grouped registrations for basics, recovery, and teardown. Separate test entrypoints invoke each group, and a new suite validates the file layout, exports, dispatch branches, and selected shutdown contracts.

Changes

Upstream manager suite split

Layer / File(s) Summary
Contract dispatcher and grouped registrations
tests/helpers/upstream-manager-contracts.ts
Updates helper paths, exports the contract-group API, and organizes registrations into basics, recovery, and teardown wrappers.
Lifecycle-specific test entrypoints
tests/upstream-manager-*.test.ts
Adds separate entrypoints that register each upstream-manager contract group.
Suite layout validation
tests/upstream-manager-suite-layout.test.ts
Verifies the legacy entrypoint is absent and checks lifecycle registrations, dispatcher branches, exports, and shutdown-related contract text.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

I’m a rabbit with tests in a row,
Three contract burrows now neatly grow.
Basics, recovery, teardown in flight,
Each hops through its own entrypoint bright.
The suite checks the paths,
And avoids old-file wrath.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately describes the test-splitting change for upstream lifecycle contracts.
Description check ✅ Passed It includes a summary, impact, and detailed validation results, though the headings differ from the template.
Linked Issues check ✅ Passed The split test entrypoints, shared helper, and suite-layout regression coverage match the issue's goal without weakening isolation or raising timeouts.
Out of Scope Changes check ✅ Passed Changes stay within test helpers and entrypoints; no unrelated production code or extra behaviors were introduced.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-255-same-file-lifecycle-groups

Comment @coderabbitai help to get the list of available commands.

@mohanagy
mohanagy marked this pull request as ready for review July 28, 2026 16:52

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@mohanagy
mohanagy merged commit 36bcab5 into development Jul 28, 2026
12 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.

test: eliminate aggregate-suite upstream startup instability

1 participant