From e2eb4f902af13e72c86c3ba3cba3c7f0dd87a241 Mon Sep 17 00:00:00 2001 From: Jaromir Obr Date: Tue, 11 Nov 2025 13:04:24 +0000 Subject: [PATCH] fix: prevent Data() screenshot filename collisions with uniqueScreenshotNames - Fix timestamp precision from seconds to milliseconds in testToFileName - Move unique suffix application after data part removal to preserve uniqueness - Add comprehensive unit tests for Data() scenario collision prevention - Ensure each Data() iteration gets unique screenshot filename instead of overwriting Resolves issue where Data() test iterations were overwriting screenshots with identical filenames when uniqueScreenshotNames: true was enabled. Fixes #5298 --- lib/mocha/test.js | 6 +- test/unit/plugin/screenshotOnFail_test.js | 99 ++++++++++++++++++++++- 2 files changed, 101 insertions(+), 4 deletions(-) diff --git a/lib/mocha/test.js b/lib/mocha/test.js index e4a33f346..8df34b0f0 100644 --- a/lib/mocha/test.js +++ b/lib/mocha/test.js @@ -153,14 +153,16 @@ function cloneTest(test) { function testToFileName(test, { suffix = '', unique = false } = {}) { let fileName = test.title - if (unique) fileName = `${fileName}_${test?.uid || Math.floor(new Date().getTime() / 1000)}` - if (suffix) fileName = `${fileName}_${suffix}` // remove tags with empty string (disable for now) // fileName = fileName.replace(/\@\w+/g, '') fileName = fileName.slice(0, 100) if (fileName.indexOf('{') !== -1) { fileName = fileName.substr(0, fileName.indexOf('{') - 3).trim() } + + // Apply unique suffix AFTER removing data part to ensure uniqueness + if (unique) fileName = `${fileName}_${test?.uid || Math.floor(new Date().getTime())}` + if (suffix) fileName = `${fileName}_${suffix}` if (test.ctx && test.ctx.test && test.ctx.test.type === 'hook') fileName = clearString(`${test.title}_${test.ctx.test.title}`) // TODO: add suite title to file name // if (test.parent && test.parent.title) { diff --git a/test/unit/plugin/screenshotOnFail_test.js b/test/unit/plugin/screenshotOnFail_test.js index c17f47c5c..3629a9a17 100644 --- a/test/unit/plugin/screenshotOnFail_test.js +++ b/test/unit/plugin/screenshotOnFail_test.js @@ -72,7 +72,7 @@ describe('screenshotOnFail', () => { await recorder.promise() expect(screenshotSaved.called).is.ok const fileName = screenshotSaved.getCall(0).args[0] - const regexpFileName = /test1_[0-9]{10}.failed.png/ + const regexpFileName = /test1_[0-9]{13}.failed.png/ expect(fileName.match(regexpFileName).length).is.equal(1) }) @@ -84,7 +84,7 @@ describe('screenshotOnFail', () => { await recorder.promise() expect(screenshotSaved.called).is.ok const fileName = screenshotSaved.getCall(0).args[0] - const regexpFileName = /test1_[0-9]{10}.failed.png/ + const regexpFileName = /test1_[0-9]{13}.failed.png/ expect(fileName.match(regexpFileName).length).is.equal(1) }) @@ -139,5 +139,100 @@ describe('screenshotOnFail', () => { const screenshotFileName = screenshotSaved.getCall(0).args[0] expect(spy.getCall(0).args[1]).to.equal(screenshotFileName) }) + + describe('Data() scenarios', () => { + let savedFilenames = [] + + beforeEach(() => { + savedFilenames = [] + + // Override screenshotSaved to capture filenames + const originalSaveScreenshot = screenshotSaved + screenshotSaved = sinon.stub().callsFake(filename => { + savedFilenames.push(filename) + return Promise.resolve() + }) + + container.clear({ + WebDriver: { + options: {}, + saveScreenshot: screenshotSaved, + }, + }) + }) + + it('should generate unique screenshot names for Data() iterations with uniqueScreenshotNames: true', async () => { + screenshotOnFail({ uniqueScreenshotNames: true }) + + // Simulate Data() test scenario - same test title, different data + const dataScenario1 = createTest('test something | {"nr":"1","url":"http://codecept.io"}') + const dataScenario2 = createTest('test something | {"nr":"2","url":"http://playwright.dev"}') + + // Both tests don't have uid (typical for Data() scenarios) + dataScenario1.uid = null + dataScenario2.uid = null + + // Use fake timers to control timing but allow small progression + const clock = sinon.useFakeTimers(1731340123000) + + // Emit first failure + event.dispatcher.emit(event.test.failed, dataScenario1) + await recorder.promise() + + // Advance time slightly (simulate quick succession like Data() iterations) + clock.tick(100) // 100ms later + + // Emit second failure + event.dispatcher.emit(event.test.failed, dataScenario2) + await recorder.promise() + + clock.restore() + + // Verify both screenshots were attempted + expect(screenshotSaved.callCount).to.equal(2) + + // Get the generated filenames + const filename1 = savedFilenames[0] + const filename2 = savedFilenames[1] + + // Verify filenames are different (no collision) + expect(filename1).to.not.equal(filename2, `Screenshot filenames should be unique for Data() iterations. Got: ${filename1} and ${filename2}`) + + // Verify both contain the base test name (without data part) + expect(filename1).to.include('test_something') + expect(filename2).to.include('test_something') + + // Verify both have unique suffixes (timestamp-based) + expect(filename1).to.match(/test_something_[0-9]{13}\.failed\.png/) + expect(filename2).to.match(/test_something_[0-9]{13}\.failed\.png/) + }) + + it('should generate same filename for Data() iterations with uniqueScreenshotNames: false', async () => { + screenshotOnFail({ uniqueScreenshotNames: false }) + + // Same scenario but without unique names + const dataScenario1 = createTest('test something | {"nr":"1","url":"http://codecept.io"}') + const dataScenario2 = createTest('test something | {"nr":"2","url":"http://playwright.dev"}') + + // Emit failures + event.dispatcher.emit(event.test.failed, dataScenario1) + await recorder.promise() + + event.dispatcher.emit(event.test.failed, dataScenario2) + await recorder.promise() + + // Verify both screenshots were attempted + expect(screenshotSaved.callCount).to.equal(2) + + // Get the generated filenames + const filename1 = savedFilenames[0] + const filename2 = savedFilenames[1] + + // With uniqueScreenshotNames: false, both should have the same base name + expect(filename1).to.equal('test_something.failed.png') + expect(filename2).to.equal('test_something.failed.png') + }) + }) + // TODO: write more tests for different options })