Skip to content

test(web): checkpoint web test cleanup#1848

Draft
tyler-dane wants to merge 1 commit into
mainfrom
chore/web-test-cleanup
Draft

test(web): checkpoint web test cleanup#1848
tyler-dane wants to merge 1 commit into
mainfrom
chore/web-test-cleanup

Conversation

@tyler-dane
Copy link
Copy Markdown
Contributor

Web Test Isolation Implementation Plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: Make web tests independent enough that bun test --cwd packages/web can 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.ts instead 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.

  1. packages/scripts/src/testing/run.ts now knows too much about web test layout. A package test command should not need custom filesystem walking, chunk sizing, or WEB_TEST_BATCH_SIZE to be reliable.
  2. Batch size is a hidden compatibility contract. Increasing from 3 to 4 already exposed module mock leakage, which means the suite still depends on file ordering and process boundaries.
  3. Top-level 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.
  4. Shared teardown can reset DOM, storage, MSW, and browser URL, but it cannot make an already-imported module forget a global top-level module mock.
  5. The deleted shallow PlannerSidebar.test.tsx exposed the core smell: parent tests that mock whole child modules are cheap to write but expensive for suite isolation.
  6. The final web command should be boring: bun test --cwd packages/web. A separate script runner should not be needed for web tests.

Desired End State

package.json should eventually contain:

{
  "scripts": {
    "test:web": "bun test --cwd packages/web"
  }
}

packages/scripts/src/testing/run.ts should eventually treat web like a normal project command:

web: {
  cmd: ["bun", "test", "--cwd", "packages/web"],
},

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 lint

File Structure

  • Modify: package.json
    • Point test:web at the direct Bun command after isolation work is complete.
  • Modify: packages/scripts/src/testing/run.ts
    • Remove web-specific test discovery, chunking, and batch-size handling.
  • Modify: packages/web/src/__tests__/web.preload.ts
    • Keep only universal runtime cleanup: React cleanup, DOM reset, storage reset, MSW reset, URL reset.
  • Create: packages/web/src/__tests__/test-module-isolation.test.ts
    • A guard test that makes broad module mock leakage visible while the migration is in progress.
  • Create: packages/web/src/__tests__/render-with-store.tsx
    • Shared Redux provider helper for component tests that currently mock store hooks.
  • Modify: packages/web/src/components/HeaderInfoIcon/HeaderInfoIcon.test.tsx
    • Replace broad icon-module and child-module mocks with real rendering or local props/state.
  • Modify: packages/web/src/components/ContextMenu/ContextMenuItems.test.tsx
    • Keep the real Redux store pattern and move reusable setup into render-with-store.tsx.
  • Modify: packages/web/src/common/utils/draft/someday.draft.util.test.ts
    • Keep the real event utility dependency and mocked session user; do not mock event.util.
  • Modify: all web tests containing mock.module(
    • Convert broad top-level module mocks into one of the approved patterns below.

Approved patterns:

  1. Mock leaf modules only when the real module is network, browser, timer, or native-boundary code.
  2. If a component needs Redux, render it with a real configured store.
  3. If a hook needs a session, mock only the session leaf module and export every symbol from that module that the imported code may use.
  4. If a parent component is hard to test without mocking children, test the behavior through the real children or delete the shallow parent test when child coverage already protects the behavior.

Task 1: Add A Direct-Run Failure Harness

Files:

  • Create: packages/web/src/__tests__/test-module-isolation.test.ts

  • Step 1: Add a guard test that documents why broad top-level module mocks are dangerous

import { describe, expect, it, mock } from "bun:test";

describe("web test module isolation guard", () => {
  it("documents that Bun module mocks are process-scoped once a module is imported", async () => {
    const modulePath = `./module-isolation-${Date.now()}-${Math.random()}.ts`;

    mock.module(modulePath, () => ({
      firstExport: "mocked",
    }));

    const firstImport = (await import(modulePath)) as { firstExport: string };

    expect(firstImport.firstExport).toBe("mocked");
  });
});
  • Step 2: Run the existing direct command and capture the first real leak

Run:

bun test --cwd packages/web

Expected: 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.

  • Step 3: Commit the guard
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.tsx

  • Modify: packages/web/src/components/ContextMenu/ContextMenuItems.test.tsx

  • Step 1: Create the Redux render helper

import { configureStore, type PreloadedState } from "@reduxjs/toolkit";
import { type PropsWithChildren, type ReactElement } from "react";
import { Provider } from "react-redux";
import { render } from "@testing-library/react";
import { reducers } from "@web/store/reducers";
import { type RootState } from "@web/store/store";

export function createTestStore(preloadedState?: PreloadedState<RootState>) {
  return configureStore({
    reducer: reducers,
    preloadedState,
    middleware: (getDefaultMiddleware) =>
      getDefaultMiddleware({
        serializableCheck: false,
        immutableCheck: false,
        thunk: false,
      }),
  });
}

export function renderWithStore(
  ui: ReactElement,
  preloadedState?: PreloadedState<RootState>,
) {
  const store = createTestStore(preloadedState);

  function StoreProvider({ children }: PropsWithChildren) {
    return <Provider store={store}>{children}</Provider>;
  }

  return {
    store,
    ...render(ui, { wrapper: StoreProvider }),
  };
}
  • Step 2: Replace local store setup in ContextMenuItems.test.tsx

Use:

import { renderWithStore } from "@web/__tests__/render-with-store";

Replace local configureStore, Provider, and reducers setup with:

const renderWithTheme = (component: React.ReactElement) =>
  renderWithStore(<ThemeProvider theme={theme}>{component}</ThemeProvider>, currentState);
  • Step 3: Verify the component test

Run:

bun test --cwd packages/web src/components/ContextMenu/ContextMenuItems.test.tsx

Expected: PASS.

  • Step 4: Commit
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:

rg -n "mock\\.module\\(" packages/web/src --glob '*.{test.ts,test.tsx}' > /tmp/web-module-mocks.txt

Expected: /tmp/web-module-mocks.txt lists every top-level module mock.

  • Step 2: Classify each mock

For each entry, write one of these labels beside it in local notes:

keep-leaf-boundary
replace-with-real-provider
replace-with-real-child
replace-with-function-parameter
delete-shallow-parent-test
  • Step 3: Replace broad UI child mocks

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:

// Delete this pattern:
mock.module("@web/components/Tooltip/TooltipWrapper", () => ({
  TooltipWrapper: ({ children }: { children: React.ReactNode }) => <>{children}</>,
}));

// Use the real component and query visible behavior:
expect(screen.getByRole("button", { name: /shortcuts/i })).toBeInTheDocument();
  • Step 4: Replace broad store hook mocks

Use renderWithStore instead of mocking @web/store/store.hooks.

Example:

renderWithStore(<ComponentUnderTest />, {
  events: {
    // Provide only the state this component reads.
  },
});
  • Step 5: Replace broad utility mocks

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:

mock.module("@web/auth/compass/session/session.util", () => ({
  getUserId: mock().mockResolvedValue("mock-user"),
}));
  • Step 6: Verify after each converted cluster

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.ts

Expected: PASS. This catches exactly the cross-file leak the batch runner currently hides.

  • Step 7: Commit each cluster
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.json

  • Modify: packages/scripts/src/testing/run.ts

  • Step 1: Run direct web tests before changing scripts

Run:

bun test --cwd packages/web

Expected: PASS. Do not proceed until this passes.

  • Step 2: Replace the package script

Set package.json:

{
  "scripts": {
    "test:web": "bun test --cwd packages/web"
  }
}

Keep the rest of the scripts unchanged.

  • Step 3: Remove web batching from the shared runner

In packages/scripts/src/testing/run.ts, remove:

const WEB_ROOT = resolve(process.cwd(), "packages/web");
const WEB_SRC = resolve(WEB_ROOT, "src");
const WEB_TEST_FILE_PATTERN = /\.(spec|test)\.[tj]sx?$/;
const DEFAULT_WEB_TEST_BATCH_SIZE = 3;
function findWebTestFiles(dir: string): string[] { /* ... */ }
function chunk<T>(items: T[], size: number): T[][] { /* ... */ }
function getWebTestBatchSize() { /* ... */ }

Set the web project command to:

web: {
  cmd: ["bun", "test", "--cwd", "packages/web"],
},

Use the generic project path:

function runProject(projectName: keyof typeof TEST_PROJECTS) {
  runCommand(TEST_PROJECTS[projectName].cmd);
}
  • Step 4: Verify all affected commands

Run:

bun run test:web
bun run type-check:web-tests
bun lint
bun run test:scripts

Expected: all commands exit 0.

  • Step 5: Commit
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.md

  • Step 1: Add web isolation rules

Add this section:

## Web Test Isolation

Web tests should run with `bun test --cwd packages/web` without file batching or process-level ordering assumptions.

Avoid broad top-level `mock.module()` calls. A module mock is acceptable only for a leaf boundary such as network, browser API, timer, storage adapter, or session adapter. Do not mock parent UI children, Redux store hooks, or shared utility modules just to make setup shorter.

Prefer real providers:

- Use React Testing Library with semantic queries.
- Use a real Redux test store for components that read or dispatch Redux state.
- Use MSW for network behavior.
- Mock the smallest leaf dependency when external state is required.

When changing a test that uses `mock.module()`, run that test with the next sorted web test file:

```bash
bun test --cwd packages/web path/to/current.test.ts path/to/next.test.ts

Then run the whole web suite:

bun test --cwd packages/web

- [ ] **Step 2: Verify docs lint**

Run:

```bash
bun lint

Expected: PASS.

  • Step 3: Commit
git add docs/development/testing-playbook.md
git commit -m "docs: document web test isolation rules"

Acceptance Criteria

  • bun test --cwd packages/web passes from the repo root.
  • bun run test:web is a direct alias for bun test --cwd packages/web.
  • packages/scripts/src/testing/run.ts has no web-specific file discovery, chunking, or batch-size configuration.
  • WEB_TEST_BATCH_SIZE no longer exists.
  • Web tests do not rely on broad top-level mocks of UI children, Redux hooks, or shared utility modules.
  • bun run type-check:web-tests, bun lint, and bun run test:scripts pass.

Self-Review

  • Spec coverage: the plan directly addresses the user's objection to batching, the custom TypeScript runner smell, and leaking web tests.
  • Placeholder scan: no task uses placeholder implementation language; each task includes concrete files, commands, and expected outcomes.
  • Type consistency: helper names are consistent: createTestStore, renderWithStore, and RootState.

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