You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
test: remove 152 low-value test declarations - #38182
Remove low-value tests to reduce maintenance and CI work.
Files
Added
Deleted
Net
Test
43
$\color{#1a7f37}{\Huge{\mathbf{+}}}$13
$\color{#cf222e}{\Huge{\mathbf{−}}}$2718
$\color{#cf222e}{\Huge{\mathbf{−}}}$2705
Prod
2
$\color{#1a7f37}{\Huge{\mathbf{+}}}$29
$\color{#cf222e}{\Huge{\mathbf{−}}}$47
$\color{#cf222e}{\Huge{\mathbf{−}}}$18
What Changed
Remove 152 authored registrations from 60 reviewed main-process test sources: six whole suites, 37 partial suites and 17 unchanged suites. Preserve 223 registrations protecting meaningful ownership, cleanup, races, persistence and external behavior. Retained callbacks and tables stay exact; three files receive an EOF blank-line trim and 17 receive formatting cleanup.
Remove six obsolete timing rows and update four reliability gates. Archive one complete affected historical receipt and the unvalidated active selectors/assertion references. Preserve every unrelated receipt and prior gap. No production changes or equivalent replacement-coverage claim.
Why
Source and decisive-owner review identified private counters, self-derived expectations, telemetry wording and repeated mock registry characterization with little regression value. Intentional losses are recorded per registration; mocked I/O behavior with meaningful outcomes is retained.
Linked Issue
Maintainer-directed test audit.
Visual Proof
N/A: tests and inventories only; no production UI change.
Testing
All 24 final check:changed checks passed against the actual merged base.
50 ordinary roots, including both actual registration drivers: 1772 expanded assertions, 1769 passed and 3 platform-gated/skipped assertions recorded in the runtime report.
Four special roots passed all six retained cases: real POSIX process-tree teardown, packaged orcad CLI RPCs, real node-pty/zsh crash/ready behavior, and the install/restart/update Electron extension lifecycle.
Static review authenticated all current code bodies, preserved survivor callbacks/context, and found no deleted-suite importers or affected support orphans.
Tests/apps ran with background launch on isolated Xvfb. Packaged artifacts and native dependencies were verified; validation-local copies and caches preserve shared stores. Linux native validation only; no macOS/Windows/remote execution claim. The runtime receipts retain their original executed tree. The final candidate additionally inherits seven separately validated upstream renderer paths from the actual parent squash; selected registration/callback/wrapper bytes and owner, driver, resolver and metadata identities remain unchanged. The 17 formatting changes preserve complete source ASTs; final checks passed afterward. Authored registrations count once, independently of expanded runtime rows. Historical reliability claims are not promoted by these runs.
AI Disclosure
Internal maintainer-directed change.
Review
Self-reviewed and independently value-reviewed; final parent static acceptance recorded. Full PR CI must pass before merge.
Agent skill upstream boundary
Not applicable.
Checklist
Focused on reviewed tests and their inventories.
Intentional coverage losses recorded; no production behavior changed.
Cross-platform, remote, mobile and folder-workspace impact considered.
Local checks passed; full PR CI required before merge.
This change removes or reduces test coverage across agent sessions, browser routing, IPC, persistence, plugins, Git, and other areas. It also updates reliability-gate records to document selected coverage losses and removes six test-duration entries from CI shard timings. One repository default-branch test is added for an unavailable SSH provider. No production implementation behavior changes.
Merge Risk: Low · up to 387fd
The change reduces protection against future terminal ownership and worktree pruning regressions. It is mergeable with awareness of these coverage gaps.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title "test: remove 152 low-value test declarations" matches the main change. The PR removes 152 authored test registrations across 60 main-process test sources, and the summary and objectives des…
Description check
The description fills in the template's main sections: ELI5, What Changed, Why, Testing, Risk-adjacent notes, AI Disclosure, Review, upstream boundary, and checklist. It explains the scope (152 remove…
Docstring Coverage
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
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title "test: remove 152 low-value test declarations" matches the main change. The PR removes 152 authored test registrations across 60 main-process test sources, and the summary and objectives describe the same scope. It is specific and concise, and it has no file lists or vague terms. The title does not mention the timing rows or reliability-gate metadata edits, which are secondary.
Full details: Description check
Explanation
The description fills in the template's main sections: ELI5, What Changed, Why, Testing, Risk-adjacent notes, AI Disclosure, Review, upstream boundary, and checklist. It explains the scope (152 removed registrations, 223 retained, six timing rows, four reliability gates) and gives a Testing summary with Linux-only validation. It has gaps. The Linked Issue section reads "Maintainer-directed test audit" and has no issue reference, which the template says should always exist. The Visual Proof section is N/A with a reason. The Risk section is missing: there is no 'Level' selection (Low/Medium/High), and it does not say how to back out the change. The Testing checkboxes are not ticked, and the 'Every added test fails without this change' item is not addressed. The Review section is filled in, but the 'Ensure no issues in' notes are not addressed explicitly for security, SSH, or mobile impact beyond a brief cross-platform note. The template's Linked Issue and Risk requirements are not met, but the description is mostly complete overall.
Resolution
Add the Risk section with a Level (Low, Medium, or High), a short statement of what could break, and how to revert. Replace the Linked Issue placeholder with an actual issue reference, or state why none exists. Tick or address the Testing checkboxes, including the test-anti-pattern item. Note the Notes/checklist items for security and SSH impact explicitly.
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 13 files. (1 skipped: 1 unsupported.)
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.
Restore boundary cases for isForeignLink in the prune guard tests.
isForeignLink decides which worktree entries receive protective locks before pruning. The removed matrix was the only direct test of that classifier. The remaining prune tests do not cover relative links, a //server/share link under Windows Git, or a \\?\ extended path under WSL Git. A future change to the classifier can break these cases without a failing test. Restore the four uncovered rows as a small table:
Suggested test restoration
-import { PRUNE_GUARD_LOCK_REASON, pruneGitWorktrees } from './worktree-prune-guard'+import { isForeignLink, PRUNE_GUARD_LOCK_REASON, pruneGitWorktrees } from './worktree-prune-guard'
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
ELI5
Remove low-value tests to reduce maintenance and CI work.
What Changed
Remove 152 authored registrations from 60 reviewed main-process test sources: six whole suites, 37 partial suites and 17 unchanged suites. Preserve 223 registrations protecting meaningful ownership, cleanup, races, persistence and external behavior. Retained callbacks and tables stay exact; three files receive an EOF blank-line trim and 17 receive formatting cleanup.
Remove six obsolete timing rows and update four reliability gates. Archive one complete affected historical receipt and the unvalidated active selectors/assertion references. Preserve every unrelated receipt and prior gap. No production changes or equivalent replacement-coverage claim.
Why
Source and decisive-owner review identified private counters, self-derived expectations, telemetry wording and repeated mock registry characterization with little regression value. Intentional losses are recorded per registration; mocked I/O behavior with meaningful outcomes is retained.
Linked Issue
Maintainer-directed test audit.
Visual Proof
N/A: tests and inventories only; no production UI change.
Testing
Tests/apps ran with background launch on isolated Xvfb. Packaged artifacts and native dependencies were verified; validation-local copies and caches preserve shared stores. Linux native validation only; no macOS/Windows/remote execution claim. The runtime receipts retain their original executed tree. The final candidate additionally inherits seven separately validated upstream renderer paths from the actual parent squash; selected registration/callback/wrapper bytes and owner, driver, resolver and metadata identities remain unchanged. The 17 formatting changes preserve complete source ASTs; final checks passed afterward. Authored registrations count once, independently of expanded runtime rows. Historical reliability claims are not promoted by these runs.
AI Disclosure
Internal maintainer-directed change.
Review
Self-reviewed and independently value-reviewed; final parent static acceptance recorded. Full PR CI must pass before merge.
Agent skill upstream boundary
Not applicable.
Checklist