Skip to content

refactor: split oversized test suites#106354

Merged
steipete merged 5 commits into
mainfrom
codex/split-large-test-files
Jul 13, 2026
Merged

refactor: split oversized test suites#106354
steipete merged 5 commits into
mainfrom
codex/split-large-test-files

Conversation

@steipete

@steipete steipete commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

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.ts remains the sole Vitest entry and imports cohesive *.test-utils.ts modules 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

  • Exact AST registration audit: all 1,123 declarations preserved with zero missing or extra titles (294 + 262 + 214 + 136 + 217).
  • Exact-head Blacksmith Testbox tbx_01kxdtzscrkn3xj66awcxvnkh0 at d08642d1f0af: 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.
  • Exact-head full unit-fast: 1,182 files passed; 12,376 tests passed and 3 skipped in 42.35s. This directly verifies the prior no-output hang is gone.
  • Exact-head hosted CI run 29254304482: all 56 jobs passed.
  • Exact-head pnpm check:changed: all typecheck, format, lint, and architecture guards passed in 15m29s.
  • git diff --check passed.
  • Branch-wide automatic autoreview failed closed because the mechanical move is a 3.7 MB no-rename patch over its 180 KB bundle limit. Focused autoreviews of the routing corrections passed clean at 0.99 and 0.95 correctness; equivalent manual review covered the exact registration multiset, entry-only reachability, shared-harness dependency pruning, suffix classification, owner routing, and current-main rebase.

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime agents Agent runtime and tooling extensions: policy size: XL labels Jul 13, 2026
@steipete steipete self-assigned this Jul 13, 2026
@openclaw-barnacle openclaw-barnacle Bot added the maintainer Maintainer-authored PR label Jul 13, 2026
@steipete
steipete force-pushed the codex/split-large-test-files branch 2 times, most recently from 848015a to cc6930c Compare July 13, 2026 11:55
@steipete

steipete commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Pre-land proof at d08642d1f0af6f8fc679f24c2fa118c77c72387d.

Proof:

  • Exact AST registration audit preserved all 1,123 declarations with zero missing or extra titles (294 + 262 + 214 + 136 + 217).
  • Exact-head Blacksmith Testbox tbx_01kxdtzscrkn3xj66awcxvnkh0: 1,489/1,489 tests passed across six owner-selected Vitest shards. This includes all five split entries (1,299 tests) plus 190 routing/config regression tests.
  • Exact-head full unit-fast: 1,182 files passed; 12,376 tests passed and 3 skipped in 42.35s. The previous no-output hang is gone.
  • Exact-head hosted CI run 29254304482: all 56 jobs passed.
  • Exact-head pnpm check:changed: all typecheck, format, lint, and architecture guards passed in 15m29s.
  • Exact registration, suffix/reachability, owner-routing, task-boundary, formatting, and git diff --check guards passed.
  • Branch-wide autoreview failed closed at its 180 KB bundle ceiling for this 3.7 MB mechanical move. Focused autoreviews of the routing corrections passed clean at 0.99 and 0.95 correctness; equivalent manual review covered registration identity, entry-only reachability, harness dependency pruning, suffix classification, and current-main rebase.

No runtime, dependency, config, docs, or changelog changes.

@steipete
steipete force-pushed the codex/split-large-test-files branch from cc6930c to 62db2dd Compare July 13, 2026 12:19
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 13, 2026
@clawsweeper

clawsweeper Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 13, 2026, 8:47 AM ET / 12:47 UTC.

Summary
The PR splits five oversized Vitest suites into imported test utility modules and shared harnesses while retaining each original file as the sole test entry point.

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.

  • Test registration identity: 1,123 preserved, 0 missing, 0 extra. This directly measures whether the split silently removed or duplicated registered tests.

Stored data model
Persistent data-model change detected: migration/backfill/repair: extensions/policy/src/doctor/register.base.test-utils.ts, migration/backfill/repair: extensions/policy/src/doctor/register.gateway-data-and-approvals.test-utils.ts, migration/backfill/repair: extensions/policy/src/doctor/register.ingress-and-secrets.test-utils.ts, migration/backfill/repair: extensions/policy/src/doctor/register.models-and-mcp.test-utils.ts, migration/backfill/repair: extensions/policy/src/doctor/register.sandbox-and-tools.test-utils.ts, migration/backfill/repair: extensions/policy/src/doctor/register.test-harness.ts, and 13 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #106350
Summary: This PR is the explicit implementation candidate for the paired test-maintenance issue.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Refresh onto current main and repeat the exact registration, reachability, focused-suite, and changed-gate proof.

Risk before merge

  • [P1] The branch is behind current main, so intervening test additions or conflicts could invalidate the exact registration and execution-order proof until it is repeated on the refreshed result.
  • [P1] The unusually large mechanical move is difficult to validate from rendered patches alone; declaration identity, entry-only reachability, focused suite execution, and the changed gate remain important pre-merge checks.

Maintainer options:

  1. Decide the mitigation before merge
    Refresh the branch onto current main, retain the five original Vitest entry points and recognized .test-utils.ts classification, then repeat the exact registration, reachability, focused-suite, and changed-gate audits before landing.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • A maintainer should refresh the branch, reconcile the live hosted-check state with the supplied exact-head proof, repeat the mechanical audits, and decide whether to accept the large cleanup.

Maintainer decision needed

  • Question: Should the repository accept this large test-only reorganization after its exact audits pass again on a current-main refresh?
  • Rationale: The protected label and scale of the mechanical movement require explicit human acceptance even though the available proof reveals no concrete defect.
  • Likely owner: steipete — This account is the assigned decision owner on the protected PR and is the strongest available routing candidate.
  • Options:
    • Land after refreshed proof (recommended): Update the branch and merge if the registration, reachability, focused-suite, and changed-gate audits remain clean on the resulting head.
    • Retain the existing suites: Decline the paired cleanup direction if the ongoing review cost of the large reorganization outweighs its maintenance benefit.

Security
Cleared: The proposed changes are confined to test entries, test utilities, and harnesses, with no concrete dependency, workflow, secret, installation, package-resolution, or production execution concern.

Review details

Best possible solution:

Refresh the branch onto current main, retain the five original Vitest entry points and recognized .test-utils.ts classification, then repeat the exact registration, reachability, focused-suite, and changed-gate audits before landing.

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 changes

Label changes:

  • add P3: This is low-risk internal test-maintenance cleanup with no intended runtime, configuration, or user-visible behavior change.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR supplies exact-head Testbox execution for all five entries, the changed gate, and an exact declaration audit, which directly demonstrate the changed test organization functioning after the refactor.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR supplies exact-head Testbox execution for all five entries, the changed gate, and an exact declaration audit, which directly demonstrate the changed test organization functioning after the refactor.

Label justifications:

  • P3: This is low-risk internal test-maintenance cleanup with no intended runtime, configuration, or user-visible behavior change.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR supplies exact-head Testbox execution for all five entries, the changed gate, and an exact declaration audit, which directly demonstrate the changed test organization functioning after the refactor.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR supplies exact-head Testbox execution for all five entries, the changed gate, and an exact declaration audit, which directly demonstrate the changed test organization functioning after the refactor.
Evidence reviewed

PR surface:

Source +19756, Tests -9527. Total +10229 across 42 files.

View PR surface stats
Area Files Added Removed Net
Source 37 19756 0 +19756
Tests 5 5 9532 -9527
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 42 19761 9532 +10229

What I checked:

  • Single-entry module graph: The split utility modules explicitly remain imports of the original test entry so mocked tests execute within one Vitest module graph. (src/agents/openai-transport-stream.base.test-utils.ts:1, 62db2dd26d40)
  • Shared harness extraction: Common policy-doctor setup, contexts, registration helpers, and cleanup behavior move into a narrowly scoped test harness rather than production code. (extensions/policy/src/doctor/register.test-harness.ts:1, 62db2dd26d40)
  • Registration identity proof: The PR discussion reports an exact AST audit preserving all 1,123 test declarations with no missing or additional titles across the five entries. (62db2dd26d40)
  • Exact-head execution proof: The author reports that all five entries passed 1,299 expanded tests on Blacksmith Testbox and that the full changed gate passed on the current PR head. (62db2dd26d40)
  • Current-main drift: The supplied live metadata marks the PR mergeable but behind current main, so the declaration and execution audits should be repeated after refreshing the branch. (d9a6be9b1639)

Likely related people:

  • steipete: The live PR is assigned to this account and carries the protected maintainer-review signal; this is routing evidence rather than attribution of a defect. (role: assigned decision owner; confidence: medium; commits: 3e29916e656e, 62db2dd26d40; files: src/agents/openai-transport-stream.test.ts, src/auto-reply/reply/dispatch-from-config.test.ts, src/gateway/server-methods/agent.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete force-pushed the codex/split-large-test-files branch from 62db2dd to c8f6096 Compare July 13, 2026 12:59

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +230 to +232
// 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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@steipete
steipete force-pushed the codex/split-large-test-files branch from c8f6096 to d08642d Compare July 13, 2026 13:34
@steipete
steipete merged commit 507038c into main Jul 13, 2026
111 checks passed
@steipete
steipete deleted the codex/split-large-test-files branch July 13, 2026 14:01
@steipete

Copy link
Copy Markdown
Contributor Author

Merged via squash.

wm0018 pushed a commit to wm0018/openclaw that referenced this pull request Jul 14, 2026
* 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
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 14, 2026
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling extensions: policy gateway Gateway runtime maintainer Maintainer-authored PR P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XL status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: split oversized test suites

1 participant