Skip to content

🤖 tests: isolate renderer test globals#3574

Merged
ThomasK33 merged 1 commit into
mainfrom
tests-ui-test-isolation
Jun 16, 2026
Merged

🤖 tests: isolate renderer test globals#3574
ThomasK33 merged 1 commit into
mainfrom
tests-ui-test-isolation

Conversation

@ThomasK33

Copy link
Copy Markdown
Member

Summary

  • Stabilizes local renderer unit-test runs by keeping test-only module mocks and DOM globals scoped to the suites that need them.

Background

  • bun test src runs many renderer test files in one process locally, so file-scope mock.module() registrations and direct globalThis.window/document overwrites can leak into later suites.
  • The repro surfaced as useChatTranscriptFullWidth not calling its real API context after useModelsFromSettings, events.test seeing document as undefined, chatCommands inheriting a remote window.location, and ImmersiveReviewView rendering a stubbed diff renderer from ProposePlanToolCall.

Implementation

  • Moves useModelsFromSettings and ProposePlanToolCall module mocks behind setup/teardown helpers that restore the real module exports after each test.
  • Uses installDom() or explicit previous-global restoration so renderer tests leave the global DOM state as they found it.
  • Makes events.test install its own DOM per test instead of relying on a one-time side-effect import.

Validation

  • bun test src/browser/hooks/useModelsFromSettings.test.ts src/browser/hooks/useChatTranscriptFullWidth.test.tsx src/browser/utils/events.test.ts
  • bun test src/browser/hooks/useRouting.test.ts src/browser/utils/chatCommands.test.ts
  • bun test src/browser/features/Tools/ProposePlanToolCall.test.tsx src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.test.tsx
  • bun test src/browser/hooks src/browser/utils
  • make static-check

Risks

  • Low product risk: this is test-only isolation. The main risk is that moving mocks into hooks could reveal tests that were unintentionally depending on cross-file pollution; the targeted cross-suite commands cover the known leak chains.

Pains

  • make test now gets past the renderer failures but still exits on the pre-existing local Storybook budget guard (71 > 70 snapshot-enabled story files, 276 > 262 estimated snapshots), which is unrelated to this diff and not part of the CI unit shard.

Generated with mux • Model: openai:gpt-5.5 • Thinking: xhigh • Cost: $46.22

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: ccd7666ea3

ℹ️ 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".

@ThomasK33 ThomasK33 added this pull request to the merge queue Jun 16, 2026
Merged via the queue into main with commit ec3314e Jun 16, 2026
23 checks passed
@ThomasK33 ThomasK33 deleted the tests-ui-test-isolation branch June 16, 2026 10:36
@mux-bot mux-bot Bot mentioned this pull request Jun 16, 2026
LeonidasZhak pushed a commit to LeonidasZhak/mux that referenced this pull request Jun 20, 2026
## Summary

Long-lived auto-cleanup PR maintained by the **Auto-Cleanup Agent**.
Each pass lands at most one extremely low-risk, behavior-preserving
cleanup drawn from recently merged `main` activity, then advances the
checkpoint below.

## This pass

Replaced an inline terminal-status check in
`WorkflowService.interruptRunOnAbort`
(`src/node/services/workflows/WorkflowService.ts`) with the
`isTerminalWorkflowRunStatus` helper introduced by coder#3587 ("show
workflow-only sidebar activity"). The guard

```ts
if (
  run == null ||
  run.status === "completed" ||
  run.status === "failed" ||
  run.status === "interrupted"
) {
  return;
}
```

now reads `if (run == null || isTerminalWorkflowRunStatus(run.status))`.

- **Behavior-preserving:** `isTerminalWorkflowRunStatus(status)` is
defined as `status === "completed" || status === "failed" || status ===
"interrupted"` — byte-for-byte the same three-status disjunction (same
short-circuit order, same boolean result), so the `run == null || ...`
early-return is unchanged.
- **Why it helps:** `isTerminalWorkflowRunStatus` is already the
canonical terminal-status predicate everywhere else (`taskService.ts`,
`tools/task_await.ts`); this was the only remaining hand-inlined copy in
`WorkflowService`, so the dedup keeps the terminal-status set defined in
exactly one place.
- **Scoped:** one file, one guard; the helper was already exported from
`@/common/types/workflow` and is now added to the existing import in
`WorkflowService.ts` (next to the already-imported
`isActiveWorkflowRunStatus`).

## Validation

- `bun test src/node/services/workflows/WorkflowService.test.ts` — 58
pass / 0 fail.
- `make static-check` — eslint, tsc (both configs), Prettier, and the
generated workflow-runtime bundle drift check all pass. The only failing
step is `fmt-shell-check`, which requires `shfmt` (not installed in this
environment); no shell scripts were touched, so CI (which has `shfmt`)
is unaffected.

<details>
<summary>Prior passes</summary>

- Deduplicated the identical `workspaceId`-only metadata JSON Schema
that three id-targeted host actions repeat in
`src/node/services/workflows/workspaceHostActions.ts` into a
module-level `WORKSPACE_ID_ONLY_INPUT_SCHEMA` constant (the third copy
was introduced by coder#3583 when it added `workspace.unarchive`).
Behavior-preserving: nothing mutates the metadata, and the shared
constant has identical key order/values so the generated stub bundle is
byte-identical (drift check passing).
- Extracted a shared `BlockedBadge` component in
`src/browser/features/Tools/WorkflowDefinitionToolCall.tsx` to
deduplicate the `<WorkflowBadge tone="danger">blocked</WorkflowBadge>`
markup that coder#3578 introduced verbatim in three spots. Also resolved a
trivial rebase conflict from coder#3582 in
`src/node/services/workflows/workflowArgs.ts`.
- Deduplicated the repeated `"Workflow metadata.argsSchema.properties
must be an object"` error string in
`src/node/services/workflows/workflowArgs.ts` (added in coder#3575) into a
module-level `ARGS_SCHEMA_PROPERTIES_ERROR_MESSAGE` constant.
- Deduplicated the four mocked-module specifier strings in
`src/browser/hooks/useModelsFromSettings.test.ts` into shared local
constants so `installUseModelsModuleMocks()` /
`restoreUseModelsModuleMocks()` can't drift (after coder#3574).
- Removed a redundant `toggleWorkflowExpanded` wrapper in
`src/browser/features/Tools/WorkflowRunToolCall.tsx`, left behind by
coder#3571.
- Deduplicated two repeated fallback error strings in
`src/browser/components/AutomationModal/AutomationModal.tsx` (after
coder#3560) into `SAVE_AUTOMATION_ERROR_MESSAGE` /
`REMOVE_AUTOMATION_ERROR_MESSAGE` module-level constants.
- Replaced a duplicated `"wfr_"` literal in
`src/node/services/workflows/WorkflowRunner.ts` with the canonical
`isWorkflowRunTaskId()` predicate from
`src/node/services/tools/taskId.ts` (after coder#3565).
- Removed a duplicated `"chat.jsonl"` literal in
`src/node/services/analytics/etl.ts` by re-exporting `CHAT_FILE_NAME`
from the canonical `src/common/constants/paths.ts` (after coder#3541).
- Extracted a `harvestRecordTime` helper in
`src/node/services/memoryConsolidationService.ts` so
`findNewestHarvestRecord` and `pruneHarvestRecords` derive the
`completedAt ?? startedAt` ordering key from one place (after coder#3558).

</details>

Auto-cleanup checkpoint: a7f047b

---

_Generated with `mux` • Model: `anthropic:claude-opus-4-8` • Thinking:
`xhigh` • Cost: `n/a`_

<!-- mux-attribution: model=anthropic:claude-opus-4-8 thinking=xhigh
costs=n/a -->

---------

Co-authored-by: mux-bot[bot] <264182336+mux-bot[bot]@users.noreply.github.com>
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