Skip to content

test: Remove duplicate and theatrical tests#2426

Merged
la14-1 merged 1 commit intomainfrom
qa/dedup-scanner
Mar 10, 2026
Merged

test: Remove duplicate and theatrical tests#2426
la14-1 merged 1 commit intomainfrom
qa/dedup-scanner

Conversation

@la14-1
Copy link
Copy Markdown
Member

@la14-1 la14-1 commented Mar 10, 2026

Summary

  • Removed duplicate getSpawnDir (8 tests) and getHistoryPath (1 test) describe blocks from history.test.ts — these were already covered in paths.test.ts
  • Moved 4 unique test cases from the duplicates into paths.test.ts (dot-relative path, .. segment resolution, outside-home rejection, home-as-SPAWN_HOME acceptance)
  • Net result: -64 lines, +21 lines, zero coverage loss

Scan results

  • Duplicate describe blocks: 1 found (getSpawnDir + getHistoryPath in history.test.ts vs paths.test.ts) — fixed
  • Bash-grep theatrical tests: 0 found
  • Always-pass conditional patterns: 1 found (fs-sandbox.test.ts line 55) — intentional guardrail test, left as-is
  • Excessive subprocess spawning: 0 found (ssh-keys.test.ts uses spyOn(Bun, "spawnSync") mocks, not real subprocesses)

Test plan

  • bun test — 1478 tests pass, 0 failures
  • biome check — 0 errors

-- qa/dedup-scanner

Copy link
Copy Markdown
Member

@louisgv louisgv left a comment

Choose a reason for hiding this comment

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

Security Review

Verdict: APPROVED
Commit: af4291e

Summary

This PR removes duplicate test coverage from history.test.ts and consolidates path validation tests in paths.test.ts. The changes improve code organization without introducing any security concerns.

Changes

  • Removed: 63 lines of duplicate getSpawnDir() and getHistoryPath() tests from history.test.ts
  • Added: 5 missing test cases to paths.test.ts (dot-relative paths, .. segments, path traversal, etc.)

Security Findings

No security issues found.

  • No shell scripts modified
  • No credential handling changes
  • No command injection vectors
  • No path traversal vulnerabilities introduced
  • Actually improves test coverage for path validation security (consolidates all path security tests in one place)

Tests

  • bun test: ✅ PASS (54 tests, 0 failures)
  • bash -n: ✅ N/A (no shell scripts modified)
  • curl|bash safety: ✅ N/A (no shell scripts modified)
  • macOS compat: ✅ N/A (no shell scripts modified)

Recommendation

APPROVED - Clean test refactoring with improved organization. Ready to merge.


-- security/pr-reviewer

…test.ts

These path-utility tests were duplicated between history.test.ts and
paths.test.ts. Consolidate into paths.test.ts (the canonical location)
and move 4 unique test cases (dot-relative path, .. resolution, outside
home rejection, home-as-SPAWN_HOME) that only existed in history.test.ts.

Removes 64 lines of duplicate test code with zero coverage loss.
@la14-1 la14-1 force-pushed the qa/dedup-scanner branch from c39b480 to ac26ac5 Compare March 10, 2026 09:34
@la14-1 la14-1 merged commit 73ab90f into main Mar 10, 2026
5 checks passed
@la14-1 la14-1 deleted the qa/dedup-scanner branch March 10, 2026 09:35
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.

2 participants