From cc519a42de34821d6ef484fb5b0f590392f6351c Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Tue, 4 Aug 2020 14:05:45 -0400 Subject: [PATCH 1/5] fix: truncate screenshot filenames using byteLength --- .../__snapshots__/5_screenshots_spec.js | 4 +-- packages/server/lib/screenshots.js | 28 +++++++++++-------- packages/server/test/unit/screenshots_spec.js | 20 ++++++++++++- 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/packages/server/__snapshots__/5_screenshots_spec.js b/packages/server/__snapshots__/5_screenshots_spec.js index e4fef3d1c010..123a64036d63 100644 --- a/packages/server/__snapshots__/5_screenshots_spec.js +++ b/packages/server/__snapshots__/5_screenshots_spec.js @@ -147,11 +147,11 @@ Because this error occurred during a \`after each\` hook we are skipping the rem - /XXX/XXX/XXX/cypress/screenshots/screenshots_spec.js/taking screenshots -- reall (1000x660) y long test title aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.png + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.png - /XXX/XXX/XXX/cypress/screenshots/screenshots_spec.js/taking screenshots -- reall (1000x660) y long test title aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa (1).png + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa (1).png (Video) diff --git a/packages/server/lib/screenshots.js b/packages/server/lib/screenshots.js index d5974c6991bf..58175b4e5683 100644 --- a/packages/server/lib/screenshots.js +++ b/packages/server/lib/screenshots.js @@ -289,12 +289,26 @@ const getDimensions = function (details) { } const ensureUniquePath = function (withoutExt, extension, num = 0) { - const fullPath = num ? `${withoutExt} (${num}).${extension}` : `${withoutExt}.${extension}` + const suffix = `${num ? ` (${num})` : ''}.${extension}` + // many filesystems limit filename length to 255 bytes/characters, so truncate the filename to + // the smallest common denominator of safe filenames, which is 255 bytes + // @see https://github.com/cypress-io/cypress/issues/2403 + // @see https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits + const maxSafeBytes = 255 - suffix.length + const filenameBuf = Buffer.from(path.basename(withoutExt)) + + if (filenameBuf.byteLength > maxSafeBytes) { + const truncated = filenameBuf.slice(0, maxSafeBytes).toString() + + withoutExt = path.join(path.dirname(withoutExt), truncated) + } + + const fullPath = [withoutExt, suffix].join('') return fs.pathExists(fullPath) .then((found) => { if (found) { - return ensureUniquePath(withoutExt, extension, (num += 1)) + return ensureUniquePath(withoutExt, extension, num + 1) } return fullPath @@ -323,18 +337,8 @@ const getPath = function (data, ext, screenshotsFolder) { .value() } - // truncate file names to be less than 220 characters - // to accomodate filename size limits - const maxFileNameLength = 220 const 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) { names[index] = `${names[index]} (failed)` diff --git a/packages/server/test/unit/screenshots_spec.js b/packages/server/test/unit/screenshots_spec.js index e14c68da4c96..8864bc5fd655 100644 --- a/packages/server/test/unit/screenshots_spec.js +++ b/packages/server/test/unit/screenshots_spec.js @@ -618,6 +618,24 @@ describe('lib/screenshots', () => { }) }) + // @see https://github.com/cypress-io/cypress/issues/2403 + it('truncates long paths with unicode in them', async () => { + const fullPath = await screenshots.getPath({ + titles: [ + 'WMED: [STORY] Тестовые сценарии для CI', + 'Сценарии:', + 'Сценарий 2: Создание обращения, создание медзаписи, привязка обращения к медзаписи', + '- Сценарий 2', + ], + testFailure: true, + specName: 'WMED_UAT_Scenarios_For_CI_spec.js', + }, 'png', '/jenkins-slave/workspace/test-wmed/qa/cypress/wmed_ci/cypress/screenshots/') + + const basename = path.basename(fullPath) + + expect(Buffer.from(basename).byteLength).to.be.lessThan(255) + }) + _.each([Infinity, 0 / 0, [], {}, 1, false], (value) => { it(`doesn't err and stringifies non-string test title: ${value}`, () => { return screenshots.getPath({ @@ -632,7 +650,7 @@ describe('lib/screenshots', () => { }) }) - return _.each([null, undefined], (value) => { + _.each([null, undefined], (value) => { it(`doesn't err and removes null/undefined test title: ${value}`, () => { return screenshots.getPath({ specName: 'examples$/user/list.js', From b7302c761b98a7c11ce44ce6a4b400ca91b5fc7c Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Thu, 6 Aug 2020 14:00:44 -0400 Subject: [PATCH 2/5] react to ENAMETOOLONG errors --- packages/server/lib/screenshots.js | 37 +++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/packages/server/lib/screenshots.js b/packages/server/lib/screenshots.js index 58175b4e5683..f149731d5798 100644 --- a/packages/server/lib/screenshots.js +++ b/packages/server/lib/screenshots.js @@ -18,6 +18,14 @@ const pathSeparatorRe = /[\\\/]/g // internal id incrementor let __ID__ = null +// many filesystems limit filename length to 255 bytes/characters, so truncate the filename to +// the smallest common denominator of safe filenames, which is 255 bytes. when ENAMETOOLONG +// errors are encountered, `maxSafeBytes` will be decremented to at most `MIN_PREFIX_BYTES`, at +// which point the latest ENAMTOOLONG error will be emitted. +// @see https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits +let maxSafeBytes = 255 +const MIN_PREFIX_BYTES = 64 + // TODO: when we parallelize these builds we'll need // a semaphore to access the file system when we write // screenshots since its possible two screenshots with @@ -288,17 +296,13 @@ const getDimensions = function (details) { return pick(details.image.bitmap) } -const ensureUniquePath = function (withoutExt, extension, num = 0) { +const ensureSafePath = function (withoutExt, extension, num = 0) { const suffix = `${num ? ` (${num})` : ''}.${extension}` - // many filesystems limit filename length to 255 bytes/characters, so truncate the filename to - // the smallest common denominator of safe filenames, which is 255 bytes - // @see https://github.com/cypress-io/cypress/issues/2403 - // @see https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits - const maxSafeBytes = 255 - suffix.length + const maxSafePrefixBytes = maxSafeBytes - suffix.length const filenameBuf = Buffer.from(path.basename(withoutExt)) - if (filenameBuf.byteLength > maxSafeBytes) { - const truncated = filenameBuf.slice(0, maxSafeBytes).toString() + if (filenameBuf.byteLength > maxSafePrefixBytes) { + const truncated = filenameBuf.slice(0, maxSafePrefixBytes).toString() withoutExt = path.join(path.dirname(withoutExt), truncated) } @@ -308,10 +312,21 @@ const ensureUniquePath = function (withoutExt, extension, num = 0) { return fs.pathExists(fullPath) .then((found) => { if (found) { - return ensureUniquePath(withoutExt, extension, num + 1) + return ensureSafePath(withoutExt, extension, num + 1) } - return fullPath + // path does not exist, attempt to create it to check for an ENAMETOOLONG error + return fs.outputFileAsync(fullPath, '') + .then(() => fullPath) + .catch((err) => { + if (err.code === 'ENAMETOOLONG' && maxSafePrefixBytes >= MIN_PREFIX_BYTES) { + maxSafeBytes -= 1 + + return ensureSafePath(withoutExt, extension, num) + } + + throw err + }) }) } @@ -346,7 +361,7 @@ const getPath = function (data, ext, screenshotsFolder) { const withoutExt = path.join(screenshotsFolder, ...specNames, ...names) - return ensureUniquePath(withoutExt, ext) + return ensureSafePath(withoutExt, ext) } const getPathToScreenshot = function (data, details, screenshotsFolder) { From 34985b27beb2baa3aeadd2bcf884e0d8ade4f096 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Thu, 6 Aug 2020 14:32:07 -0400 Subject: [PATCH 3/5] override max bytes for e2e tests --- .../__snapshots__/5_screenshots_spec.js | 4 +-- packages/server/lib/screenshots.js | 2 +- packages/server/test/support/helpers/e2e.ts | 2 ++ packages/server/test/unit/screenshots_spec.js | 34 ++++++++++++++++++- 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/packages/server/__snapshots__/5_screenshots_spec.js b/packages/server/__snapshots__/5_screenshots_spec.js index 123a64036d63..e9fcb36a960f 100644 --- a/packages/server/__snapshots__/5_screenshots_spec.js +++ b/packages/server/__snapshots__/5_screenshots_spec.js @@ -147,11 +147,11 @@ Because this error occurred during a \`after each\` hook we are skipping the rem - /XXX/XXX/XXX/cypress/screenshots/screenshots_spec.js/taking screenshots -- reall (1000x660) y long test title aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.png + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.png - /XXX/XXX/XXX/cypress/screenshots/screenshots_spec.js/taking screenshots -- reall (1000x660) y long test title aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa (1).png + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa (1).png (Video) diff --git a/packages/server/lib/screenshots.js b/packages/server/lib/screenshots.js index f149731d5798..99489e3b37ed 100644 --- a/packages/server/lib/screenshots.js +++ b/packages/server/lib/screenshots.js @@ -23,7 +23,7 @@ let __ID__ = null // errors are encountered, `maxSafeBytes` will be decremented to at most `MIN_PREFIX_BYTES`, at // which point the latest ENAMTOOLONG error will be emitted. // @see https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits -let maxSafeBytes = 255 +let maxSafeBytes = Number(process.env.CYPRESS_MAX_SAFE_FILENAME_BYTES) || 254 const MIN_PREFIX_BYTES = 64 // TODO: when we parallelize these builds we'll need diff --git a/packages/server/test/support/helpers/e2e.ts b/packages/server/test/support/helpers/e2e.ts index 2630569b8dc2..45098c044214 100644 --- a/packages/server/test/support/helpers/e2e.ts +++ b/packages/server/test/support/helpers/e2e.ts @@ -697,6 +697,8 @@ const e2e = { LINES: 24, }) .defaults({ + // match CircleCI's filesystem limits, so screenshot names in snapshots match + CYPRESS_MAX_SAFE_FILENAME_BYTES: 242, FAKE_CWD_PATH: '/XXX/XXX/XXX', DEBUG_COLORS: '1', // prevent any Compression progress diff --git a/packages/server/test/unit/screenshots_spec.js b/packages/server/test/unit/screenshots_spec.js index 8864bc5fd655..ef193f0ed030 100644 --- a/packages/server/test/unit/screenshots_spec.js +++ b/packages/server/test/unit/screenshots_spec.js @@ -577,6 +577,10 @@ describe('lib/screenshots', () => { }) context('.getPath', () => { + beforeEach(() => { + sinon.stub(fs, 'outputFileAsync').resolves() + }) + it('concats spec name, screenshotsFolder, and name', () => { return screenshots.getPath({ specName: 'examples/user/list.js', @@ -624,7 +628,7 @@ describe('lib/screenshots', () => { titles: [ 'WMED: [STORY] Тестовые сценарии для CI', 'Сценарии:', - 'Сценарий 2: Создание обращения, создание медзаписи, привязка обращения к медзаписи', + 'Сценарий 2: Создание обращения, создание медзаписи, привязкапривязка обращения к медзаписи', '- Сценарий 2', ], testFailure: true, @@ -636,6 +640,34 @@ describe('lib/screenshots', () => { expect(Buffer.from(basename).byteLength).to.be.lessThan(255) }) + it('reacts to ENAMETOOLONG errors and tries to shorten the filename', async () => { + const err = new Error('enametoolong') + + err.code = 'ENAMETOOLONG' + + _.times(50, (i) => fs.outputFileAsync.onCall(i).rejects(err)) + + const fullPath = await screenshots.getPath({ + specName: 'foo.js', + name: 'a'.repeat(256), + }, 'png', '/tmp') + + expect(path.basename(fullPath)).to.have.length(204) + }) + + it('rejects with ENAMETOOLONG errors if name goes below MIN_PREFIX_LENGTH', async () => { + const err = new Error('enametoolong') + + err.code = 'ENAMETOOLONG' + + _.times(150, (i) => fs.outputFileAsync.onCall(i).rejects(err)) + + await expect(screenshots.getPath({ + specName: 'foo.js', + name: 'a'.repeat(256), + }, 'png', '/tmp')).to.be.rejectedWith(err) + }) + _.each([Infinity, 0 / 0, [], {}, 1, false], (value) => { it(`doesn't err and stringifies non-string test title: ${value}`, () => { return screenshots.getPath({ From c43e693eb310eae99bae79769ec22dcc74c46285 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Mon, 10 Aug 2020 11:54:28 -0400 Subject: [PATCH 4/5] Update packages/server/lib/screenshots.js Co-authored-by: Jennifer Shehane --- packages/server/lib/screenshots.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/lib/screenshots.js b/packages/server/lib/screenshots.js index 99489e3b37ed..d2eda91958ea 100644 --- a/packages/server/lib/screenshots.js +++ b/packages/server/lib/screenshots.js @@ -21,7 +21,7 @@ let __ID__ = null // many filesystems limit filename length to 255 bytes/characters, so truncate the filename to // the smallest common denominator of safe filenames, which is 255 bytes. when ENAMETOOLONG // errors are encountered, `maxSafeBytes` will be decremented to at most `MIN_PREFIX_BYTES`, at -// which point the latest ENAMTOOLONG error will be emitted. +// which point the latest ENAMETOOLONG error will be emitted. // @see https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits let maxSafeBytes = Number(process.env.CYPRESS_MAX_SAFE_FILENAME_BYTES) || 254 const MIN_PREFIX_BYTES = 64 From dc57c16a4b79241dc011fa7e242824f591c78238 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Mon, 10 Aug 2020 12:09:27 -0400 Subject: [PATCH 5/5] add some debug logs --- packages/server/lib/screenshots.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/server/lib/screenshots.js b/packages/server/lib/screenshots.js index d2eda91958ea..2d453e0f3e1d 100644 --- a/packages/server/lib/screenshots.js +++ b/packages/server/lib/screenshots.js @@ -309,6 +309,8 @@ const ensureSafePath = function (withoutExt, extension, num = 0) { const fullPath = [withoutExt, suffix].join('') + debug('ensureSafePath %o', { withoutExt, extension, num, maxSafeBytes, maxSafePrefixBytes }) + return fs.pathExists(fullPath) .then((found) => { if (found) { @@ -319,6 +321,8 @@ const ensureSafePath = function (withoutExt, extension, num = 0) { return fs.outputFileAsync(fullPath, '') .then(() => fullPath) .catch((err) => { + debug('received error when testing path %o', { err, fullPath, maxSafePrefixBytes, maxSafeBytes }) + if (err.code === 'ENAMETOOLONG' && maxSafePrefixBytes >= MIN_PREFIX_BYTES) { maxSafeBytes -= 1