Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truncate file names for screenshots #2635

Merged
merged 9 commits into from
Oct 31, 2018
Merged

Truncate file names for screenshots #2635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9ecc6d6
Truncate file names for screenshots
lilaconlee Oct 22, 2018
0e46c93
Update screenshot filename truncation
lilaconlee Oct 29, 2018
ffa4704
Update unit tests
lilaconlee Oct 29, 2018
7a097e8
Change character limit to 238 chars
lilaconlee Oct 29, 2018
7b1f765
Update snapshot
lilaconlee Oct 30, 2018
75a0cad
Shorten max filename length
lilaconlee Oct 30, 2018
746270a
Merge branch 'develop' into issue-2403-truncate-screenshot-name
lilaconlee Oct 30, 2018
3da3a06
Update max filename length
lilaconlee Oct 31, 2018
d3cc2d1
Update max filename length one last time
lilaconlee Oct 31, 2018
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
17 changes: 11 additions & 6 deletions packages/server/__snapshots__/5_screenshots_spec.coffee.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 │
Expand Down Expand Up @@ -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)
Expand All @@ -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 -


`
77 changes: 44 additions & 33 deletions packages/server/lib/screenshots.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 "")
Expand All @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit funky. From testing outside of Cypress, seems like this value should be 255 - length of (failed) (N).png, but that still causes a name too long error on CI. That doesn't happen when this value is a bit smaller 🤷‍♀️

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))
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions ...ges/server/test/support/fixtures/projects/e2e/cypress/integration/screenshots_spec.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
41 changes: 10 additions & 31 deletions packages/server/test/unit/screenshots_spec.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand Down