Skip to content

test: remove low-value capability, copy, and source inventories - #36114

Merged
OrcaWin merged 1 commit into
mainfrom
test-audit-801-low-value-test-inventories
Oct 29, 2026
Merged

OrcaWin merged 1 commit into
mainfrom
test-audit-801-low-value-test-inventories

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Oct 29, 2026 •

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

Remove 20 authored tests (27 expanded cases) from eight audited suites. Delete the published-capability inventory, installed Vitest source-text check, and worker receipt wording suite entirely; trim default-value, label, locale inventory, and duplicate schema examples from five others.

Keep 10 authored tests (11 expanded cases), with their callbacks and setup unchanged. Remove only the two timing entries for deleted suites. No production changes.

Validation: authenticated static reconstruction, retained-callback checks, formatting/lint receipts, and final diff review. Full PR CI must pass before merge. Removed copy and inventory alarms are intentionally lost; no equivalent replacement coverage is claimed.

@greptile-apps

greptile-apps Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] This test-only cleanup appears safe to merge once the required CI checks pass.

Summary

Removes 20 authored tests from eight suites and deletes two matching timing entries. No production code changes.

  • The published-capability test no longer freezes runtime names.
  • Vitest no longer has a source-text check for its frame patch.
  • Worker-start receipts are no longer pinned to full sentences.
  • The desktop bridge test no longer checks its error message against settings labels.
  • Chat and terminal parity tests stop scanning session-ID wording.
  • RPC schema tests drop browser and older mobile payload examples.
  • The WSL form test drops distro, move-copy, and locale checks.
  • Shared settings tests drop terminal and platform paste defaults.

Reviews (1) · Last reviewed commit: "test: remove low-value test inventories ..." · Reviewed by Greptile

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Walkthrough

This change removes test assertions across runtime, interface, shared-constant, and test-tooling areas. It also removes two test-duration entries from the CI shard timing file. Some test files retain other tests. No implementation code changes are described.

Priority: Low

Merge Risk: Low · up to 3a993

WSL selection and browser requests have less regression coverage. Restore focused checks or accept the coverage loss as follow-up.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description explains the scope and validation, but it omits most required template sections, including ELI5, What Changed, Why, Linked Issue, Visual Proof, Testing checklist details, Review, Notes… Add the required template sections. Include a plain-language ELI5, user-facing before and after details, the reason and alternatives considered, a linked issue or the applicable maintainer exception, Visual Proof set to N/A with a reason, t…
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the main change: removing low-value capability, copy, and source inventory tests.
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 with cyclomatic complexity of at least 2. Documentat…
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

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.

Full details: Description check

Explanation

The description explains the scope and validation, but it omits most required template sections, including ELI5, What Changed, Why, Linked Issue, Visual Proof, Testing checklist details, Review, Notes, and the final Checklist.

Resolution

Add the required template sections. Include a plain-language ELI5, user-facing before and after details, the reason and alternatives considered, a linked issue or the applicable maintainer exception, Visual Proof set to N/A with a reason, testing steps and platforms, AI disclosure when applicable, review and compatibility notes, and completed checklist items.

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.

Follow-up Comments

Nitpick Keep unit tests for already-added and stopped WSL distros.

4-4: Trivial · Functional Correctness · Quick win

Keep unit tests for already-added and stopped WSL distros.

The removed test was the only unit test of canAddWslDistro and wslDistroStateLabel. No remaining test covers an already-added distro, which must show Already a host and stay disabled in the picker and submit button. No remaining test covers a stopped distro that stays selectable. A regression in either path would not fail the unit suite.

The WSL 1 exclusion has partial end-to-end coverage in tests/e2e/wsl-host-fake-transport.spec.ts, which checks that Ubuntu is preselected because Legacy is WSL 1. That spec does not check the already-added or stopped cases. Restore focused cases for those two states. The broader locale and copy inventory tests can stay removed.

Open the source location

Nitpick Cover the reachable browser payloads, not an empty tabClose payload.

1-4: Trivial · Data Integrity & Integration · Quick win

Cover the reachable browser payloads, not an empty tabClose payload.

Current browser.tabClose callers pass { worktree: 'id:wt-1' }. The existing dispatcher test covers neither browser.tabClose nor page-only browser.tabSwitch. Add coverage for the reachable payloads.

Suggested fix
 import { TERMINAL_METHODS } from './methods/terminal'
 import { TERMINAL_ORPHAN_METHODS } from './methods/terminal-orphan'
 import { WORKTREE_METHODS } from './methods/worktree'
+import { BROWSER_CORE_METHODS } from './methods/browser-core'

@@
     expectParses(methodParams(WORKTREE_METHODS, 'worktree.prefetchCreateBase'), { repo: 'repo-1' })
   })
 
+  it('accepts omitted browser optional fields for reachable requests', () => {
+    expectParses(methodParams(BROWSER_CORE_METHODS, 'browser.tabSwitch'), { page: 'page-1' })
+    expectParses(methodParams(BROWSER_CORE_METHODS, 'browser.tabClose'), {
+      worktree: 'id:wt-1'
+    })
+  })
+

Open the source location


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: 35fa0628-8ec9-4e1c-be60-8156ce37356f
Commits

Reviewing files that changed from the base of the PR and between 1e6b29a and 3a99373.

Files selected for processing: 9
  • config/scripts/ci-shard-timings.json
  • config/scripts/vitest-directory-frame-patch.test.mjs
  • src/main/ipc/desktop-browser-bridge-wiring.test.ts
  • src/main/runtime/orchestration/agent-facing-parity.test.ts
  • src/main/runtime/rpc/methods/orchestration-worker-start-receipt-wording.test.ts
  • src/main/runtime/rpc/schemas.test.ts
  • src/renderer/src/components/sidebar/AddWslHostFormPanel.test.tsx
  • src/shared/constants.test.ts
  • src/shared/protocol-version-published-capabilities.test.ts
Files with no reviewable changes: 7
  • config/scripts/vitest-directory-frame-patch.test.mjs
  • src/shared/protocol-version-published-capabilities.test.ts
  • src/main/runtime/rpc/methods/orchestration-worker-start-receipt-wording.test.ts
  • src/main/ipc/desktop-browser-bridge-wiring.test.ts
  • config/scripts/ci-shard-timings.json
  • src/shared/constants.test.ts
  • src/main/runtime/rpc/schemas.test.ts

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

@OrcaWin
OrcaWin merged commit fee06ab into main Oct 29, 2026
68 of 70 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