From 437edbbcc471681a9bd60beed9670802975c9106 Mon Sep 17 00:00:00 2001 From: htilly Date: Tue, 27 Jan 2026 18:56:51 +0100 Subject: [PATCH 1/2] feat: Improve queue size verification with detailed warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add searchalbum track count display (e.g., "17 tracks") - Add validators: queueSizeIncreaseExactly, extractAndStoreTrackCount, queueSizeIncreasedByStoredCount - Verify add command increases queue by exactly 1 - Search album/playlist before adding to get expected track count - Verify actual increase matches expected (catches duplicate queueing bugs) - Add three-level output: exact (silent), within tolerance (warning), outside tolerance (fail) - Show detailed baseline → actual numbers in warnings/errors --- lib/command-handlers.js | 3 +- lib/spotify.js | 3 +- test/tools/integration-test-suite.mjs | 148 +++++++++++++++++++++++++- 3 files changed, 148 insertions(+), 6 deletions(-) diff --git a/lib/command-handlers.js b/lib/command-handlers.js index 352147d..e35cd47 100644 --- a/lib/command-handlers.js +++ b/lib/command-handlers.js @@ -645,7 +645,8 @@ async function searchalbum(input, channel) { let message = `Found ${sortedAlbums.length} albums:\n`; sortedAlbums.forEach((albumResult) => { - message += `> *${albumResult.name}* by _${albumResult.artist}_\n`; + const trackInfo = albumResult.totalTracks ? ` (${albumResult.totalTracks} tracks)` : ''; + message += `> *${albumResult.name}* by _${albumResult.artist}_${trackInfo}\n`; }); sendMessage(message, channel); } catch (err) { diff --git a/lib/spotify.js b/lib/spotify.js index 652a6b5..ad0830b 100644 --- a/lib/spotify.js +++ b/lib/spotify.js @@ -303,7 +303,8 @@ module.exports = function (config, injectedLogger) { name: album.name, artist: album.artists[0].name, uri: album.uri, - popularity: album.popularity // Keep popularity for sorting + popularity: album.popularity, // Keep popularity for sorting + totalTracks: album.total_tracks || 0 })); }, diff --git a/test/tools/integration-test-suite.mjs b/test/tools/integration-test-suite.mjs index 26e6610..0a1ab59 100644 --- a/test/tools/integration-test-suite.mjs +++ b/test/tools/integration-test-suite.mjs @@ -492,14 +492,27 @@ class TestCase { // Validators const queueSizeStore = {}; +const extractedValues = {}; function extractQueueSize(responses) { const allText = responses.map(r => r.text).join(' '); + // Match "X tracks" pattern first (more specific) + const tracksMatch = allText.match(/(\d+)\s*track/i); + if (tracksMatch) return parseInt(tracksMatch[1], 10); + // Fall back to first number const match = allText.match(/\b(\d+)\b/); if (!match) return null; return parseInt(match[1], 10); } +function extractTrackCountFromResponse(responses) { + const allText = responses.map(r => r.text).join(' '); + // Match patterns like "(150 tracks)" or "150 tracks" + const match = allText.match(/\((\d+)\s*tracks?\)/i) || allText.match(/(\d+)\s*tracks?/i); + if (!match) return null; + return parseInt(match[1], 10); +} + const validators = { containsText: (text) => (responses) => { const allText = responses.map(r => r.text).join(' '); @@ -568,6 +581,69 @@ const validators = { } if (size >= baseline + minIncrease) return true; return `Expected queue size to increase by ${minIncrease} from ${baseline}, got ${size}`; + }, + + // Verify queue size increased by EXACTLY N (not double, not less) + queueSizeIncreaseExactly: (key, exactIncrease) => (responses) => { + const size = extractQueueSize(responses); + if (size === null) return 'Could not parse queue size from response'; + const baseline = queueSizeStore[key]; + if (baseline === undefined || baseline === null) { + return `No baseline queue size recorded for "${key}"`; + } + const actualIncrease = size - baseline; + + // Exact match - no output + if (actualIncrease === exactIncrease) return true; + + // Within tolerance (90-100%) - warning but pass + const tolerance = Math.floor(exactIncrease * 0.9); + if (actualIncrease >= tolerance && actualIncrease < exactIncrease) { + console.log(` ⚠️ WARNING: Queue increased by ${actualIncrease} (expected ${exactIncrease}, baseline: ${baseline} → ${size})`); + return true; + } + + // Outside tolerance - fail + return `❌ FAIL: Queue increased by ${actualIncrease} (expected exactly ${exactIncrease}, baseline: ${baseline} → ${size})`; + }, + + // Extract and store track count from search results (e.g., "(50 tracks)") + extractAndStoreTrackCount: (key) => (responses) => { + const count = extractTrackCountFromResponse(responses); + if (count === null) return 'Could not extract track count from response'; + extractedValues[key] = count; + return true; + }, + + // Verify queue increased by stored track count (with tolerance for duplicates/blacklist) + queueSizeIncreasedByStoredCount: (baselineKey, countKey, tolerancePercent = 10) => (responses) => { + const size = extractQueueSize(responses); + if (size === null) return 'Could not parse queue size from response'; + const baseline = queueSizeStore[baselineKey]; + const expectedCount = extractedValues[countKey]; + if (baseline === undefined) return `No baseline recorded for "${baselineKey}"`; + if (expectedCount === undefined) return `No track count recorded for "${countKey}"`; + + const actualIncrease = size - baseline; + const minExpected = Math.floor(expectedCount * (1 - tolerancePercent / 100)); + const maxExpected = expectedCount; // Should not exceed expected (no doubling!) + + // Exact match - no output + if (actualIncrease === expectedCount) return true; + + // Within tolerance but not exact - warning but pass + if (actualIncrease >= minExpected && actualIncrease < expectedCount) { + console.log(` ⚠️ WARNING: Queue increased by ${actualIncrease} (expected ${expectedCount}, baseline: ${baseline} → ${size}, tolerance: ${minExpected}-${maxExpected})`); + return true; + } + + // Exceeded expected (possible doubling bug) - fail + if (actualIncrease > maxExpected) { + return `❌ FAIL: Queue increased by ${actualIncrease} but expected max ${maxExpected} - possible DUPLICATE QUEUEING BUG! (baseline: ${baseline} → ${size})`; + } + + // Below minimum (too many filtered) - fail + return `❌ FAIL: Queue increased by ${actualIncrease}, expected ${minExpected}-${maxExpected} (based on ${expectedCount} tracks, baseline: ${baseline} → ${size})`; } }; @@ -730,6 +806,17 @@ const testSuiteArray = [ // PHASE 4: BUILD UP THE QUEUE (add tracks for later tests) // ═══════════════════════════════════════════════════════════════════ + // Get baseline queue size before adding tracks + new TestCase( + 'Queue Size - Initial Baseline', + 'size', + validators.and( + validators.responseCount(1, 2), + validators.recordQueueSize('initialBaseline') + ), + 4 + ), + new TestCase( 'Add Track #1 - Foo Fighters', 'add Foo Fighters - Best Of You', @@ -744,6 +831,18 @@ const testSuiteArray = [ 7 ), + // Verify exactly 1 track was added + new TestCase( + 'Queue Size - After Track #1 (+1)', + 'size', + validators.and( + validators.responseCount(1, 2), + validators.queueSizeIncreaseExactly('initialBaseline', 1), + validators.recordQueueSize('afterTrack1') + ), + 4 + ), + new TestCase( 'Add Track - Duplicate Detection', 'add Foo Fighters - Best Of You', @@ -768,6 +867,18 @@ const testSuiteArray = [ 7 ), + // Verify exactly 1 more track added (total +2 from initial) + new TestCase( + 'Queue Size - After Track #2 (+1)', + 'size', + validators.and( + validators.responseCount(1, 2), + validators.queueSizeIncreaseExactly('afterTrack1', 1), + validators.recordQueueSize('afterTrack2') + ), + 4 + ), + new TestCase( 'Add Track #3 - Queen', 'add Queen - Bohemian Rhapsody', @@ -796,6 +907,21 @@ const testSuiteArray = [ 7 ), + // Search album first to get track count + new TestCase( + 'Search Album - Abbey Road (get track count)', + 'searchalbum abbey road', + validators.and( + validators.responseCount(1, 2), + validators.or( + validators.containsText('Beatles'), + validators.containsText('Abbey Road') + ), + validators.extractAndStoreTrackCount('abbeyRoadTracks') + ), + 5 + ), + new TestCase( 'Queue Size - Baseline Before Album', 'size', @@ -820,17 +946,30 @@ const testSuiteArray = [ 10 ), + // Verify album tracks were added (not doubled!) new TestCase( - 'Queue Size - After Album', + 'Queue Size - After Album (verify no doubling)', 'size', validators.and( validators.responseCount(1, 2), - validators.queueSizeIncreaseFrom('beforeAlbum', 1), + validators.queueSizeIncreasedByStoredCount('beforeAlbum', 'abbeyRoadTracks', 20), validators.recordQueueSize('beforePlaylist') ), 4 ), + // Search playlist first to get track count + new TestCase( + 'Search Playlist - Rock Classics (get track count)', + 'searchplaylist rock classics', + validators.and( + validators.responseCount(1, 2), + validators.matchesRegex(/playlist|tracks|\d+/i), + validators.extractAndStoreTrackCount('rockClassicsTracks') + ), + 5 + ), + new TestCase( 'Add Playlist - Rock Classics', 'addplaylist rock classics', @@ -845,12 +984,13 @@ const testSuiteArray = [ 12 ), + // Verify playlist tracks added (not doubled!) new TestCase( - 'Queue Size - After Playlist', + 'Queue Size - After Playlist (verify no doubling)', 'size', validators.and( validators.responseCount(1, 2), - validators.queueSizeIncreaseFrom('beforePlaylist', 1) + validators.queueSizeIncreasedByStoredCount('beforePlaylist', 'rockClassicsTracks', 20) ), 4 ), From 0e4a436b2c31ceda901c44e9483b5e2f91b178a7 Mon Sep 17 00:00:00 2001 From: htilly Date: Tue, 27 Jan 2026 19:09:32 +0100 Subject: [PATCH 2/2] fix: Address Copilot suggestions - tolerance bug and pluralization - Disable tolerance for small exact values (1-2 tracks) to prevent accepting 0 when expecting 1 - Fix pluralization in searchalbum: '1 track' instead of '1 tracks' --- lib/command-handlers.js | 4 +++- test/tools/integration-test-suite.mjs | 15 +++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/command-handlers.js b/lib/command-handlers.js index e35cd47..9721e82 100644 --- a/lib/command-handlers.js +++ b/lib/command-handlers.js @@ -645,7 +645,9 @@ async function searchalbum(input, channel) { let message = `Found ${sortedAlbums.length} albums:\n`; sortedAlbums.forEach((albumResult) => { - const trackInfo = albumResult.totalTracks ? ` (${albumResult.totalTracks} tracks)` : ''; + const trackInfo = albumResult.totalTracks + ? ` (${albumResult.totalTracks} ${albumResult.totalTracks === 1 ? 'track' : 'tracks'})` + : ''; message += `> *${albumResult.name}* by _${albumResult.artist}_${trackInfo}\n`; }); sendMessage(message, channel); diff --git a/test/tools/integration-test-suite.mjs b/test/tools/integration-test-suite.mjs index 0a1ab59..eef9bb2 100644 --- a/test/tools/integration-test-suite.mjs +++ b/test/tools/integration-test-suite.mjs @@ -596,14 +596,17 @@ const validators = { // Exact match - no output if (actualIncrease === exactIncrease) return true; - // Within tolerance (90-100%) - warning but pass - const tolerance = Math.floor(exactIncrease * 0.9); - if (actualIncrease >= tolerance && actualIncrease < exactIncrease) { - console.log(` ⚠️ WARNING: Queue increased by ${actualIncrease} (expected ${exactIncrease}, baseline: ${baseline} → ${size})`); - return true; + // Disable tolerance for very small expected increases (1-2 tracks), + // where a "90%" tolerance would allow clearly incorrect results (e.g., 0 of 1). + if (exactIncrease >= 3) { + const tolerance = Math.floor(exactIncrease * 0.9); + if (actualIncrease >= tolerance && actualIncrease < exactIncrease) { + console.log(` ⚠️ WARNING: Queue increased by ${actualIncrease} (expected ${exactIncrease}, baseline: ${baseline} → ${size})`); + return true; + } } - // Outside tolerance - fail + // Outside tolerance or small exact value - fail return `❌ FAIL: Queue increased by ${actualIncrease} (expected exactly ${exactIncrease}, baseline: ${baseline} → ${size})`; },