diff --git a/src/commands/run.js b/src/commands/run.js index 518566c8..84edf08a 100644 --- a/src/commands/run.js +++ b/src/commands/run.js @@ -236,6 +236,7 @@ export async function runCommand( message, environment: config.build.environment, threshold: config.comparison.threshold, + minClusterSize: config.comparison.minClusterSize, eager: config.eager || false, allowNoToken: config.allowNoToken || false, wait: config.wait || options.wait || false, diff --git a/src/tdd/tdd-service.js b/src/tdd/tdd-service.js index b954fa62..397a1e46 100644 --- a/src/tdd/tdd-service.js +++ b/src/tdd/tdd-service.js @@ -1131,6 +1131,20 @@ export class TddService { return metadata; } + /** + * Upsert a comparison result - replaces existing if same ID, otherwise appends. + * This prevents stale results from accumulating in daemon mode. + * @private + */ + _upsertComparison(result) { + let existingIndex = this.comparisons.findIndex(c => c.id === result.id); + if (existingIndex >= 0) { + this.comparisons[existingIndex] = result; + } else { + this.comparisons.push(result); + } + } + /** * Compare a screenshot against baseline */ @@ -1248,7 +1262,7 @@ export class TddService { properties: validatedProperties, }); - this.comparisons.push(result); + this._upsertComparison(result); return result; } @@ -1288,7 +1302,7 @@ export class TddService { honeydiffResult, }); - this.comparisons.push(result); + this._upsertComparison(result); return result; } else { let hotspotAnalysis = this.getHotspotForScreenshot(name); @@ -1315,7 +1329,7 @@ export class TddService { diff: diffInfo, }); - this.comparisons.push(result); + this._upsertComparison(result); return result; } } catch (error) { @@ -1356,7 +1370,7 @@ export class TddService { properties: validatedProperties, }); - this.comparisons.push(result); + this._upsertComparison(result); return result; } @@ -1371,7 +1385,7 @@ export class TddService { errorMessage: error.message, }); - this.comparisons.push(result); + this._upsertComparison(result); return result; } } @@ -1792,7 +1806,7 @@ export class TddService { signature, }; - this.comparisons.push(result); + this._upsertComparison(result); output.info(`Baseline created for ${name}`); return result; } diff --git a/tests/commands/run.test.js b/tests/commands/run.test.js index 2f2ed576..1e2de77b 100644 --- a/tests/commands/run.test.js +++ b/tests/commands/run.test.js @@ -435,6 +435,49 @@ describe('commands/run', () => { assert.strictEqual(exitCode, 1); }); + it('passes minClusterSize from config to runOptions', async () => { + // This test verifies the fix for issue #160 + let output = createMockOutput(); + let capturedRunOptions = null; + + await runCommand( + 'npm test', + {}, + {}, + { + loadConfig: async () => + createMockConfig({ + comparison: { threshold: 2.0, minClusterSize: 5 }, + }), + createServerManager: () => ({ + start: async () => {}, + stop: async () => {}, + }), + createUploader: () => ({}), + runTests: async ({ runOptions }) => { + capturedRunOptions = runOptions; + return { buildId: null, screenshotsCaptured: 0 }; + }, + detectBranch: async () => 'main', + detectCommit: async () => 'abc123', + detectCommitMessage: async () => 'test', + detectPullRequestNumber: () => null, + generateBuildNameWithGit: async () => 'test-build', + output, + exit: () => {}, + processOn: () => {}, + processRemoveListener: () => {}, + } + ); + + assert.strictEqual( + capturedRunOptions.minClusterSize, + 5, + 'minClusterSize should be passed from config to runOptions' + ); + assert.strictEqual(capturedRunOptions.threshold, 2.0); + }); + it('uses provided git metadata options', async () => { let output = createMockOutput(); let capturedRunOptions = null; diff --git a/tests/tdd/tdd-service.test.js b/tests/tdd/tdd-service.test.js index 61435cd8..58a7a34d 100644 --- a/tests/tdd/tdd-service.test.js +++ b/tests/tdd/tdd-service.test.js @@ -346,6 +346,98 @@ describe('tdd/tdd-service', () => { }); }); + describe('_upsertComparison', () => { + it('replaces existing comparison when ID matches', () => { + let mockDeps = createMockDeps(); + let service = new TddService({}, '/test', false, null, mockDeps); + + // Add initial comparison + service.comparisons = [ + { + id: 'comp-1', + name: 'homepage', + status: 'failed', + diffPercentage: 5.0, + }, + { id: 'comp-2', name: 'button', status: 'passed' }, + ]; + + // Upsert with same ID but different status + service._upsertComparison({ + id: 'comp-1', + name: 'homepage', + status: 'passed', + diffPercentage: 0, + }); + + assert.strictEqual(service.comparisons.length, 2); + assert.strictEqual(service.comparisons[0].status, 'passed'); + assert.strictEqual(service.comparisons[0].diffPercentage, 0); + }); + + it('appends new comparison when ID does not exist', () => { + let mockDeps = createMockDeps(); + let service = new TddService({}, '/test', false, null, mockDeps); + + service.comparisons = [ + { id: 'comp-1', name: 'homepage', status: 'passed' }, + ]; + + service._upsertComparison({ + id: 'comp-2', + name: 'button', + status: 'new', + }); + + assert.strictEqual(service.comparisons.length, 2); + assert.strictEqual(service.comparisons[1].id, 'comp-2'); + }); + + it('prevents stale results from accumulating in daemon mode', async () => { + // This test verifies the fix for issue #158 + // In daemon mode, re-running tests should replace old results, not accumulate them + let mockDeps = createMockDeps({ + baseline: { baselineExists: () => true }, + comparison: { + compareImages: async () => ({ + isDifferent: true, + diffPercentage: 5.5, + diffPixels: 1000, + diffClusters: [], + }), + buildFailedComparison: params => ({ + id: params.signature ? `comp-${params.signature}` : 'test-id', + status: 'failed', + ...params, + }), + }, + signature: { + generateScreenshotSignature: () => 'homepage|1920|chrome', + generateBaselineFilename: () => 'homepage_hash.png', + generateComparisonId: sig => `comp-${sig}`, + sanitizeScreenshotName: name => name, + validateScreenshotProperties: props => props, + safePath: (...parts) => parts.join('/'), + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + // First run - screenshot fails + await service.compareScreenshot('homepage', Buffer.from('test1'), {}); + assert.strictEqual(service.comparisons.length, 1); + assert.strictEqual(service.comparisons[0].status, 'failed'); + + // Second run - same screenshot, still fails (simulating daemon mode re-run) + // Without the fix, this would add a second comparison + await service.compareScreenshot('homepage', Buffer.from('test2'), {}); + assert.strictEqual( + service.comparisons.length, + 1, + 'Should still have only 1 comparison, not 2' + ); + }); + }); + describe('compareScreenshot', () => { it('creates new baseline when none exists', async () => { let savedBaseline = null;