Skip to content

test: Remove duplicate boundary tests in history-trimming#2242

Merged
louisgv merged 1 commit intomainfrom
qa/dedup-scanner
Mar 6, 2026
Merged

test: Remove duplicate boundary tests in history-trimming#2242
louisgv merged 1 commit intomainfrom
qa/dedup-scanner

Conversation

@la14-1
Copy link
Copy Markdown
Member

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

Summary

  • Removed 2 duplicate tests from history-trimming.test.ts in the "sequential saves at the boundary" section that were exact copies of tests already in the "MAX_HISTORY_ENTRIES trimming" section:
    • "99 to 100 entries" == "should keep all entries when at exactly 100" (both: pre-populate 99 records, save 1 more, verify no trim)
    • "100 to 101 entries" == "should trim to 100 when adding entry that exceeds the limit" (both: pre-populate 100 records, save 1 more, verify trim)
  • Kept the unique "rapid sequential saves that build up from zero" test in the boundary section

Scan results

Scanned all 50 test files in packages/cli/src/__tests__/ for:

  • Duplicate describe blocks: No top-level describe names appear in multiple files. Files that test the same functions (e.g., security.test.ts, security-edge-cases.test.ts, security-encoding.test.ts) are intentionally split by scope (core, edge cases, encoding attacks).
  • Bash-grep tests: None found. All tests call actual functions with inputs and check outputs.
  • Always-pass patterns: No conditional expects that silently skip.
  • Excessive subprocess spawning: ssh-keys.test.ts uses spyOn(Bun, "spawnSync") (mocked, not real subprocesses). commands-update-download.test.ts mocks execSync/spawnSync. No actual subprocess spawning in tests.

Test plan

  • bun test passes (1415 tests, 0 failures)
  • biome check passes (0 errors)

-- qa/dedup-scanner

Remove two tests from "sequential saves at the boundary" that were
exact duplicates of tests in the "MAX_HISTORY_ENTRIES trimming" section:
- "99 to 100 entries" duplicated "should keep all entries when at exactly 100"
- "100 to 101 entries" duplicated "should trim to 100 when adding entry that exceeds the limit"

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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: 3c67e81

Findings

No security issues found. This PR removes duplicate test cases that were redundant with tests in the MAX_HISTORY_ENTRIES section.

Tests

  • bun test: PASS (30 tests, 0 failures)
  • bash -n: N/A (TypeScript file)
  • curl|bash: N/A (not a shell script)
  • macOS compat: N/A (not a shell script)

Summary

Clean refactoring that improves test suite maintainability by removing duplicate boundary tests. No production code affected.


-- security/pr-reviewer

@louisgv louisgv merged commit 435bc9c into main Mar 6, 2026
6 checks passed
@louisgv louisgv deleted the qa/dedup-scanner branch March 6, 2026 12:50
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