Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/commands/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
26 changes: 20 additions & 6 deletions src/tdd/tdd-service.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -1248,7 +1262,7 @@ export class TddService {
properties: validatedProperties,
});

this.comparisons.push(result);
this._upsertComparison(result);
return result;
}

Expand Down Expand Up @@ -1288,7 +1302,7 @@ export class TddService {
honeydiffResult,
});

this.comparisons.push(result);
this._upsertComparison(result);
return result;
} else {
let hotspotAnalysis = this.getHotspotForScreenshot(name);
Expand All @@ -1315,7 +1329,7 @@ export class TddService {
diff: diffInfo,
});

this.comparisons.push(result);
this._upsertComparison(result);
return result;
}
} catch (error) {
Expand Down Expand Up @@ -1356,7 +1370,7 @@ export class TddService {
properties: validatedProperties,
});

this.comparisons.push(result);
this._upsertComparison(result);
return result;
}

Expand All @@ -1371,7 +1385,7 @@ export class TddService {
errorMessage: error.message,
});

this.comparisons.push(result);
this._upsertComparison(result);
return result;
}
}
Expand Down Expand Up @@ -1792,7 +1806,7 @@ export class TddService {
signature,
};

this.comparisons.push(result);
this._upsertComparison(result);
output.info(`Baseline created for ${name}`);
return result;
}
Expand Down
43 changes: 43 additions & 0 deletions tests/commands/run.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
92 changes: 92 additions & 0 deletions tests/tdd/tdd-service.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down