Skip to content

test: remove 47 low-value CLI and runtime tests - #34451

Merged
OrcaWin merged 1 commit into
mainfrom
test-audit-606-cli-runtime
Oct 27, 2026
Merged

OrcaWin merged 1 commit into
mainfrom
test-audit-606-cli-runtime

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 11 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2 $\color{#cf222e}{\Huge{\mathbf{−}}}$​950 $\color{#cf222e}{\Huge{\mathbf{−}}}$​948
Prod 0 0 0 0

Removes 47 low-value CLI and runtime tests: mock-supplied installation/removal results, source-string inventories, duplicate queue/helper replays, and focus checks that only inspect no-op spies. This removes 948 lines across 11 test files.

Retains real generated-shell rollback, filesystem ownership races, registry persistence, dirty-buffer projection, and connected DOM focus checks. All 46 retained registrations and callbacks are unchanged. Formatting, lint, discovery and static checks passed; runtime validation runs in CI. No local runtime tests were run.

@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Walkthrough

The pull request removes selected tests and related unused imports or fixtures from CLI and renderer runtime test files. The supplied summaries report no implementation changes. Other tests remain in the affected files.

Priority: Low

Merge Risk: Low · up to 0033d

The launcher behavior is unchanged, but a useful check for whether bare orca can execute has been removed. Restoring that assertion is worthwhile; the coverage gap alone does not block merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description follows the template's main sections: an LoC summary, what was removed and retained, and validation notes. It explains the change and the test scope. Several template sections are miss… Add the missing template content. Fill in ELI5 with a plain-language summary. Fill in Why with the reason for removing these tests and why removal is preferred over alternatives such as fixing or rewriting them. Fill in Linked Issue with an…
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title "test: remove 47 low-value CLI and runtime tests" matches the main change. The raw summary shows removals across the CLI and renderer runtime test files, and the PR objectives describe the s…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi…
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: Title check

Explanation

The title "test: remove 47 low-value CLI and runtime tests" matches the main change. The raw summary shows removals across the CLI and renderer runtime test files, and the PR objectives describe the same 47 removed tests. The title is specific and follows a conventional commit style. It does not list every file, which is acceptable.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. 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 0 functions across 1 files.

Full details: Description check

Explanation

The description follows the template's main sections: an LoC summary, what was removed and retained, and validation notes. It explains the change and the test scope. Several template sections are missing or left unfilled: ELI5, Why (rationale for removing these tests over alternatives), Linked Issue (required for outside contributors), Testing checkboxes and platform details, the Agent skill upstream boundary checkbox, the Checklist, and the Visual Proof section (should state N/A with reason). The author also states that no local runtime tests were run, which the Testing section should address.

Resolution

Add the missing template content. Fill in ELI5 with a plain-language summary. Fill in Why with the reason for removing these tests and why removal is preferred over alternatives such as fixing or rewriting them. Fill in Linked Issue with an issue reference or confirm maintainer status. Fill in Visual Proof with N/A and a short reason. Complete the Testing section with the steps taken and the platforms tested, and mark the checkboxes that apply. Complete the Agent skill upstream boundary checkbox. Complete the Checklist, and state the AI Disclosure if applicable. The description is largely incomplete against the template, so this check does not pass.

Finishing Touches
📝 Generate docstrings
  • 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

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

@coderabbitai coderabbitai 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.

Nitpick comments: 1
src/main/cli/linux-bare-orca-dispatcher.test.ts: 1

84-84: Trivial · Functional Correctness · Quick win

Retain the executable-permission assertion.

The retained successful-install test checks only the dispatcher marker. A non-executable dispatcher can pass that test even though bare orca cannot run. This assertion covers a distinct launcher contract and is not duplicate mock or predicate coverage.

Suggested fix
-import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'
+import { mkdir, mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises'
...
     await expect(readFile(result.dispatcherPath, 'utf8')).resolves.toContain(
       '# orca-serve-bare-orca-dispatcher'
     )
+    const mode = (await stat(result.dispatcherPath)).mode & 0o777
+    expect(mode & 0o111).not.toBe(0)

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: a481596f-7456-45a9-9489-9295399638ec
Commits

Reviewing files that changed from the base of the PR and between 7140e7f and 0033d44.

Files selected for processing: 11
  • src/main/cli/linux-bare-orca-dispatcher.test.ts
  • src/main/cli/orca-cli-child-path.test.ts
  • src/main/cli/wsl-cli-installer.test.ts
  • src/main/cli/wsl-cli-login-path.test.ts
  • src/main/cli/wsl-cli-registration-operation.test.ts
  • src/main/cli/wsl-cli-registration-reconciliation.test.ts
  • src/main/cli/wsl-cli-registration-registry.test.ts
  • src/renderer/src/runtime/close-mirrored-editor-tab.test.ts
  • src/renderer/src/runtime/editor-buffer-dirty-readers.test.ts
  • src/renderer/src/runtime/focus-runtime-terminal-surface-chat-view.test.ts
  • src/renderer/src/runtime/focus-runtime-terminal-surface-readiness.test.ts
Files with no reviewable changes: 10
  • src/renderer/src/runtime/editor-buffer-dirty-readers.test.ts
  • src/renderer/src/runtime/focus-runtime-terminal-surface-readiness.test.ts
  • src/renderer/src/runtime/close-mirrored-editor-tab.test.ts
  • src/main/cli/wsl-cli-registration-operation.test.ts
  • src/main/cli/wsl-cli-registration-reconciliation.test.ts
  • src/renderer/src/runtime/focus-runtime-terminal-surface-chat-view.test.ts
  • src/main/cli/wsl-cli-login-path.test.ts
  • src/main/cli/wsl-cli-installer.test.ts
  • src/main/cli/orca-cli-child-path.test.ts
  • src/main/cli/wsl-cli-registration-registry.test.ts

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

@OrcaWin
OrcaWin merged commit 93a0650 into main Oct 27, 2026
33 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