Truncate file names for screenshots (#2635)

* Truncate file names for screenshots * Update screenshot filename truncation * Update unit tests * Change character limit to 238 chars * Update snapshot * Shorten max filename length * Update max filename length * Update max filename length one last time
cypress-io · Oct 31, 2018 · 5da5bd2 · 5da5bd2
1 parent 6087111
commit 5da5bd2
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 70 deletions.
diff --git a/packages/server/__snapshots__/5_screenshots_spec.coffee.js b/packages/server/__snapshots__/5_screenshots_spec.coffee.js
@@ -42,9 +42,12 @@ exports['e2e screenshots passes 1'] = `
     each hooks
       4) "before each" hook for "empty test 2"
       5) "after each" hook for "empty test 2"
+    really long test title aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+      ✓ takes a screenshot
+      ✓ takes another screenshot
 
 
-  15 passing
+  17 passing
   1 pending
   5 failing
 
@@ -80,12 +83,12 @@ Because this error occurred during a 'after each' hook we are skipping the remai
   (Results)
 
   ┌───────────────────────────────────────┐
-  │ Tests:        20                      │
-  │ Passing:      15                      │
+  │ Tests:        22                      │
+  │ Passing:      17                      │
   │ Failing:      4                       │
   │ Pending:      1                       │
   │ Skipped:      0                       │
-  │ Screenshots:  23                      │
+  │ Screenshots:  25                      │
   │ Video:        true                    │
   │ Duration:     X seconds               │
   │ Spec Ran:     screenshots_spec.coffee │
@@ -117,6 +120,8 @@ Because this error occurred during a 'after each' hook we are skipping the remai
   - /foo/bar/.projects/e2e/cypress/screenshots/screenshots_spec.coffee/taking screenshots -- before hooks -- empty test 1 -- before all hook (failed).png (1280x720)
   - /foo/bar/.projects/e2e/cypress/screenshots/screenshots_spec.coffee/taking screenshots -- each hooks -- empty test 2 -- before each hook (failed).png (1280x720)
   - /foo/bar/.projects/e2e/cypress/screenshots/screenshots_spec.coffee/taking screenshots -- each hooks -- empty test 2 -- after each hook (failed).png (1280x720)
+  - /foo/bar/.projects/e2e/cypress/screenshots/screenshots_spec.coffee/taking screenshots -- really long test title aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.png (1000x660)
+  - /foo/bar/.projects/e2e/cypress/screenshots/screenshots_spec.coffee/taking screenshots -- really long test title aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa (1).png (1000x660)
 
 
   (Video)
@@ -132,9 +137,9 @@ Because this error occurred during a 'after each' hook we are skipping the remai
 
       Spec                                                Tests  Passing  Failing  Pending  Skipped 
   ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
-  │ ✖ screenshots_spec.coffee                   XX:XX       20       15        4        1        - │
+  │ ✖ screenshots_spec.coffee                   XX:XX       22       17        4        1        - │
   └────────────────────────────────────────────────────────────────────────────────────────────────┘
-    1 of 1 failed (100%)                        XX:XX       20       15        4        1        -  
+    1 of 1 failed (100%)                        XX:XX       22       17        4        1        -  
 
 
 `
diff --git a/packages/server/lib/screenshots.coffee b/packages/server/lib/screenshots.coffee
@@ -297,12 +297,13 @@ getDimensions = (details) ->
   else
     pick(details.image.bitmap)
 
-ensureUniquePath = (takenPaths, withoutExt, extension) ->
-  fullPath = "#{withoutExt}.#{extension}"
-  num = 0
-  while _.includes(takenPaths, fullPath)
-    fullPath = "#{withoutExt} (#{++num}).#{extension}"
-  return fullPath
+ensureUniquePath = (withoutExt, extension, num = 0) ->
+  fullPath = if num then "#{withoutExt} (#{num}).#{extension}" else "#{withoutExt}.#{extension}"
+  fs.pathExists(fullPath)
+  .then (found) ->
+    if found
+      return ensureUniquePath(withoutExt, extension, num += 1)
+    return fullPath
 
 getPath = (data, ext, screenshotsFolder) ->
   specNames = (data.specName or "")
@@ -313,14 +314,24 @@ getPath = (data, ext, screenshotsFolder) ->
   else
     names = [data.titles.map(replaceInvalidChars).join(RUNNABLE_SEPARATOR)]
 
+  # truncate file names to be less than 220 characters
+  # to accomodate filename size limits
+  maxFileNameLength = 220
+  index = names.length - 1
+
+  if names[index].length > maxFileNameLength
+    names[index] = _.truncate(names[index], {
+      length: maxFileNameLength,
+      omission: ''
+    })
+
   ## append (failed) to the last name
   if data.testFailure
-    index = names.length - 1
     names[index] = names[index] + " (failed)"
 
   withoutExt = path.join(screenshotsFolder, specNames..., names...)
 
-  ensureUniquePath(data.takenPaths, withoutExt, ext)
+  ensureUniquePath(withoutExt, ext)
 
 getPathToScreenshot = (data, details, screenshotsFolder) ->
   ext = mime.extension(getType(details))
@@ -410,31 +421,31 @@ module.exports = {
       return { image, pixelRatio, multipart, takenAt }
 
   save: (data, details, screenshotsFolder) ->
-    pathToScreenshot = getPathToScreenshot(data, details, screenshotsFolder)
-
-    debug("save", pathToScreenshot)
-
-    getBuffer(details)
-    .then (buffer) ->
-      fs.outputFileAsync(pathToScreenshot, buffer)
-    .then ->
-      fs.statAsync(pathToScreenshot).get("size")
-    .then (size) ->
-      dimensions = getDimensions(details)
-
-      { multipart, pixelRatio, takenAt } = details
-
-      {
-        size
-        takenAt
-        dimensions
-        multipart
-        pixelRatio
-        name: data.name
-        specName: data.specName
-        testFailure: data.testFailure
-        path: pathToScreenshot
-      }
+    getPathToScreenshot(data, details, screenshotsFolder)
+    .then (pathToScreenshot) ->
+      debug("save", pathToScreenshot)
+
+      getBuffer(details)
+      .then (buffer) ->
+        fs.outputFileAsync(pathToScreenshot, buffer)
+      .then ->
+        fs.statAsync(pathToScreenshot).get("size")
+      .then (size) ->
+        dimensions = getDimensions(details)
+
+        { multipart, pixelRatio, takenAt } = details
+
+        {
+          size
+          takenAt
+          dimensions
+          multipart
+          pixelRatio
+          name: data.name
+          specName: data.specName
+          testFailure: data.testFailure
+          path: pathToScreenshot
+        }
 
   afterScreenshot: (data, details) ->
     duration = new Date() - new Date(data.startTime)

diff --git a/...ges/server/test/support/fixtures/projects/e2e/cypress/integration/screenshots_spec.coffee b/...ges/server/test/support/fixtures/projects/e2e/cypress/integration/screenshots_spec.coffee
@@ -239,3 +239,10 @@ describe "taking screenshots", ->
       throw new Error("after each hook failed")
 
     it "empty test 2", ->
+
+  context "really long test title #{Cypress._.repeat('a', 255)}", ->
+    it "takes a screenshot", ->
+      cy.screenshot()
+
+    it "takes another screenshot", ->
+      cy.screenshot()
diff --git a/packages/server/test/unit/screenshots_spec.coffee b/packages/server/test/unit/screenshots_spec.coffee
@@ -469,48 +469,27 @@ describe "lib/screenshots", ->
 
   context ".getPath", ->
     it "concats spec name, screenshotsFolder, and name", ->
-      p = screenshots.getPath({
+      screenshots.getPath({
         specName: "examples$/user/list.js"
         titles: ["bar", "baz"]
         name: "quux/lorem*"
       }, "png", "path/to/screenshots")
-
-      expect(p).to.eq(
-        "path/to/screenshots/examples$/user/list.js/quux/lorem.png"
-      )
-
-      p2 = screenshots.getPath({
-        specName: "examples$/user/list.js"
-        titles: ["bar", "baz"]
-        name: "quux*"
-        takenPaths: ["path/to/screenshots/examples$/user/list.js/quux.png"]
-      }, "png", "path/to/screenshots")
-
-      expect(p2).to.eq(
-        "path/to/screenshots/examples$/user/list.js/quux (1).png"
-      )
+      .then (p) ->
+        expect(p).to.eq(
+          "path/to/screenshots/examples$/user/list.js/quux/lorem.png"
+        )
 
     it "concats spec name, screenshotsFolder, and titles", ->
-      p = screenshots.getPath({
+      screenshots.getPath({
         specName: "examples$/user/list.js"
         titles: ["bar", "baz^"]
         takenPaths: ["a"]
         testFailure: true
       }, "png", "path/to/screenshots")
-
-      expect(p).to.eq(
-        "path/to/screenshots/examples$/user/list.js/bar -- baz (failed).png"
-      )
-
-      p2 = screenshots.getPath({
-        specName: "examples$/user/list.js"
-        titles: ["bar", "baz^"]
-        takenPaths: ["path/to/screenshots/examples$/user/list.js/bar -- baz.png"]
-      }, "png", "path/to/screenshots")
-
-      expect(p2).to.eq(
-        "path/to/screenshots/examples$/user/list.js/bar -- baz (1).png"
-      )
+      .then (p) ->
+        expect(p).to.eq(
+          "path/to/screenshots/examples$/user/list.js/bar -- baz (failed).png"
+        )
 
   context ".afterScreenshot", ->
     beforeEach ->