test(web): checkpoint web test cleanup#1848
Draft
tyler-dane wants to merge 1 commit into
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Web Test Isolation Implementation Plan
Goal: Make web tests independent enough that
bun test --cwd packages/webcan run the whole suite directly and in parallel without custom batching or order-dependent cleanup.Architecture: The current batching checkpoint reduces process launches, but it preserves the wrong abstraction: test isolation is handled by
packages/scripts/src/testing/run.tsinstead of by the tests. The real fix is to remove cross-file module mock leakage, use real providers or explicit dependency seams in tests, and keep shared cleanup limited to browser/runtime state that every test owns.Tech Stack: Bun test, React Testing Library,
user-event, Redux Toolkit, MSW, JSDOM.Problems With The Current Checkpoint
The committed batching runner is useful only as a checkpoint. It is not the desired design.
packages/scripts/src/testing/run.tsnow knows too much about web test layout. A package test command should not need custom filesystem walking, chunk sizing, orWEB_TEST_BATCH_SIZEto be reliable.mock.module()calls are still sticky across imports inside one Bun process. That makes tests contaminate later tests when a mock exports only the symbols the first test needs.PlannerSidebar.test.tsxexposed the core smell: parent tests that mock whole child modules are cheap to write but expensive for suite isolation.bun test --cwd packages/web. A separate script runner should not be needed for web tests.Desired End State
package.jsonshould eventually contain:{ "scripts": { "test:web": "bun test --cwd packages/web" } }packages/scripts/src/testing/run.tsshould eventually treatweblike a normal project command:The following commands should pass from the repo root:
bun test --cwd packages/web bun run test:web bun run type-check:web-tests bun lintFile Structure
package.jsontest:webat the direct Bun command after isolation work is complete.packages/scripts/src/testing/run.tspackages/web/src/__tests__/web.preload.tspackages/web/src/__tests__/test-module-isolation.test.tspackages/web/src/__tests__/render-with-store.tsxpackages/web/src/components/HeaderInfoIcon/HeaderInfoIcon.test.tsxpackages/web/src/components/ContextMenu/ContextMenuItems.test.tsxrender-with-store.tsx.packages/web/src/common/utils/draft/someday.draft.util.test.tsevent.util.mock.module(Approved patterns:
Task 1: Add A Direct-Run Failure Harness
Files:
Create:
packages/web/src/__tests__/test-module-isolation.test.tsStep 1: Add a guard test that documents why broad top-level module mocks are dangerous
Run:
bun test --cwd packages/webExpected: FAIL before the migration is complete. Record the first failing file pair in the implementation notes. The failure usually looks like a missing named export from a module mocked by an earlier test.
git add packages/web/src/__tests__/test-module-isolation.test.ts git commit -m "test(web): document module mock isolation risk"Task 2: Add Shared Real-Provider Test Helpers
Files:
Create:
packages/web/src/__tests__/render-with-store.tsxModify:
packages/web/src/components/ContextMenu/ContextMenuItems.test.tsxStep 1: Create the Redux render helper
ContextMenuItems.test.tsxUse:
Replace local
configureStore,Provider, andreducerssetup with:Run:
bun test --cwd packages/web src/components/ContextMenu/ContextMenuItems.test.tsxExpected: PASS.
git add packages/web/src/__tests__/render-with-store.tsx packages/web/src/components/ContextMenu/ContextMenuItems.test.tsx git commit -m "test(web): add real store render helper"Task 3: Remove Broad Top-Level Module Mocks
Files:
Modify: every file returned by
rg -n "mock\\.module\\(" packages/web/src --glob '*.{test.ts,test.tsx}'Step 1: Inventory web module mocks
Run:
Expected:
/tmp/web-module-mocks.txtlists every top-level module mock.For each entry, write one of these labels beside it in local notes:
For tests that mock child React components, use real children. If the parent behavior cannot be asserted without mocking every child, remove that parent test and keep child tests.
Example replacement:
Use
renderWithStoreinstead of mocking@web/store/store.hooks.Example:
Do not mock utility modules such as
@web/common/utils/event/event.util. Mock the leaf dependency that makes the utility hard to use.Example:
Run the converted test and the next test file in sorted order:
bun test --cwd packages/web path/to/converted.test.ts path/to/next-sorted.test.tsExpected: PASS. This catches exactly the cross-file leak the batch runner currently hides.
git add path/to/converted.test.ts path/to/related-helper.tsx git commit -m "test(web): remove leaking module mock from <area>"Task 4: Prove Direct Whole-Suite Execution
Files:
Modify:
package.jsonModify:
packages/scripts/src/testing/run.tsStep 1: Run direct web tests before changing scripts
Run:
bun test --cwd packages/webExpected: PASS. Do not proceed until this passes.
Set
package.json:{ "scripts": { "test:web": "bun test --cwd packages/web" } }Keep the rest of the scripts unchanged.
In
packages/scripts/src/testing/run.ts, remove:Set the web project command to:
Use the generic project path:
Run:
Expected: all commands exit 0.
git add package.json packages/scripts/src/testing/run.ts git commit -m "test(web): run web suite directly with bun"Task 5: Add A Test Hygiene Section To The Playbook
Files:
Modify:
docs/development/testing-playbook.mdStep 1: Add web isolation rules
Add this section:
Then run the whole web suite:
bun test --cwd packages/webExpected: PASS.
git add docs/development/testing-playbook.md git commit -m "docs: document web test isolation rules"Acceptance Criteria
bun test --cwd packages/webpasses from the repo root.bun run test:webis a direct alias forbun test --cwd packages/web.packages/scripts/src/testing/run.tshas no web-specific file discovery, chunking, or batch-size configuration.WEB_TEST_BATCH_SIZEno longer exists.bun run type-check:web-tests,bun lint, andbun run test:scriptspass.Self-Review
createTestStore,renderWithStore, andRootState.