Repository navigation
test(bot): phase-2 cleanup batch 1 — analyzer + queueStateManager (-21 tests) - #836
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
LucasSantana-Dev has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Failed to generate code suggestions for PR |
Records the Phase 5 decisions from /fix-the-suite: - Adopt Lucky-specific ≤1,500 test target (full-stack 39k LOC scaled 2.6× from skill table, not the ≤30-commands 50–200 bracket). - Pin coverage floor at 65/60/60/65 (round-down of post-#821 baseline). - Cleanup proceeds in single-file gate-checked batches; deletion only when no distinct branch is exercised. it.each consolidates LOC but not jest's reported test count. - Defer mutation testing (Stryker not installed); safety rests on preserved (input → expected) pairs and unchanged coverage numbers. Phase-2 batch 1 (PR #836) shipped under this strategy: 2 files, −21 tests, −1097 LOC, gate held at 67/63/64/68. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
LucasSantana-Dev has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
LucasSantana-Dev has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
LucasSantana-Dev has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
LucasSantana-Dev has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
LucasSantana-Dev has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
…nt negatives - Collapse 13 individual error-detection it() blocks into one it.each table (one row per category × match style); positive cases preserved. - Replace 6 identical 'does not detect X' negatives with one 'returns all-false flags for unrelated error' assertion (saves 5 tests). - Replace 6 near-identical getErrorResponse describes (full YouTubeErrorInfo literal each) with one it.each table + shared infoWith() factory. - Fold 6 'should prioritize X' tests into one assertion that walks the full priority chain (saves 5 tests). - Replace 4 integration scenarios with one it.each (one row per scenario). Net: 56 → 41 tests (-15) on this file. Bot suite: 2848 → 2833. Coverage unchanged at 67/63/64/68 — gate (65/60/60/65) holds with same headroom. Refs .agents/plans/test-cleanup-phase2.md
- isQueueEmpty: 4 tests → 1 it.each (3 rows) - isQueueFull: 7 tests → 1 it.each (7 rows, default-arg covered inline) - getQueueState duration coercion: 3 tests → 1 it.each (3 rows) - getQueueState position fallback: 3 tests → 1 it.each (3 rows) - getQueueStats: collapse separate 'unique artists' + 'empty when no author' + 'deduplicate' into one comprehensive test - getTrackAtPosition: 6 position-validity tests → 1 it.each (6 rows) - isTrackInQueue: 3 match/no-match tests → 1 it.each (3 rows) - getTrackPosition: 5 lookup tests → 1 it.each (5 rows) - Extract withTracks/withTracksThrow helpers to remove repeated toArray.mockReturnValue/mockImplementation boilerplate. Net: 59 → 53 tests (-6) on this file. Full bot suite: 2833 → 2827. Coverage unchanged at 67/63/64/68 — gate (65/60/60/65) holds. Refs .agents/plans/test-cleanup-phase2.md
Records the Phase 5 decisions from /fix-the-suite: - Adopt Lucky-specific ≤1,500 test target (full-stack 39k LOC scaled 2.6× from skill table, not the ≤30-commands 50–200 bracket). - Pin coverage floor at 65/60/60/65 (round-down of post-#821 baseline). - Cleanup proceeds in single-file gate-checked batches; deletion only when no distinct branch is exercised. it.each consolidates LOC but not jest's reported test count. - Defer mutation testing (Stryker not installed); safety rests on preserved (input → expected) pairs and unchanged coverage numbers. Phase-2 batch 1 (PR #836) shipped under this strategy: 2 files, −21 tests, −1097 LOC, gate held at 67/63/64/68. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses pr-test-analyzer findings: - queueStateManager.spec.ts: restore exact averageDuration assertion (was toBeGreaterThan(0) — mutation-survivable) Changed: expect(stats.averageDuration).toBeCloseTo(206666.66666666666, 5) (620000/3 = 206666.666... exact float) - Added dedicated edge case test for equal-duration tracks (3 × 200000ms → 200000 exact average) - analyzer.spec.ts: add explicit negative-flags rows to it.each table so "set X without setting Y/Z" is parameterized coverage (3 rows) Per ADR 2026-05-09: coverage gate (65/60/60/65) held — see PR comment with text-summary output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bf21007 to
0aafb2f
Compare
There was a problem hiding this comment.
LucasSantana-Dev has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Coverage gate verification after restoring exact assertions + adding negative-flags rows: |
There was a problem hiding this comment.
LucasSantana-Dev has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
LucasSantana-Dev has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
LucasSantana-Dev has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
…1 tests) (#836) * test(youtube): consolidate analyzer.spec via it.each + remove redundant negatives - Collapse 13 individual error-detection it() blocks into one it.each table (one row per category × match style); positive cases preserved. - Replace 6 identical 'does not detect X' negatives with one 'returns all-false flags for unrelated error' assertion (saves 5 tests). - Replace 6 near-identical getErrorResponse describes (full YouTubeErrorInfo literal each) with one it.each table + shared infoWith() factory. - Fold 6 'should prioritize X' tests into one assertion that walks the full priority chain (saves 5 tests). - Replace 4 integration scenarios with one it.each (one row per scenario). Net: 56 → 41 tests (-15) on this file. Bot suite: 2848 → 2833. Coverage unchanged at 67/63/64/68 — gate (65/60/60/65) holds with same headroom. Refs .agents/plans/test-cleanup-phase2.md * test(queue): consolidate queueStateManager.spec via it.each tables - isQueueEmpty: 4 tests → 1 it.each (3 rows) - isQueueFull: 7 tests → 1 it.each (7 rows, default-arg covered inline) - getQueueState duration coercion: 3 tests → 1 it.each (3 rows) - getQueueState position fallback: 3 tests → 1 it.each (3 rows) - getQueueStats: collapse separate 'unique artists' + 'empty when no author' + 'deduplicate' into one comprehensive test - getTrackAtPosition: 6 position-validity tests → 1 it.each (6 rows) - isTrackInQueue: 3 match/no-match tests → 1 it.each (3 rows) - getTrackPosition: 5 lookup tests → 1 it.each (5 rows) - Extract withTracks/withTracksThrow helpers to remove repeated toArray.mockReturnValue/mockImplementation boilerplate. Net: 59 → 53 tests (-6) on this file. Full bot suite: 2833 → 2827. Coverage unchanged at 67/63/64/68 — gate (65/60/60/65) holds. Refs .agents/plans/test-cleanup-phase2.md * docs(adr): bot test suite cleanup strategy and proportionality target Records the Phase 5 decisions from /fix-the-suite: - Adopt Lucky-specific ≤1,500 test target (full-stack 39k LOC scaled 2.6× from skill table, not the ≤30-commands 50–200 bracket). - Pin coverage floor at 65/60/60/65 (round-down of post-#821 baseline). - Cleanup proceeds in single-file gate-checked batches; deletion only when no distinct branch is exercised. it.each consolidates LOC but not jest's reported test count. - Defer mutation testing (Stryker not installed); safety rests on preserved (input → expected) pairs and unchanged coverage numbers. Phase-2 batch 1 (PR #836) shipped under this strategy: 2 files, −21 tests, −1097 LOC, gate held at 67/63/64/68. * test(bot): restore exact assertions + negative-flags coverage Addresses pr-test-analyzer findings: - queueStateManager.spec.ts: restore exact averageDuration assertion (was toBeGreaterThan(0) — mutation-survivable) Changed: expect(stats.averageDuration).toBeCloseTo(206666.66666666666, 5) (620000/3 = 206666.666... exact float) - Added dedicated edge case test for equal-duration tracks (3 × 200000ms → 200000 exact average) - analyzer.spec.ts: add explicit negative-flags rows to it.each table so "set X without setting Y/Z" is parameterized coverage (3 rows) Per ADR 2026-05-09: coverage gate (65/60/60/65) held — see PR comment with text-summary output. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>



What
First batch of phase-2 test cleanup. Two files consolidated via
it.eachtables and removal of genuinely-redundant negatives. No behavior
changes, no source changes, just test rewrites.
Numbers
Coverage gate (65/60/60/65 from #835) holds with same headroom in every
dimension. Mutation behavior should be identical — same input/output
pairs are exercised, just tabularised.
Files
packages/bot/src/utils/music/youtubeErrorHandler/analyzer.spec.ts(56 → 41)it()blocks → oneit.eachtable(positive + case-insensitive match per category).
flags' assertion (saves 5).
getErrorResponsedescribes (fullYouTubeErrorInfoliteral each) → oneit.each+ sharedinfoWith()factory.priority chain (saves 5).
it.each.packages/bot/src/utils/music/queueStateManager.spec.ts(59 → 53)isQueueEmpty: 4 → 1it.each(3 rows).isQueueFull: 7 → 1it.each(default-arg case folded inline).getQueueStateduration coercion: 3 → 1it.each.getQueueStateposition fallback: 3 → 1it.each.getQueueStats: collapse 'unique' + 'empty author' + 'deduplicate'into one comprehensive test.
getTrackAtPosition: 6 → 1it.each(6 rows).isTrackInQueue: 3 → 1it.each(3 rows).getTrackPosition: 5 → 1it.each(5 rows).withTracks/withTracksThrowhelpers replace repeatedtoArray.mockReturnValueboilerplate.Why this is safe
row in an
it.eachtable — same cardinality, same assertions, samebranch coverage.
it()blocks asserted thesame predicate against different inputs that did not exercise
distinct branches (e.g., 'does not detect parser', 'does not detect
composite', 'does not detect hype' — all hit the same default
branch with no category match).
unreached.
Phase-2 progress
This is batch 1 of the cleanup pass tracked in
.agents/plans/test-cleanup-phase2.md. Follow-up batches will targetspotifyApi.spec.ts,lastFmApi.spec.ts, and the player-handlerfragmentation cluster, each as its own focused PR with the same
'gate-holds-or-restore' rule.