Skip to content

Commit e2eb4f9

Browse files
committed
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
1 parent 5ac9fb5 commit e2eb4f9

File tree

2 files changed

+101
-4
lines changed

2 files changed

+101
-4
lines changed

lib/mocha/test.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,14 +153,16 @@ function cloneTest(test) {
153153
function testToFileName(test, { suffix = '', unique = false } = {}) {
154154
let fileName = test.title
155155

156-
if (unique) fileName = `${fileName}_${test?.uid || Math.floor(new Date().getTime() / 1000)}`
157-
if (suffix) fileName = `${fileName}_${suffix}`
158156
// remove tags with empty string (disable for now)
159157
// fileName = fileName.replace(/\@\w+/g, '')
160158
fileName = fileName.slice(0, 100)
161159
if (fileName.indexOf('{') !== -1) {
162160
fileName = fileName.substr(0, fileName.indexOf('{') - 3).trim()
163161
}
162+
163+
// Apply unique suffix AFTER removing data part to ensure uniqueness
164+
if (unique) fileName = `${fileName}_${test?.uid || Math.floor(new Date().getTime())}`
165+
if (suffix) fileName = `${fileName}_${suffix}`
164166
if (test.ctx && test.ctx.test && test.ctx.test.type === 'hook') fileName = clearString(`${test.title}_${test.ctx.test.title}`)
165167
// TODO: add suite title to file name
166168
// if (test.parent && test.parent.title) {

test/unit/plugin/screenshotOnFail_test.js

Lines changed: 97 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ describe('screenshotOnFail', () => {
7272
await recorder.promise()
7373
expect(screenshotSaved.called).is.ok
7474
const fileName = screenshotSaved.getCall(0).args[0]
75-
const regexpFileName = /test1_[0-9]{10}.failed.png/
75+
const regexpFileName = /test1_[0-9]{13}.failed.png/
7676
expect(fileName.match(regexpFileName).length).is.equal(1)
7777
})
7878

@@ -84,7 +84,7 @@ describe('screenshotOnFail', () => {
8484
await recorder.promise()
8585
expect(screenshotSaved.called).is.ok
8686
const fileName = screenshotSaved.getCall(0).args[0]
87-
const regexpFileName = /test1_[0-9]{10}.failed.png/
87+
const regexpFileName = /test1_[0-9]{13}.failed.png/
8888
expect(fileName.match(regexpFileName).length).is.equal(1)
8989
})
9090

@@ -139,5 +139,100 @@ describe('screenshotOnFail', () => {
139139
const screenshotFileName = screenshotSaved.getCall(0).args[0]
140140
expect(spy.getCall(0).args[1]).to.equal(screenshotFileName)
141141
})
142+
143+
describe('Data() scenarios', () => {
144+
let savedFilenames = []
145+
146+
beforeEach(() => {
147+
savedFilenames = []
148+
149+
// Override screenshotSaved to capture filenames
150+
const originalSaveScreenshot = screenshotSaved
151+
screenshotSaved = sinon.stub().callsFake(filename => {
152+
savedFilenames.push(filename)
153+
return Promise.resolve()
154+
})
155+
156+
container.clear({
157+
WebDriver: {
158+
options: {},
159+
saveScreenshot: screenshotSaved,
160+
},
161+
})
162+
})
163+
164+
it('should generate unique screenshot names for Data() iterations with uniqueScreenshotNames: true', async () => {
165+
screenshotOnFail({ uniqueScreenshotNames: true })
166+
167+
// Simulate Data() test scenario - same test title, different data
168+
const dataScenario1 = createTest('test something | {"nr":"1","url":"http://codecept.io"}')
169+
const dataScenario2 = createTest('test something | {"nr":"2","url":"http://playwright.dev"}')
170+
171+
// Both tests don't have uid (typical for Data() scenarios)
172+
dataScenario1.uid = null
173+
dataScenario2.uid = null
174+
175+
// Use fake timers to control timing but allow small progression
176+
const clock = sinon.useFakeTimers(1731340123000)
177+
178+
// Emit first failure
179+
event.dispatcher.emit(event.test.failed, dataScenario1)
180+
await recorder.promise()
181+
182+
// Advance time slightly (simulate quick succession like Data() iterations)
183+
clock.tick(100) // 100ms later
184+
185+
// Emit second failure
186+
event.dispatcher.emit(event.test.failed, dataScenario2)
187+
await recorder.promise()
188+
189+
clock.restore()
190+
191+
// Verify both screenshots were attempted
192+
expect(screenshotSaved.callCount).to.equal(2)
193+
194+
// Get the generated filenames
195+
const filename1 = savedFilenames[0]
196+
const filename2 = savedFilenames[1]
197+
198+
// Verify filenames are different (no collision)
199+
expect(filename1).to.not.equal(filename2, `Screenshot filenames should be unique for Data() iterations. Got: ${filename1} and ${filename2}`)
200+
201+
// Verify both contain the base test name (without data part)
202+
expect(filename1).to.include('test_something')
203+
expect(filename2).to.include('test_something')
204+
205+
// Verify both have unique suffixes (timestamp-based)
206+
expect(filename1).to.match(/test_something_[0-9]{13}\.failed\.png/)
207+
expect(filename2).to.match(/test_something_[0-9]{13}\.failed\.png/)
208+
})
209+
210+
it('should generate same filename for Data() iterations with uniqueScreenshotNames: false', async () => {
211+
screenshotOnFail({ uniqueScreenshotNames: false })
212+
213+
// Same scenario but without unique names
214+
const dataScenario1 = createTest('test something | {"nr":"1","url":"http://codecept.io"}')
215+
const dataScenario2 = createTest('test something | {"nr":"2","url":"http://playwright.dev"}')
216+
217+
// Emit failures
218+
event.dispatcher.emit(event.test.failed, dataScenario1)
219+
await recorder.promise()
220+
221+
event.dispatcher.emit(event.test.failed, dataScenario2)
222+
await recorder.promise()
223+
224+
// Verify both screenshots were attempted
225+
expect(screenshotSaved.callCount).to.equal(2)
226+
227+
// Get the generated filenames
228+
const filename1 = savedFilenames[0]
229+
const filename2 = savedFilenames[1]
230+
231+
// With uniqueScreenshotNames: false, both should have the same base name
232+
expect(filename1).to.equal('test_something.failed.png')
233+
expect(filename2).to.equal('test_something.failed.png')
234+
})
235+
})
236+
142237
// TODO: write more tests for different options
143238
})

0 commit comments

Comments
 (0)