fix: prevent Data() screenshot filename collisions with uniqueScreenshotNames (#5299)

Author: mirao

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) {

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,99 @@ 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
+      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
 })