refactor: split oversized test suites#106354
Conversation
848015a to
cc6930c
Compare
|
Pre-land proof at Proof:
No runtime, dependency, config, docs, or changelog changes. |
cc6930c to
62db2dd
Compare
|
Codex review: needs maintainer review before merge. Reviewed July 13, 2026, 8:47 AM ET / 12:47 UTC. Summary PR surface: Source +19756, Tests -9527. Total +10229 across 42 files. Reproducibility: not applicable. This PR addresses test-suite maintainability rather than a runtime bug. Its preservation claim is evaluated through declaration identity, test discovery, and exact-head suite execution. Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Refresh the branch onto current main, retain the five original Vitest entry points and recognized Do we have a high-confidence way to reproduce the issue? Not applicable; this PR addresses test-suite maintainability rather than a runtime bug. Its preservation claim is evaluated through declaration identity, test discovery, and exact-head suite execution. Is this the best way to solve the issue? Yes, conditionally. Keeping one entry per suite while importing cohesive test utilities is a bounded way to reduce file size without deliberately changing the non-isolated mocked module graph, provided the refreshed-head audits remain exact. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d9a6be9b1639. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +19756, Tests -9527. Total +10229 across 42 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
62db2dd to
c8f6096
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8f6096d9c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Explicit bundled ownership outranks content-based discovery. Otherwise extracting | ||
| // a test body can silently move its entry to a config with the wrong mocked setup. | ||
| ...bundledPluginDependentUnitTestFiles, |
There was a problem hiding this comment.
Exclude split suites from unit-fast routing
When this split leaves the entry files as import-only, the unit-fast scanner still only reads the .test.ts entry body. I checked isUnitFastTestFile()/buildVitestRunPlans(), and both src/agents/openai-transport-stream.test.ts and src/auto-reply/reply/dispatch-from-config.test.ts now route to test/vitest/vitest.unit-fast.config.ts, even though their imported utilities contain vi.useFakeTimers, vi.mock, and registry resets. That removes them from the agents/auto-reply scoped configs and can run fake-timer/mock suites in the env-only parallel unit-fast lane; add these split entries to the explicit skip list or teach the scanner to follow imported test-utils.
Useful? React with 👍 / 👎.
c8f6096 to
d08642d
Compare
|
Merged via squash.
|
* refactor: split oversized test suites * test: classify split modules as test utilities * test: preserve bundled routing after suite splits * style: format unit-fast routing import * test: preserve owner routing for split suites
* refactor: split oversized test suites * test: classify split modules as test utilities * test: preserve bundled routing after suite splits * style: format unit-fast routing import * test: preserve owner routing for split suites
Closes #106350
What Problem This Solves
Five core test suites had grown to roughly 9,500–13,800 lines each, making navigation, ownership review, and focused changes unnecessarily difficult.
Why This Change Was Made
Each existing
*.test.tsremains the sole Vitest entry and imports cohesive*.test-utils.tsmodules plus a narrowly shared harness. This preserves the original mocked module graph and runtime ordering; independent test entries were rejected because non-isolated Vitest workers can retain singleton state between files.The import-only entries are explicitly kept in their existing owner configurations. That prevents content-based unit-fast discovery from silently moving the OpenAI transport and configured-dispatch suites, and keeps the plugin-loader suite in its bundled environment. Task-boundary scanning also ignores the new test-only utility and harness suffixes.
User Impact
No user-visible or runtime behavior change. Maintainers get focused modules capped at 2,273 lines instead of five 9,500–13,800-line files. No changelog entry because this is test-only.
Evidence
294 + 262 + 214 + 136 + 217).tbx_01kxdtzscrkn3xj66awcxvnkh0atd08642d1f0af: 1,489/1,489 tests passed across six owner-selected shards. This includes all five split entries (1,299 tests) plus 190 routing/config regression tests.pnpm check:changed: all typecheck, format, lint, and architecture guards passed in 15m29s.git diff --checkpassed.