Skip to content

test: convert script-style test files to node:test blocks - #428

Merged
Waishnav merged 46 commits into
test/trim-unit-fakesfrom
test/node-test
Oct 27, 2026
Merged

Waishnav merged 46 commits into
test/trim-unit-fakesfrom
test/node-test

Conversation

@Waishnav

@Waishnav Waishnav commented Oct 27, 2026 •

Copy link
Copy Markdown
Owner

Stacked on #425.

46 of the test files were top-level scripts: chains of assert calls with no test() blocks. When one assertion failed, the file crashed and reported no test name, every later scenario in the file was skipped, and scenarios shared mutable state. For example, local-agent-manager.test.ts created a stale record around line 182 and only checked it around line 432. The ACP and OpenCode tests counted protocol calls on a request log that earlier scenarios had already written to.

Every file is now a set of named node:test blocks with its own fixtures and t.after cleanup. Patched HOME, env, and Date.now are restored after each test. The suite goes from 209 to 552 named tests. Where a call-count assertion depended on calls made by an earlier scenario, that test now makes those calls itself, so the count still checks the same thing. Steps that genuinely build on each other stay in one test: the dev-state seed/reset flow, the Pi pool lifecycle, and the CLI package-install smoke. The Pi sandbox's shared try block is split into six tests, so one boundary failure no longer hides the others. It still fails when DEVSPACE_REQUIRE_PI_SANDBOX=1 is set and the sandbox is unavailable, and skips otherwise.

Assertions are unchanged. Per-file assertion counts are equal, except three tool-presence preconditions now repeated in the tests that rely on them. Timing values are untouched. Every test was also run alone with --test-name-pattern to confirm it passes on its own.

Coverage is unchanged file by file: each converted file lost zero lines against its own baseline. The full-suite comparison reports five lines in src/local-agent-manager.ts (264–268), but that is an artifact of Node's coverage merge, not lost coverage. None of the three test files that touch that code reaches those lines when run alone, before or after this change, yet merging any two of them reports a hit.

Most of the diff is re-indentation; git diff -w is +2052/−1017.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Reorganized the test suite into independently reported test cases, improving clarity when identifying failures.
    • Added isolated temporary-file setup and cleanup for tests, with platform-specific checks and skips where needed.
    • Existing test scenarios and expected behavior remain unchanged.

Waishnav and others added 30 commits October 27, 2026 12:09
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…st blocks

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…locks

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…st blocks

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…locks

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ocks

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Waishnav and others added 16 commits October 27, 2026 12:09
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 01004cb6-3fe8-4e4d-b287-c4dc9bc1b767
Commits

Reviewing files that changed from the base of the PR and between c7be3f7 and 5fecbdc.

Files selected for processing: 46
  • src/agents-skill.test.ts
  • src/apply-patch.test.ts
  • src/artifact-download.test.ts
  • src/bin-launcher.test.ts
  • src/cli-workspace.test.ts
  • src/cli.test.ts
  • src/config-schema.test.ts
  • src/config.test.ts
  • src/dev-state.test.ts
  • src/incoming-artifacts.test.ts
  • src/local-agent-acp.test.ts
  • src/local-agent-adapters.test.ts
  • src/local-agent-availability.test.ts
  • src/local-agent-claude.test.ts
  • src/local-agent-codex.test.ts
  • src/local-agent-config.test.ts
  • src/local-agent-daemon-lifecycle.test.ts
  • src/local-agent-daemon.test.ts
  • src/local-agent-events.test.ts
  • src/local-agent-grok.test.ts
  • src/local-agent-manager.test.ts
  • src/local-agent-opencode-v2.test.ts
  • src/local-agent-opencode-version.test.ts
  • src/local-agent-opencode.test.ts
  • src/local-agent-pi-rpc.test.ts
  • src/local-agent-pi-sandbox.test.ts
  • src/local-agent-pi.test.ts
  • src/local-agent-presentation.test.ts
  • src/local-agent-profiles.test.ts
  • src/local-agent-runtime.test.ts
  • src/local-agent-targets.test.ts
  • src/local-agent-worktree.test.ts
  • src/oauth-refresh-grace.test.ts
  • src/oauth-store.test.ts
  • src/onboarding.test.ts
  • src/process-platform.test.ts
  • src/process-sessions.test.ts
  • src/roots.test.ts
  • src/server-shutdown.test.ts
  • src/skills.test.ts
  • src/user-config.test.ts
  • src/workflow-cli.test.ts
  • src/workflow-runner.test.ts
  • src/workflow-schema.test.ts
  • src/workflow-script.test.ts
  • test/package-install-smoke.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

The pull request converts existing script-style checks into named node:test cases across the test suite. It adds isolated fixtures and test-runner cleanup hooks in several suites, and registers platform-specific checks with skip options.

Changes: Test Runner Migration

Layer / File(s) Summary
Utility and artifact test suites
src/agents-skill.test.ts, src/apply-patch.test.ts, src/artifact-download.test.ts, src/bin-launcher.test.ts, src/incoming-artifacts.test.ts, src/skills.test.ts, test/package-install-smoke.test.ts
Registers existing assertions as named tests. Temporary directories use test-context cleanup where needed, and unsupported platform download checks use skip options.
CLI and configuration test suites
src/cli-workspace.test.ts, src/cli.test.ts, src/config*.test.ts, src/dev-state.test.ts, src/onboarding.test.ts, src/roots.test.ts, src/user-config.test.ts, src/workflow-cli.test.ts
Separates CLI, configuration, onboarding, root-resolution, migration, and workflow CLI checks into named tests. Shared setup and cleanup use fixtures or runner hooks.
Local agent provider and presentation tests
src/local-agent-acp.test.ts, src/local-agent-adapters.test.ts, src/local-agent-availability.test.ts, src/local-agent-claude.test.ts, src/local-agent-codex.test.ts, src/local-agent-config.test.ts, src/local-agent-events.test.ts, src/local-agent-grok.test.ts, src/local-agent-opencode*.test.ts, src/local-agent-pi*.test.ts, src/local-agent-presentation.test.ts, src/local-agent-profiles.test.ts, src/local-agent-targets.test.ts
Organizes existing provider, adapter, runtime, availability, and presentation checks as named tests. Several suites use per-test fixtures and cleanup hooks.
Agent manager, daemon, and worktree tests
src/local-agent-manager.test.ts, src/local-agent-daemon*.test.ts, src/local-agent-runtime.test.ts, src/local-agent-worktree.test.ts
Moves manager, daemon, runtime, and worktree checks into isolated test cases. Fixtures create scoped state and register cleanup with the test runner.
Process, persistence, and workflow tests
src/process-*.test.ts, src/oauth-*.test.ts, src/server-shutdown.test.ts, src/workflow-runner.test.ts, src/workflow-schema.test.ts, src/workflow-script.test.ts
Registers process, OAuth, shutdown, and workflow checks as Node tests. Test-scoped cleanup replaces several shared temporary roots and try/finally blocks.

Priority: Low

Estimated code review effort: 3 (Moderate) | ~30 minutes

Change: Refactor

Possibly related PRs

  • Waishnav/devspace#423: Both PRs convert script-style tests to Node test-runner cases in overlapping parts of the suite.
  • Waishnav/devspace#425: Both PRs restructure the test suite, with this PR converting remaining checks and adding test-scoped fixtures.

Suggested reviewers: kotovasia5120, jplew, zhenyu98

Merge Risk: Minimal · up to 5fecb

This change reorganizes tests into named blocks with per-test cleanup and does not alter product behavior. No concrete merge-blocking issue was found.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 17.07% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: converting script-style tests into named node:test blocks.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 17.07% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 41 functions across 46 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 2
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks each case by name,
And gives each test its proper frame.
Fresh roots are cleared when tests are done,
While skipped checks await their run.
The suite now hops from start to end,
With tidy fixtures as its friend.

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

🚀 Deploying Preview to Cloudflare 🚀

Preview Deployments by commit

Status Deployment URL Commit Updated (UTC) See this deployment's details
  • Build: Failed ❌

View logs ↗
5fecbdc 2026-10-27T06:48:26.246Z View logs ↗

@Waishnav
Waishnav added this pull request to stack #426 October 27, 2026 18:26
@Waishnav
Waishnav merged commit c3d9ef6 into main Oct 27, 2026
4 of 5 checks passed
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