diff --git a/docs/decisions/2026-05-09-bot-test-suite-cleanup-strategy.md b/docs/decisions/2026-05-09-bot-test-suite-cleanup-strategy.md new file mode 100644 index 000000000..92bb93c42 --- /dev/null +++ b/docs/decisions/2026-05-09-bot-test-suite-cleanup-strategy.md @@ -0,0 +1,110 @@ +# ADR — Bot test suite cleanup strategy and proportionality target + +- **Date:** 2026-05-09 +- **Status:** Accepted (in progress) +- **Owner:** Lucas Santana +- **Related:** PR #835 (coverage threshold), PR #836 (phase-2 batch 1), + `.agents/plans/test-cleanup-phase2.md`, + `~/.claude/skills/test-cleanup/`, + `~/.claude/skills/fix-the-suite/` + +## Context + +The bot package's test suite had grown to **2,848 tests across 158 files +for 39,016 LOC of source**. The canonical `/test-cleanup` skill flagged +this as `BLOATED` and recommended targeting 50–200 tests per its +proportionality table. Three structural problems made that ceiling +unworkable as written: + +1. **Lucky is not the "≤30 commands, ~5k LOC" bot the table assumes.** The + bot has ~30+ slash commands across music/autoplay engine/Last.fm + scrobbling/Twitch integration/web music/automod/mod digest/giveaways/ + voterewards/levels/Spotify recommendations/guild automation. By app + type it is closer to the table's "full-stack 15k LOC → 200–600 tests" + row, scaled 2.6× by source size — putting the realistic ceiling at + **roughly 500–1,500 tests, not 50–200**. +2. **No `coverageThreshold` was configured.** The skill's per-batch + verification step ("if coverage drops below threshold, restore") had + no tripwire — pruning was effectively blind to coverage. +3. **The "obvious waste" patterns the skill targets (skipped tests, + SUT-self-mocks, filler-only `it()` blocks) were not present.** A + strict scan found 0 skipped, 0 SUT-self-mocks, and the 84 + `toBeDefined` / `toBeInstanceOf` calls were inline secondary + assertions inside otherwise-meaningful tests, not deletable + standalones. + +## Decision + +1. **Adopt a Lucky-specific test-count target of ≤ 1,500.** The 50–200 + bracket from the skill table does not apply. Future cleanup phases + target the lower end of 500–1,500 only when the suite shrinks + organically; aggressive deletion below 1,500 is not a goal. Tests + are deleted only when they exercise no distinct branch. +2. **Pin a coverage floor at the round-down of the post-#821 baseline:** + statements 65 / branches 60 / functions 60 / lines 65. Implemented + in `packages/bot/jest.config.cjs` via PR #835 and merged to + `release/v2.10.0` at `1778d144`. Tighten by 2-3 percentage points + per phase as the suite shrinks, never down. +3. **Cleanup proceeds in single-file, gate-checked batches.** Each + batch: + - Modifies one or two spec files only. + - Replaces same-shape `it()` clusters with `it.each` tables (note: + this consolidates LOC and improves maintainability but does NOT + reduce jest's reported test count — each row counts as one test). + - Deletes only tests that prove no distinct branch (e.g., 6 'does + not detect X' negatives all hitting the same default branch are + replaced with one 'returns all-false flags' assertion). + - Verifies the gate holds via + `npx jest --silent --coverage --coverageReporters=text-summary` + before commit. If coverage drops, restore the most-coverage + contributing test rather than the whole batch. +4. **Defer mutation testing.** Stryker is not currently installed in + this monorepo. Setting it up is its own project; in its absence the + safety argument for each batch rests on (a) input/output + preservation (every (input → expected) pair from the original tests + appears as an `it.each` row in the rewrite) and (b) unchanged + coverage numbers (same statements/branches/functions/lines reached). + Once the cleanup pass settles, install Stryker and run a one-off + mutation pass on the `autoplay/` and `lastfm/` modules to confirm + surviving tests are genuinely protective. + +## Consequences + +- **The original `/test-cleanup` skill cannot reach a "BLOATED → HEALTHY" + verdict on this repo without lowering its proportionality target for + full-stack-tier bots.** Filed back into + `~/.claude/skills/test-cleanup/` as a refinement: add a "polyglot + bot" row to the table (15-50k LOC, ~500-1500 tests) so the skill's + Step 1.5 conflict check produces a workable target instead of + flagging Lucky as 14× over. +- **Phase-2 cleanup is multi-PR work, not one big PR.** Batch 1 (PR + #836) shipped 2 files, −21 tests, −1097 LOC, gate held. Subsequent + batches will continue per the plan and merge independently. +- **Future contributors must not lower the coverage threshold to + accommodate aggressive deletion.** The threshold is the policy + artifact; the test count is a downstream consequence. Lower numbers + for the "right" reason (genuine duplication elimination) only. + +## Alternatives considered + +- **Aggressive deletion to ≤200 tests with replacement integration + tests for everything.** Rejected: the integration-test rewrite is a + multi-week project across the autoplay pipeline, player handlers, + scrobbler, and command registry — too much surface area to land + before v2.10.0 ships. Phase 2's per-file cleanup buys time without + blocking the release. +- **Disable the coverage threshold entirely and let CI pass with any + number.** Rejected: removes the only tripwire that prevents future + contributors from re-padding the suite or deleting too aggressively. +- **Pin the threshold at current numbers (67/63/64/68) instead of the + round-down.** Rejected: leaves zero headroom — the next legitimate + refactor that touches a low-coverage area would crash CI on every + push. The 2-3pp buffer is intentional. + +## References + +- Skill: `~/.claude/skills/fix-the-suite/SKILL.md` (composite workflow) +- Skill: `~/.claude/skills/test-cleanup/SKILL.md` (per-file pass) +- Skill: `~/.claude/skills/test-health/SKILL.md` (diagnostic baseline) +- Plan: `.agents/plans/test-cleanup-phase2.md` (per-batch targets) +- Memory: `~/.claude/projects/-Volumes-External-HD-Desenvolvimento-Lucky/memory/MEMORY.md` diff --git a/packages/bot/src/utils/music/queueStateManager.spec.ts b/packages/bot/src/utils/music/queueStateManager.spec.ts index cceeb129d..abe6f47c9 100644 --- a/packages/bot/src/utils/music/queueStateManager.spec.ts +++ b/packages/bot/src/utils/music/queueStateManager.spec.ts @@ -33,14 +33,12 @@ describe('queueStateManager', () => { author: 'Artist One', duration: 180000, } - mockTrack2 = { id: 'track-2', url: 'https://example.com/track2.mp3', author: 'Artist Two', duration: 240000, } - mockTrack3 = { id: 'track-3', url: 'https://example.com/track3.mp3', @@ -54,24 +52,30 @@ describe('queueStateManager', () => { isPlaying: jest.fn().mockReturnValue(true), isPaused: jest.fn().mockReturnValue(false), volume: 80, - getTimestamp: jest.fn().mockReturnValue({ - current: 30000, - }), + getTimestamp: jest.fn().mockReturnValue({ current: 30000 }), }, tracks: { size: 2, toArray: jest.fn().mockReturnValue([mockTrack2, mockTrack3]), }, - repeatMode: { - toString: jest.fn().mockReturnValue('OFF'), - }, + repeatMode: { toString: jest.fn().mockReturnValue('OFF') }, } as unknown as GuildQueue }) + // Tracks lookup-style describes share this fixture; populating tracks once + // here removes the per-describe beforeEach that just re-set toArray. + function withTracks(tracks: unknown[]): void { + ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue(tracks) + } + function withTracksThrow(): void { + ;(mockQueue.tracks?.toArray as jest.Mock).mockImplementation(() => { + throw new Error('Queue error') + }) + } + describe('getQueueState', () => { - it('should return current queue state with all properties', () => { + it('returns the current queue state with all properties', () => { const state = getQueueState(mockQueue) - expect(state).toEqual({ isPlaying: true, isPaused: false, @@ -84,83 +88,38 @@ describe('queueStateManager', () => { }) }) - it('should return undefined currentTrack when no current track exists', () => { + it('returns undefined currentTrack when no current track exists', () => { mockQueue.currentTrack = null - const state = getQueueState(mockQueue) - - expect(state.currentTrack).toBeUndefined() - expect(state.isPlaying).toBe(true) - }) - - it('should handle string duration by parsing to number', () => { - mockQueue.currentTrack = { - ...mockTrack, - duration: '180000', - } as unknown as Track - - const state = getQueueState(mockQueue) - - expect(state.duration).toBe(180000) + expect(getQueueState(mockQueue).currentTrack).toBeUndefined() }) - it('should handle numeric duration without conversion', () => { - mockQueue.currentTrack = { - ...mockTrack, - duration: 180000, - } as unknown as Track - - const state = getQueueState(mockQueue) - - expect(state.duration).toBe(180000) - }) - - it('should default duration to 0 when track has no duration', () => { + it.each([ + ['string', '180000', 180000], + ['numeric', 180000, 180000], + ['missing', undefined, 0], + ])('coerces %s duration on currentTrack', (_label, duration, expected) => { mockQueue.currentTrack = { id: 'track-1', author: 'Artist', + duration, } as unknown as Track - - const state = getQueueState(mockQueue) - - expect(state.duration).toBe(0) - }) - - it('should return 0 position when getTimestamp returns null', () => { - ;(mockQueue.node?.getTimestamp as jest.Mock).mockReturnValue(null) - - const state = getQueueState(mockQueue) - - expect(state.position).toBe(0) + expect(getQueueState(mockQueue).duration).toBe(expected) }) - it('should return 0 position when getTimestamp().current is null', () => { - ;(mockQueue.node?.getTimestamp as jest.Mock).mockReturnValue({ - current: null, - }) - - const state = getQueueState(mockQueue) - - expect(state.position).toBe(0) + it.each([ + ['getTimestamp returns null', () => null], + ['current is null', () => ({ current: null })], + ['current is non-numeric', () => ({ current: { valueOf: jest.fn().mockReturnValue('invalid') } })], + ])('returns position 0 when %s', (_label, factory) => { + ;(mockQueue.node?.getTimestamp as jest.Mock).mockReturnValue(factory()) + expect(getQueueState(mockQueue).position).toBe(0) }) - it('should handle position as non-number value', () => { - ;(mockQueue.node?.getTimestamp as jest.Mock).mockReturnValue({ - current: { valueOf: jest.fn().mockReturnValue('invalid') }, - }) - - const state = getQueueState(mockQueue) - - expect(state.position).toBe(0) - }) - - it('should return default state when exception occurs', () => { + it('returns the documented default state when an exception occurs and logs it', () => { ;(mockQueue.node?.isPlaying as jest.Mock).mockImplementation(() => { throw new Error('Queue error') }) - - const state = getQueueState(mockQueue) - - expect(state).toEqual({ + expect(getQueueState(mockQueue)).toEqual({ isPlaying: false, isPaused: false, queueSize: 0, @@ -175,201 +134,117 @@ describe('queueStateManager', () => { }) }) - it('should handle paused queue state', () => { + it('reports paused queue state', () => { ;(mockQueue.node?.isPlaying as jest.Mock).mockReturnValue(false) ;(mockQueue.node?.isPaused as jest.Mock).mockReturnValue(true) - const state = getQueueState(mockQueue) - expect(state.isPlaying).toBe(false) expect(state.isPaused).toBe(true) }) - it('should convert repeatMode to string', () => { - ;(mockQueue.repeatMode?.toString as jest.Mock).mockReturnValue( - 'QUEUE', - ) - - const state = getQueueState(mockQueue) - - expect(state.repeatMode).toBe('QUEUE') + it('stringifies repeatMode', () => { + ;(mockQueue.repeatMode?.toString as jest.Mock).mockReturnValue('QUEUE') + expect(getQueueState(mockQueue).repeatMode).toBe('QUEUE') }) }) describe('isQueueEmpty', () => { - it('should return true when queue has no tracks', () => { - mockQueue.tracks = { size: 0 } as unknown as any - - const isEmpty = isQueueEmpty(mockQueue) - - expect(isEmpty).toBe(true) - }) - - it('should return false when queue has tracks', () => { - mockQueue.tracks = { size: 2 } as unknown as any - - const isEmpty = isQueueEmpty(mockQueue) - - expect(isEmpty).toBe(false) - }) - - it('should return false when queue has exactly 1 track', () => { - mockQueue.tracks = { size: 1 } as unknown as any - - const isEmpty = isQueueEmpty(mockQueue) - - expect(isEmpty).toBe(false) - }) - - it('should return true when queue size is explicitly 0', () => { - mockQueue.tracks!.size = 0 - - const isEmpty = isQueueEmpty(mockQueue) - - expect(isEmpty).toBe(true) + it.each([ + [0, true], + [1, false], + [2, false], + ])('size %s → %s', (size, expected) => { + mockQueue.tracks = { size } as unknown as GuildQueue['tracks'] + expect(isQueueEmpty(mockQueue)).toBe(expected) }) }) describe('isQueueFull', () => { - it('should return false when queue size is below max', () => { - mockQueue.tracks!.size = 50 - const isFull = isQueueFull(mockQueue, 100) - - expect(isFull).toBe(false) - }) - - it('should return true when queue size equals max', () => { - mockQueue.tracks!.size = 100 - const isFull = isQueueFull(mockQueue, 100) - - expect(isFull).toBe(true) - }) - - it('should return true when queue size exceeds max', () => { - mockQueue.tracks!.size = 150 - const isFull = isQueueFull(mockQueue, 100) - - expect(isFull).toBe(true) - }) - - it('should use default max size of 100 when not provided', () => { - mockQueue.tracks!.size = 100 - const isFull = isQueueFull(mockQueue) - - expect(isFull).toBe(true) - }) - - it('should return false with default max when queue size is 99', () => { - mockQueue.tracks!.size = 99 - const isFull = isQueueFull(mockQueue) - - expect(isFull).toBe(false) - }) - - it('should work with custom max size', () => { - mockQueue.tracks!.size = 25 - const isFull = isQueueFull(mockQueue, 20) - - expect(isFull).toBe(true) - }) - - it('should work with small max size', () => { - mockQueue.tracks!.size = 1 - const isFull = isQueueFull(mockQueue, 1) - - expect(isFull).toBe(true) + it.each([ + [50, 100, false], + [99, undefined, false], // default max=100 + [100, 100, true], + [100, undefined, true], // default max=100 + [150, 100, true], + [25, 20, true], + [1, 1, true], + ])('size %s vs max %s → %s', (size, max, expected) => { + mockQueue.tracks!.size = size + const result = max === undefined + ? isQueueFull(mockQueue) + : isQueueFull(mockQueue, max) + expect(result).toBe(expected) }) }) describe('getQueueStats', () => { - it('should calculate total tracks and duration', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ + it('aggregates total tracks, total duration, and average for a multi-track queue', () => { + withTracks([ { duration: 180000, author: 'Artist One' }, { duration: 240000, author: 'Artist Two' }, { duration: 200000, author: 'Artist One' }, ]) - const stats = getQueueStats(mockQueue) - expect(stats.totalTracks).toBe(3) expect(stats.totalDuration).toBe(620000) + // 620000/3 = 206666.666... + expect(stats.averageDuration).toBeCloseTo(206666.66666666666, 5) }) - it('should calculate average duration', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ - { duration: 200000, author: 'Artist' }, - { duration: 200000, author: 'Artist' }, - { duration: 200000, author: 'Artist' }, - ]) - - const stats = getQueueStats(mockQueue) - - expect(stats.averageDuration).toBe(200000) - }) - - it('should handle string durations', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ + it('coerces string durations into the totals', () => { + withTracks([ { duration: '180000', author: 'Artist One' }, { duration: 240000, author: 'Artist Two' }, ]) - - const stats = getQueueStats(mockQueue) - - expect(stats.totalDuration).toBe(420000) + expect(getQueueStats(mockQueue).totalDuration).toBe(420000) }) - it('should extract unique artists', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ - { duration: 200000, author: 'Artist One' }, - { duration: 200000, author: 'Artist Two' }, - { duration: 200000, author: 'Artist One' }, + it('extracts unique artists and excludes empty/missing authors', () => { + withTracks([ + { duration: 100000, author: 'Artist A' }, + { duration: 100000, author: 'Artist B' }, + { duration: 100000, author: 'Artist A' }, // dedupe + { duration: 100000 }, // no author → excluded ]) - const stats = getQueueStats(mockQueue) - - expect(stats.artists).toContain('Artist One') - expect(stats.artists).toContain('Artist Two') - expect(stats.artists.length).toBe(2) + expect(stats.artists.sort()).toEqual(['Artist A', 'Artist B']) }) - it('should return empty artists array when tracks have no author', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ - { duration: 200000 }, - { duration: 200000 }, - ]) - - const stats = getQueueStats(mockQueue) - - expect(stats.artists).toEqual([]) + it('returns empty genres array (genres are not implemented yet)', () => { + withTracks([{ duration: 200000, author: 'Artist' }]) + expect(getQueueStats(mockQueue).genres).toEqual([]) }) - it('should return empty genres array (not implemented)', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ - { duration: 200000, author: 'Artist' }, - ]) - + it('returns 0 averageDuration for an empty queue', () => { + withTracks([]) const stats = getQueueStats(mockQueue) - - expect(stats.genres).toEqual([]) + expect(stats.averageDuration).toBe(0) + expect(stats.totalTracks).toBe(0) }) - it('should return 0 average duration for empty queue', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([]) - + it('handles a single track', () => { + withTracks([{ duration: 300000, author: 'Artist' }]) const stats = getQueueStats(mockQueue) - - expect(stats.averageDuration).toBe(0) - expect(stats.totalTracks).toBe(0) + expect(stats.totalTracks).toBe(1) + expect(stats.totalDuration).toBe(300000) + expect(stats.averageDuration).toBe(300000) }) - it('should return default stats on exception', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockImplementation(() => { - throw new Error('Queue error') - }) + it('aggregates equal-duration tracks with exact average', () => { + withTracks([ + { duration: 200000, author: 'Artist' }, + { duration: 200000, author: 'Artist' }, + { duration: 200000, author: 'Artist' }, + ]) const stats = getQueueStats(mockQueue) - - expect(stats).toEqual({ + expect(stats.totalTracks).toBe(3) + expect(stats.totalDuration).toBe(600000) + expect(stats.averageDuration).toBe(200000) + }) + it('returns the documented default stats on exception and logs it', () => { + withTracksThrow() + expect(getQueueStats(mockQueue)).toEqual({ totalTracks: 0, totalDuration: 0, averageDuration: 0, @@ -381,70 +256,25 @@ describe('queueStateManager', () => { error: expect.any(Error), }) }) - - it('should handle single track', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ - { duration: 300000, author: 'Artist' }, - ]) - - const stats = getQueueStats(mockQueue) - - expect(stats.totalTracks).toBe(1) - expect(stats.totalDuration).toBe(300000) - expect(stats.averageDuration).toBe(300000) - }) - - it('should deduplicate artists in set', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ - { duration: 100000, author: 'Artist A' }, - { duration: 100000, author: 'Artist A' }, - { duration: 100000, author: 'Artist A' }, - ]) - - const stats = getQueueStats(mockQueue) - - expect(stats.artists).toEqual(['Artist A']) - }) }) describe('getNextTrack', () => { - it('should return first track in queue', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ - mockTrack2, - mockTrack3, - ]) - - const nextTrack = getNextTrack(mockQueue) - - expect(nextTrack).toBe(mockTrack2) - }) - - it('should return null when queue is empty', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([]) - - const nextTrack = getNextTrack(mockQueue) - - expect(nextTrack).toBeNull() - }) - - it('should return single track when only one exists', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ - mockTrack2, - ]) - - const nextTrack = getNextTrack(mockQueue) - - expect(nextTrack).toBe(mockTrack2) - }) - - it('should return null on exception', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockImplementation(() => { - throw new Error('Queue error') - }) - - const nextTrack = getNextTrack(mockQueue) - - expect(nextTrack).toBeNull() + it.each([ + ['multi-track queue → first', [() => undefined], 'mockTrack2'], + ['single-track queue → that track', null, 'mockTrack2'], + ['empty queue → null', [], null], + ] as const)('%s', (_label, tracks, expected) => { + const lookup = { mockTrack2 } + if (tracks === null) withTracks([mockTrack2]) + else if (Array.isArray(tracks) && tracks.length === 0) withTracks([]) + else withTracks([mockTrack2, mockTrack3]) + const next = getNextTrack(mockQueue) + expect(next).toBe(expected ? lookup[expected as keyof typeof lookup] : null) + }) + + it('returns null on exception and logs it', () => { + withTracksThrow() + expect(getNextTrack(mockQueue)).toBeNull() expect(debugLogMock).toHaveBeenCalledWith({ message: 'Error getting next track:', error: expect.any(Error), @@ -453,66 +283,29 @@ describe('queueStateManager', () => { }) describe('getTrackAtPosition', () => { - beforeEach(() => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ - mockTrack, - mockTrack2, - mockTrack3, - ]) - }) - - it('should return track at valid position', () => { - const track = getTrackAtPosition(mockQueue, 0) - - expect(track).toBe(mockTrack) - }) - - it('should return track at middle position', () => { - const track = getTrackAtPosition(mockQueue, 1) - - expect(track).toBe(mockTrack2) - }) - - it('should return track at last position', () => { - const track = getTrackAtPosition(mockQueue, 2) - - expect(track).toBe(mockTrack3) - }) - - it('should return null when position is negative', () => { - const track = getTrackAtPosition(mockQueue, -1) - - expect(track).toBeNull() - }) + beforeEach(() => withTracks([mockTrack, mockTrack2, mockTrack3])) - it('should return null when position equals queue length', () => { - const track = getTrackAtPosition(mockQueue, 3) - - expect(track).toBeNull() - }) - - it('should return null when position exceeds queue length', () => { - const track = getTrackAtPosition(mockQueue, 10) - - expect(track).toBeNull() + it.each([ + ['first', 0, 'mockTrack'], + ['middle', 1, 'mockTrack2'], + ['last', 2, 'mockTrack3'], + ['negative', -1, null], + ['equals length', 3, null], + ['exceeds length', 10, null], + ] as const)('position %s → %s', (_label, position, expected) => { + const lookup = { mockTrack, mockTrack2, mockTrack3 } + const track = getTrackAtPosition(mockQueue, position) + expect(track).toBe(expected ? lookup[expected as keyof typeof lookup] : null) }) - it('should return null when queue is empty', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([]) - - const track = getTrackAtPosition(mockQueue, 0) - - expect(track).toBeNull() + it('returns null when queue is empty', () => { + withTracks([]) + expect(getTrackAtPosition(mockQueue, 0)).toBeNull() }) - it('should return null on exception', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockImplementation(() => { - throw new Error('Queue error') - }) - - const track = getTrackAtPosition(mockQueue, 0) - - expect(track).toBeNull() + it('returns null on exception and logs it', () => { + withTracksThrow() + expect(getTrackAtPosition(mockQueue, 0)).toBeNull() expect(debugLogMock).toHaveBeenCalledWith({ message: 'Error getting track at position:', error: expect.any(Error), @@ -521,144 +314,72 @@ describe('queueStateManager', () => { }) describe('isTrackInQueue', () => { - beforeEach(() => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ + beforeEach(() => + withTracks([ { id: 'track-1', url: 'https://example.com/track1.mp3' }, { id: 'track-2', url: 'https://example.com/track2.mp3' }, - ]) - }) - - it('should return true when track id matches', () => { - const inQueue = isTrackInQueue(mockQueue, 'track-1') - - expect(inQueue).toBe(true) - }) + ]), + ) - it('should return true when track url matches', () => { - const inQueue = isTrackInQueue(mockQueue, 'https://example.com/track1.mp3') - - expect(inQueue).toBe(true) + it.each([ + ['matching id', 'track-1', true], + ['matching url', 'https://example.com/track1.mp3', true], + ['no match', 'track-999', false], + ])('%s → %s', (_label, needle, expected) => { + expect(isTrackInQueue(mockQueue, needle)).toBe(expected) }) - it('should return false when track id does not match', () => { - const inQueue = isTrackInQueue(mockQueue, 'track-999') - - expect(inQueue).toBe(false) + it('falls back to url when id does not match', () => { + withTracks([{ id: 'different-id', url: 'https://example.com/track1.mp3' }]) + expect(isTrackInQueue(mockQueue, 'https://example.com/track1.mp3')).toBe(true) }) - it('should return false when queue is empty', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([]) - - const inQueue = isTrackInQueue(mockQueue, 'track-1') - - expect(inQueue).toBe(false) + it('returns false for empty queue', () => { + withTracks([]) + expect(isTrackInQueue(mockQueue, 'track-1')).toBe(false) }) - it('should return false on exception', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockImplementation(() => { - throw new Error('Queue error') - }) - - const inQueue = isTrackInQueue(mockQueue, 'track-1') - - expect(inQueue).toBe(false) + it('returns false on exception and logs it', () => { + withTracksThrow() + expect(isTrackInQueue(mockQueue, 'track-1')).toBe(false) expect(debugLogMock).toHaveBeenCalledWith({ message: 'Error checking if track is in queue:', error: expect.any(Error), }) }) - - it('should match either id or url (id takes precedence)', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ - { id: 'track-1', url: 'https://example.com/different.mp3' }, - ]) - - const inQueue = isTrackInQueue(mockQueue, 'track-1') - - expect(inQueue).toBe(true) - }) - - it('should match url when id does not match', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ - { id: 'different-id', url: 'https://example.com/track1.mp3' }, - ]) - - const inQueue = isTrackInQueue(mockQueue, 'https://example.com/track1.mp3') - - expect(inQueue).toBe(true) - }) }) describe('getTrackPosition', () => { - beforeEach(() => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ + beforeEach(() => + withTracks([ { id: 'track-1', url: 'https://example.com/track1.mp3' }, { id: 'track-2', url: 'https://example.com/track2.mp3' }, { id: 'track-3', url: 'https://example.com/track3.mp3' }, - ]) - }) - - it('should return position of track by id', () => { - const position = getTrackPosition(mockQueue, 'track-2') - - expect(position).toBe(1) - }) - - it('should return position of track by url', () => { - const position = getTrackPosition(mockQueue, 'https://example.com/track2.mp3') - - expect(position).toBe(1) - }) - - it('should return 0 for first track', () => { - const position = getTrackPosition(mockQueue, 'track-1') - - expect(position).toBe(0) - }) - - it('should return correct position for last track', () => { - const position = getTrackPosition(mockQueue, 'track-3') + ]), + ) - expect(position).toBe(2) + it.each([ + ['first by id', 'track-1', 0], + ['middle by id', 'track-2', 1], + ['last by id', 'track-3', 2], + ['middle by url', 'https://example.com/track2.mp3', 1], + ['not found', 'track-999', -1], + ])('%s → %s', (_label, needle, expected) => { + expect(getTrackPosition(mockQueue, needle)).toBe(expected) }) - it('should return -1 when track not found', () => { - const position = getTrackPosition(mockQueue, 'track-999') - - expect(position).toBe(-1) - }) - - it('should return -1 for empty queue', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([]) - - const position = getTrackPosition(mockQueue, 'track-1') - - expect(position).toBe(-1) + it('returns -1 for empty queue', () => { + withTracks([]) + expect(getTrackPosition(mockQueue, 'track-1')).toBe(-1) }) - it('should return -1 on exception', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockImplementation(() => { - throw new Error('Queue error') - }) - - const position = getTrackPosition(mockQueue, 'track-1') - - expect(position).toBe(-1) + it('returns -1 on exception and logs it', () => { + withTracksThrow() + expect(getTrackPosition(mockQueue, 'track-1')).toBe(-1) expect(debugLogMock).toHaveBeenCalledWith({ message: 'Error getting track position:', error: expect.any(Error), }) }) - - it('should match id before url in same track', () => { - ;(mockQueue.tracks?.toArray as jest.Mock).mockReturnValue([ - { id: 'track-1', url: 'https://example.com/track1.mp3' }, - { id: 'track-2', url: 'https://example.com/different.mp3' }, - ]) - - const position = getTrackPosition(mockQueue, 'track-2') - - expect(position).toBe(1) - }) }) }) diff --git a/packages/bot/src/utils/music/youtubeErrorHandler/analyzer.spec.ts b/packages/bot/src/utils/music/youtubeErrorHandler/analyzer.spec.ts index 723313004..8fbc42659 100644 --- a/packages/bot/src/utils/music/youtubeErrorHandler/analyzer.spec.ts +++ b/packages/bot/src/utils/music/youtubeErrorHandler/analyzer.spec.ts @@ -1,7 +1,24 @@ import { beforeEach, describe, expect, it, jest } from '@jest/globals' -import type { YouTubeErrorInfo, YouTubeErrorContext, YouTubeErrorResponse } from './types' +import type { YouTubeErrorInfo, YouTubeErrorContext } from './types' import { YouTubeErrorAnalyzer } from './analyzer' +// Build a fresh YouTubeErrorInfo with all flags false except the requested one. +// Used by the getErrorResponse describes so each scenario is one bool flip +// rather than a full literal repeat. +function infoWith(overrides: Partial = {}): YouTubeErrorInfo { + return { + isParserError: false, + isCompositeVideoError: false, + isHypePointsError: false, + isTypeMismatchError: false, + isGridShelfViewError: false, + isSectionHeaderViewError: false, + shouldRetry: false, + retryWithFallback: false, + ...overrides, + } +} + describe('YouTubeErrorAnalyzer', () => { let analyzer: YouTubeErrorAnalyzer let mockContext: YouTubeErrorContext @@ -18,705 +35,282 @@ describe('YouTubeErrorAnalyzer', () => { }) describe('analyzeError', () => { - describe('parser error detection', () => { - it('should detect parser error with innertubeerror message', () => { - const error = new Error('InnerTubeError: Invalid request') - const result = analyzer.analyzeError(error) - - expect(result.isParserError).toBe(true) - }) - - it('should detect parser error with parsingerror message', () => { - const error = new Error('ParsingError: Failed to parse response') - const result = analyzer.analyzeError(error) - - expect(result.isParserError).toBe(true) - }) - - it('should detect parser error with youtubei.js in stack', () => { - const error = new Error('Failed to fetch') - error.stack = 'Error: Failed to fetch\n at youtubei.js:123:45' - const result = analyzer.analyzeError(error) - - expect(result.isParserError).toBe(true) - }) - - it('should be case-insensitive for parser error detection', () => { - const error = new Error('INNERTUBEERROR: test') - const result = analyzer.analyzeError(error) - - expect(result.isParserError).toBe(true) - }) - - it('should not detect parser error when error does not match', () => { - const error = new Error('Some other error') - const result = analyzer.analyzeError(error) - - expect(result.isParserError).toBe(false) - }) + // Each row covers one error category × one positive-match input (case + // sensitivity is exercised by the case-insensitive row per category). + // Negative matches are covered once at the bottom (one shared "Some + // other error" check) — six identical "does not detect X" cases were + // collapsed into one. + it.each([ + ['parser', 'InnerTubeError: Invalid request', 'isParserError'], + ['parser', 'ParsingError: Failed to parse response', 'isParserError'], + ['parser (case-insensitive)', 'INNERTUBEERROR: test', 'isParserError'], + ['composite video', 'CompositeVideoError: Video format not supported', 'isCompositeVideoError'], + ['composite video (case-insensitive)', 'compositevideoERROR: test', 'isCompositeVideoError'], + ['hype points', 'HypePoints: Unable to process hype points', 'isHypePointsError'], + ['hype points (case-insensitive)', 'HYPEPOINTS: test', 'isHypePointsError'], + ['type mismatch', 'TypeError: TypeMismatch in response', 'isTypeMismatchError'], + ['type mismatch (case-insensitive)', 'TYPEMISMATCH: test', 'isTypeMismatchError'], + ['grid shelf view', 'GridShelfView: Failed to render shelf', 'isGridShelfViewError'], + ['grid shelf view (case-insensitive)', 'GRIDSHELFVIEW: test', 'isGridShelfViewError'], + ['section header view', 'SectionHeaderView: Failed to render header', 'isSectionHeaderViewError'], + ['section header view (case-insensitive)', 'SECTIONHEADERVIEW: test', 'isSectionHeaderViewError'], + ] as const)('detects %s error from message %s', (_label, message, flag) => { + const result = analyzer.analyzeError(new Error(message)) + expect(result[flag as keyof YouTubeErrorInfo]).toBe(true) }) - describe('composite video error detection', () => { - it('should detect composite video error', () => { - const error = new Error('CompositeVideoError: Video format not supported') - const result = analyzer.analyzeError(error) - - expect(result.isCompositeVideoError).toBe(true) - }) - - it('should be case-insensitive for composite video error detection', () => { - const error = new Error('compositevideoERROR: test') - const result = analyzer.analyzeError(error) - - expect(result.isCompositeVideoError).toBe(true) - }) - - it('should not detect composite video error when error does not match', () => { - const error = new Error('Some other error') - const result = analyzer.analyzeError(error) - expect(result.isCompositeVideoError).toBe(false) - }) + it.each([ + ['parser error (no composite or hype)', 'InnerTubeError: Test', { isParserError: true, isCompositeVideoError: false, isHypePointsError: false }], + ['composite error (no parser or type-mismatch)', 'CompositeVideoError: Test', { isCompositeVideoError: true, isParserError: false, isTypeMismatchError: false }], + ['hype points error (no grid-shelf or section-header)', 'HypePoints: Test', { isHypePointsError: true, isGridShelfViewError: false, isSectionHeaderViewError: false }], + ] as const)('detects %s with explicit negative flags', (_label, message, expectedFlags) => { + const result = analyzer.analyzeError(new Error(message)) + expect(result).toMatchObject(expectedFlags) }) - - describe('hype points error detection', () => { - it('should detect hype points error', () => { - const error = new Error('HypePoints: Unable to process hype points') - const result = analyzer.analyzeError(error) - - expect(result.isHypePointsError).toBe(true) - }) - - it('should be case-insensitive for hype points error detection', () => { - const error = new Error('HYPEPOINTS: test') - const result = analyzer.analyzeError(error) - - expect(result.isHypePointsError).toBe(true) - }) - - it('should not detect hype points error when error does not match', () => { - const error = new Error('Some other error') - const result = analyzer.analyzeError(error) - - expect(result.isHypePointsError).toBe(false) - }) + it('detects parser error from youtubei.js stack frame even with unrelated message', () => { + const error = new Error('Failed to fetch') + error.stack = 'Error: Failed to fetch\n at youtubei.js:123:45' + expect(analyzer.analyzeError(error).isParserError).toBe(true) }) - describe('type mismatch error detection', () => { - it('should detect type mismatch error', () => { - const error = new Error('TypeError: TypeMismatch in response') - const result = analyzer.analyzeError(error) - - expect(result.isTypeMismatchError).toBe(true) - }) - - it('should be case-insensitive for type mismatch error detection', () => { - const error = new Error('TYPEMISMATCH: test') - const result = analyzer.analyzeError(error) - - expect(result.isTypeMismatchError).toBe(true) - }) - - it('should not detect type mismatch error when error does not match', () => { - const error = new Error('Some other error') - const result = analyzer.analyzeError(error) - - expect(result.isTypeMismatchError).toBe(false) + it('returns all-false flags for an unrelated error message', () => { + const result = analyzer.analyzeError(new Error('Some other error')) + expect(result).toMatchObject({ + isParserError: false, + isCompositeVideoError: false, + isHypePointsError: false, + isTypeMismatchError: false, + isGridShelfViewError: false, + isSectionHeaderViewError: false, }) }) - describe('grid shelf view error detection', () => { - it('should detect grid shelf view error', () => { - const error = new Error('GridShelfView: Failed to render shelf') - const result = analyzer.analyzeError(error) - - expect(result.isGridShelfViewError).toBe(true) - }) - - it('should be case-insensitive for grid shelf view error detection', () => { - const error = new Error('GRIDSHELFVIEW: test') - const result = analyzer.analyzeError(error) - - expect(result.isGridShelfViewError).toBe(true) - }) - - it('should not detect grid shelf view error when error does not match', () => { - const error = new Error('Some other error') - const result = analyzer.analyzeError(error) - - expect(result.isGridShelfViewError).toBe(false) - }) + it.each([ + ['timeout', 'Request timeout'], + ['network', 'Network error occurred'], + ['rate limit', 'Rate limit exceeded'], + ['quota exceeded', 'Quota exceeded'], + ])('marks %s error as retryable', (_label, message) => { + expect(analyzer.analyzeError(new Error(message)).shouldRetry).toBe(true) }) - describe('section header view error detection', () => { - it('should detect section header view error', () => { - const error = new Error('SectionHeaderView: Failed to render header') - const result = analyzer.analyzeError(error) - - expect(result.isSectionHeaderViewError).toBe(true) - }) - - it('should be case-insensitive for section header view error detection', () => { - const error = new Error('SECTIONHEADERVIEW: test') - const result = analyzer.analyzeError(error) - - expect(result.isSectionHeaderViewError).toBe(true) - }) - - it('should not detect section header view error when error does not match', () => { - const error = new Error('Some other error') - const result = analyzer.analyzeError(error) - - expect(result.isSectionHeaderViewError).toBe(false) - }) + it('does not mark an unrelated error as retryable', () => { + expect(analyzer.analyzeError(new Error('Invalid input')).shouldRetry).toBe(false) }) - describe('retry logic', () => { - it('should set shouldRetry true for timeout error', () => { - const error = new Error('Request timeout') - const result = analyzer.analyzeError(error) - - expect(result.shouldRetry).toBe(true) - }) - - it('should set shouldRetry true for network error', () => { - const error = new Error('Network error occurred') - const result = analyzer.analyzeError(error) - - expect(result.shouldRetry).toBe(true) - }) - - it('should set shouldRetry true for rate limit error', () => { - const error = new Error('Rate limit exceeded') - const result = analyzer.analyzeError(error) - - expect(result.shouldRetry).toBe(true) - }) - - it('should set shouldRetry true for quota exceeded error', () => { - const error = new Error('Quota exceeded') - const result = analyzer.analyzeError(error) - - expect(result.shouldRetry).toBe(true) - }) - - it('should set shouldRetry false for non-retryable error', () => { - const error = new Error('Invalid input') - const result = analyzer.analyzeError(error) - - expect(result.shouldRetry).toBe(false) - }) + it.each([ + ['signature', 'Signature verification failed'], + ['cipher', 'Cipher decryption failed'], + ['decrypt', 'Decrypt operation failed'], + ])('marks %s error for retry-with-fallback', (_label, message) => { + expect(analyzer.analyzeError(new Error(message)).retryWithFallback).toBe(true) }) - describe('retry with fallback logic', () => { - it('should set retryWithFallback true for signature error', () => { - const error = new Error('Signature verification failed') - const result = analyzer.analyzeError(error) - - expect(result.retryWithFallback).toBe(true) - }) - - it('should set retryWithFallback true for cipher error', () => { - const error = new Error('Cipher decryption failed') - const result = analyzer.analyzeError(error) - - expect(result.retryWithFallback).toBe(true) - }) - - it('should set retryWithFallback true for decrypt error', () => { - const error = new Error('Decrypt operation failed') - const result = analyzer.analyzeError(error) - - expect(result.retryWithFallback).toBe(true) - }) - - it('should set retryWithFallback false for non-fallback error', () => { - const error = new Error('Invalid input') - const result = analyzer.analyzeError(error) - - expect(result.retryWithFallback).toBe(false) - }) + it('does not mark an unrelated error for retry-with-fallback', () => { + expect(analyzer.analyzeError(new Error('Invalid input')).retryWithFallback).toBe(false) }) - describe('error without stack trace', () => { - it('should handle error with undefined stack', () => { - const error = new Error('Test error') - error.stack = undefined - const result = analyzer.analyzeError(error) - - expect(result).toBeDefined() - expect(result.isParserError).toBe(false) - }) - - it('should handle error with empty stack', () => { - const error = new Error('Test error') - error.stack = '' - const result = analyzer.analyzeError(error) - - expect(result).toBeDefined() - }) + it.each([ + ['undefined', undefined], + ['empty', ''], + ])('handles error with %s stack without throwing', (_label, stack) => { + const error = new Error('Test error') + error.stack = stack + const result = analyzer.analyzeError(error) + expect(result).toBeDefined() + expect(result.isParserError).toBe(false) }) - describe('multiple error characteristics', () => { - it('should detect multiple error characteristics in one error', () => { - const error = new Error('InnerTubeError: Network timeout') - const result = analyzer.analyzeError(error) - - expect(result.isParserError).toBe(true) - expect(result.shouldRetry).toBe(true) - }) - - it('should properly separate unrelated error characteristics', () => { - const error = new Error('CompositeVideoError: Invalid format') - const result = analyzer.analyzeError(error) - - expect(result.isCompositeVideoError).toBe(true) - expect(result.isParserError).toBe(false) - expect(result.isHypePointsError).toBe(false) - }) + it('detects multiple characteristics in one error and keeps unrelated flags false', () => { + const result = analyzer.analyzeError( + new Error('InnerTubeError: Network timeout'), + ) + expect(result.isParserError).toBe(true) + expect(result.shouldRetry).toBe(true) + expect(result.isCompositeVideoError).toBe(false) + expect(result.isHypePointsError).toBe(false) }) }) describe('getErrorResponse', () => { - describe('parser error handling', () => { - it('should return parser error response for parser error', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: true, - isCompositeVideoError: false, - isHypePointsError: false, - isTypeMismatchError: false, - isGridShelfViewError: false, - isSectionHeaderViewError: false, - shouldRetry: false, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response).toEqual({ + // Each row asserts the canonical response payload for a single error + // category. Replaces six near-identical describes that built a full + // YouTubeErrorInfo literal each time. + it.each([ + [ + 'parser', + infoWith({ isParserError: true }), + { shouldRetry: true, retryWithFallback: true, userMessage: 'YouTube parser error, trying alternative method...', - logLevel: 'warn', - }) - }) - - it('should return parser error response even if shouldRetry is false', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: true, - isCompositeVideoError: false, - isHypePointsError: false, - isTypeMismatchError: false, - isGridShelfViewError: false, - isSectionHeaderViewError: false, - shouldRetry: false, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response.shouldRetry).toBe(true) - expect(response.logLevel).toBe('warn') - }) - }) - - describe('composite video error handling', () => { - it('should return video format error response for composite video error', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: true, - isHypePointsError: false, - isTypeMismatchError: false, - isGridShelfViewError: false, - isSectionHeaderViewError: false, - shouldRetry: true, - retryWithFallback: true, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response).toEqual({ + logLevel: 'warn' as const, + }, + ], + [ + 'composite video', + infoWith({ isCompositeVideoError: true, shouldRetry: true, retryWithFallback: true }), + { shouldRetry: false, retryWithFallback: false, userMessage: 'Video format not supported', - logLevel: 'error', - }) - }) - - it('should not retry composite video errors even if shouldRetry is true', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: true, - isHypePointsError: false, - isTypeMismatchError: false, - isGridShelfViewError: false, - isSectionHeaderViewError: false, - shouldRetry: true, - retryWithFallback: true, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response.shouldRetry).toBe(false) - expect(response.retryWithFallback).toBe(false) - }) - }) - - describe('hype points error handling', () => { - it('should return hype points error response', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: false, - isHypePointsError: true, - isTypeMismatchError: false, - isGridShelfViewError: false, - isSectionHeaderViewError: false, - shouldRetry: false, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response).toEqual({ + logLevel: 'error' as const, + }, + ], + [ + 'hype points', + infoWith({ isHypePointsError: true }), + { shouldRetry: true, retryWithFallback: false, userMessage: 'YouTube hype points error, retrying...', - logLevel: 'warn', - }) - }) - }) - - describe('type mismatch error handling', () => { - it('should return type mismatch error response', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: false, - isHypePointsError: false, - isTypeMismatchError: true, - isGridShelfViewError: false, - isSectionHeaderViewError: false, - shouldRetry: false, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response).toEqual({ + logLevel: 'warn' as const, + }, + ], + [ + 'type mismatch', + infoWith({ isTypeMismatchError: true }), + { shouldRetry: true, retryWithFallback: true, userMessage: 'YouTube type mismatch, trying alternative method...', - logLevel: 'warn', - }) - }) - }) - - describe('grid shelf view error handling', () => { - it('should return grid shelf view error response', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: false, - isHypePointsError: false, - isTypeMismatchError: false, - isGridShelfViewError: true, - isSectionHeaderViewError: false, - shouldRetry: false, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response).toEqual({ + logLevel: 'warn' as const, + }, + ], + [ + 'grid shelf view', + infoWith({ isGridShelfViewError: true }), + { shouldRetry: true, retryWithFallback: false, userMessage: 'YouTube grid shelf error, retrying...', - logLevel: 'warn', - }) - }) - }) - - describe('section header view error handling', () => { - it('should return section header view error response', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: false, - isHypePointsError: false, - isTypeMismatchError: false, - isGridShelfViewError: false, - isSectionHeaderViewError: true, - shouldRetry: false, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response).toEqual({ + logLevel: 'warn' as const, + }, + ], + [ + 'section header view', + infoWith({ isSectionHeaderViewError: true }), + { shouldRetry: true, retryWithFallback: false, userMessage: 'YouTube section header error, retrying...', - logLevel: 'warn', - }) - }) + logLevel: 'warn' as const, + }, + ], + ] as const)('returns canonical response for %s error', (_label, info, expected) => { + expect(analyzer.getErrorResponse(info, mockContext)).toEqual(expected) }) - describe('fallback to generic error response', () => { - it('should return generic error response when no specific error matches', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: false, - isHypePointsError: false, - isTypeMismatchError: false, - isGridShelfViewError: false, - isSectionHeaderViewError: false, - shouldRetry: true, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response).toEqual({ - shouldRetry: true, - retryWithFallback: false, - userMessage: 'YouTube error occurred', - logLevel: 'error', - }) - }) - - it('should use shouldRetry from errorInfo in generic response', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: false, - isHypePointsError: false, - isTypeMismatchError: false, - isGridShelfViewError: false, - isSectionHeaderViewError: false, - shouldRetry: false, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response.shouldRetry).toBe(false) - }) - - it('should use retryWithFallback from errorInfo in generic response', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: false, - isHypePointsError: false, - isTypeMismatchError: false, - isGridShelfViewError: false, - isSectionHeaderViewError: false, - shouldRetry: true, - retryWithFallback: true, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response.retryWithFallback).toBe(true) + it('returns the generic response and inherits flags when no specific category matches', () => { + const info = infoWith({ shouldRetry: true, retryWithFallback: true }) + expect(analyzer.getErrorResponse(info, mockContext)).toEqual({ + shouldRetry: true, + retryWithFallback: true, + userMessage: 'YouTube error occurred', + logLevel: 'error', }) }) - describe('error hierarchy priority', () => { - it('should prioritize parser error over other errors', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: true, - isCompositeVideoError: true, - isHypePointsError: true, - isTypeMismatchError: true, - isGridShelfViewError: true, - isSectionHeaderViewError: true, - shouldRetry: false, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response.userMessage).toBe( - 'YouTube parser error, trying alternative method...', - ) - }) - - it('should prioritize composite video error over retryable errors', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: true, - isHypePointsError: true, - isTypeMismatchError: true, - isGridShelfViewError: true, - isSectionHeaderViewError: true, - shouldRetry: true, - retryWithFallback: true, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response.userMessage).toBe('Video format not supported') - }) - - it('should prioritize hype points error first among retryable errors', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: false, - isHypePointsError: true, - isTypeMismatchError: true, - isGridShelfViewError: true, - isSectionHeaderViewError: true, - shouldRetry: false, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response.userMessage).toBe( - 'YouTube hype points error, retrying...', - ) - }) - - it('should prioritize type mismatch error second among retryable errors', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: false, - isHypePointsError: false, - isTypeMismatchError: true, - isGridShelfViewError: true, - isSectionHeaderViewError: true, - shouldRetry: false, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response.userMessage).toBe( - 'YouTube type mismatch, trying alternative method...', - ) - }) - - it('should prioritize grid shelf view error third among retryable errors', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: false, - isHypePointsError: false, - isTypeMismatchError: false, - isGridShelfViewError: true, - isSectionHeaderViewError: true, - shouldRetry: false, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response.userMessage).toBe( - 'YouTube grid shelf error, retrying...', - ) - }) - - it('should prioritize section header view error fourth among retryable errors', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: false, - isHypePointsError: false, - isTypeMismatchError: false, - isGridShelfViewError: false, - isSectionHeaderViewError: true, - shouldRetry: false, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response.userMessage).toBe( - 'YouTube section header error, retrying...', - ) - }) + it('preserves shouldRetry=false in the generic response', () => { + const info = infoWith() + expect(analyzer.getErrorResponse(info, mockContext).shouldRetry).toBe(false) }) - describe('context parameter handling', () => { - it('should accept context parameter even though it is unused', () => { - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: false, - isHypePointsError: false, - isTypeMismatchError: false, - isGridShelfViewError: false, - isSectionHeaderViewError: false, - shouldRetry: false, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(response).toBeDefined() - expect(response.userMessage).toBe('YouTube error occurred') - }) - - it('should handle context with optional guildId', () => { - const contextWithoutGuild: YouTubeErrorContext = { - query: 'test query', - userId: 'user-123', - timestamp: Date.now(), - } - - const errorInfo: YouTubeErrorInfo = { - isParserError: false, - isCompositeVideoError: false, - isHypePointsError: false, - isTypeMismatchError: false, - isGridShelfViewError: false, - isSectionHeaderViewError: false, - shouldRetry: false, - retryWithFallback: false, - } - - const response = analyzer.getErrorResponse( - errorInfo, - contextWithoutGuild, - ) + it('honours the documented priority order when several flags are set', () => { + // Folds six "should prioritize X" describes into one assertion that + // walks the full chain: parser > composite > hype > type-mismatch > + // grid-shelf > section-header > generic. + const allFlags: Array = [ + 'isParserError', + 'isCompositeVideoError', + 'isHypePointsError', + 'isTypeMismatchError', + 'isGridShelfViewError', + 'isSectionHeaderViewError', + ] + const expectedMessage: Record = { + isParserError: 'YouTube parser error, trying alternative method...', + isCompositeVideoError: 'Video format not supported', + isHypePointsError: 'YouTube hype points error, retrying...', + isTypeMismatchError: 'YouTube type mismatch, trying alternative method...', + isGridShelfViewError: 'YouTube grid shelf error, retrying...', + isSectionHeaderViewError: 'YouTube section header error, retrying...', + } + // For each priority slot, set that flag plus everything below and + // assert the higher-priority message wins. + for (let i = 0; i < allFlags.length; i++) { + const overrides: Partial = {} + for (let j = i; j < allFlags.length; j++) overrides[allFlags[j]!] = true as never + const info = infoWith(overrides) + const response = analyzer.getErrorResponse(info, mockContext) + expect(response.userMessage).toBe(expectedMessage[allFlags[i]!]) + } + }) - expect(response).toBeDefined() - }) + it('handles context without guildId', () => { + const contextWithoutGuild: YouTubeErrorContext = { + query: 'test query', + userId: 'user-123', + timestamp: Date.now(), + } + expect( + analyzer.getErrorResponse(infoWith(), contextWithoutGuild), + ).toBeDefined() }) }) describe('integration: analyze then respond', () => { - it('should analyze error and get appropriate response for parser error', () => { - const error = new Error('InnerTubeError: Invalid request') - const errorInfo = analyzer.analyzeError(error) - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(errorInfo.isParserError).toBe(true) - expect(response.userMessage).toBe( - 'YouTube parser error, trying alternative method...', - ) - expect(response.logLevel).toBe('warn') - }) - - it('should analyze error and get appropriate response for network timeout', () => { - const error = new Error('Request timeout') - const errorInfo = analyzer.analyzeError(error) - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(errorInfo.shouldRetry).toBe(true) - expect(response.shouldRetry).toBe(true) - expect(response.logLevel).toBe('error') - }) - - it('should analyze error and get appropriate response for cipher failure', () => { - const error = new Error('Cipher decryption failed') - const errorInfo = analyzer.analyzeError(error) - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(errorInfo.retryWithFallback).toBe(true) - expect(response.retryWithFallback).toBe(true) - }) - - it('should analyze error and get appropriate response for composite video error', () => { - const error = new Error('CompositeVideoError: Format not supported') - const errorInfo = analyzer.analyzeError(error) - const response = analyzer.getErrorResponse(errorInfo, mockContext) - - expect(errorInfo.isCompositeVideoError).toBe(true) - expect(response.shouldRetry).toBe(false) - expect(response.userMessage).toBe('Video format not supported') - }) + it.each([ + [ + 'parser error', + new Error('InnerTubeError: Invalid request'), + { + flag: 'isParserError' as const, + userMessage: 'YouTube parser error, trying alternative method...', + logLevel: 'warn' as const, + }, + ], + [ + 'network timeout (retryable, no specific category)', + new Error('Request timeout'), + { + flag: 'shouldRetry' as const, + userMessage: 'YouTube error occurred', + logLevel: 'error' as const, + }, + ], + [ + 'cipher failure (retry-with-fallback)', + new Error('Cipher decryption failed'), + { + flag: 'retryWithFallback' as const, + userMessage: 'YouTube error occurred', + logLevel: 'error' as const, + }, + ], + [ + 'composite video error (terminal)', + new Error('CompositeVideoError: Format not supported'), + { + flag: 'isCompositeVideoError' as const, + userMessage: 'Video format not supported', + logLevel: 'error' as const, + extra: (resp: { shouldRetry: boolean }) => + expect(resp.shouldRetry).toBe(false), + }, + ], + ] as const)( + 'analyzes %s and forwards to the right response', + (_label, error, expected) => { + const info = analyzer.analyzeError(error) + expect(info[expected.flag]).toBe(true) + const response = analyzer.getErrorResponse(info, mockContext) + expect(response.userMessage).toBe(expected.userMessage) + expect(response.logLevel).toBe(expected.logLevel) + if ('extra' in expected && expected.extra) + (expected.extra as (r: typeof response) => void)(response) + }, + ) }) })