diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 3bf40d897764..227d85b7bd18 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -6,6 +6,7 @@ _Released 12/17/2024 (PENDING)_ **Bugfixes:** - Fixed an issue where targets may hang if `Network.enable` is not implemented for the target. Addresses [#29876](https://github.com/cypress-io/cypress/issues/29876). +- Prevent overwriting of video files across multiple runs. Addresses [#8280](https://github.com/cypress-io/cypress/issues/8280). Addressed in [#30673](https://github.com/cypress-io/cypress/pull/30673). - Updated Firefox `userChrome.css` to correctly hide the toolbox during headless mode. Addresses [#30721](https://github.com/cypress-io/cypress/issues/30721). ## 13.16.1 diff --git a/packages/server/lib/modes/run.ts b/packages/server/lib/modes/run.ts index db70e790eb7a..6dc14177f1d8 100644 --- a/packages/server/lib/modes/run.ts +++ b/packages/server/lib/modes/run.ts @@ -13,7 +13,7 @@ import Reporter from '../reporter' import browserUtils from '../browsers' import { openProject } from '../open_project' import * as videoCapture from '../video_capture' -import { fs } from '../util/fs' +import { fs, getPath } from '../util/fs' import runEvents from '../plugins/run_events' import env from '../util/env' import trash from '../util/trash' @@ -224,15 +224,24 @@ async function trashAssets (config: Cfg) { } } -async function startVideoRecording (options: { previous?: VideoRecording, project: Project, spec: SpecWithRelativeRoot, videosFolder: string }): Promise { +async function startVideoRecording (options: { previous?: VideoRecording, project: Project, spec: SpecWithRelativeRoot, videosFolder: string, overwrite: boolean }): Promise { if (!options.videosFolder) throw new Error('Missing videoFolder for recording') - function videoPath (suffix: string) { - return path.join(options.videosFolder, options.spec.relativeToCommonRoot + suffix) + async function videoPath (suffix: string, ext: string) { + const specPath = options.spec.relativeToCommonRoot + suffix + const data = { + name: specPath, + testFailure: false, + testAttemptIndex: 0, + titles: [], + } + + // getPath returns a Promise!!! + return await getPath(data, ext, options.videosFolder, options.overwrite) } - const videoName = videoPath('.mp4') - const compressedVideoName = videoPath('-compressed.mp4') + const videoName = await videoPath('', 'mp4') + const compressedVideoName = await videoPath('-compressed', 'mp4') const outputDir = path.dirname(videoName) @@ -333,6 +342,13 @@ async function compressRecording (options: { quiet: boolean, videoCompression: n if (options.videoCompression === false || options.videoCompression === 0) { debug('skipping compression') + // the getSafePath used to get the compressedVideoName creates the file + // in order to check if the path is safe or not. So here, if the compressed + // file exists, we remove it as compression is not enabled + if (fs.existsSync(options.processOptions.compressedVideoName)) { + await fs.remove(options.processOptions.compressedVideoName) + } + return } @@ -945,7 +961,7 @@ async function runSpec (config, spec: SpecWithRelativeRoot, options: { project: async function getVideoRecording () { if (!options.video) return undefined - const opts = { project, spec, videosFolder: options.videosFolder } + const opts = { project, spec, videosFolder: options.videosFolder, overwrite: options.config.trashAssetsBeforeRuns } telemetry.startSpan({ name: 'video:capture' }) diff --git a/packages/server/lib/screenshots.js b/packages/server/lib/screenshots.js index 79730f952a3f..80e88ffc97ee 100644 --- a/packages/server/lib/screenshots.js +++ b/packages/server/lib/screenshots.js @@ -1,30 +1,17 @@ const _ = require('lodash') const mime = require('mime') -const path = require('path') const Promise = require('bluebird') const dataUriToBuffer = require('data-uri-to-buffer') const Jimp = require('jimp') const sizeOf = require('image-size') const colorString = require('color-string') -const sanitize = require('sanitize-filename') let debug = require('debug')('cypress:server:screenshot') const plugins = require('./plugins') -const { fs } = require('./util/fs') - -const RUNNABLE_SEPARATOR = ' -- ' -const pathSeparatorRe = /[\\\/]/g +const { fs, getPath } = require('./util/fs') // 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 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 - // 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 @@ -293,83 +280,6 @@ const getDimensions = function (details) { return pick(details.image.bitmap) } -const ensureSafePath = function (withoutExt, extension, overwrite, num = 0) { - const suffix = `${(num && !overwrite) ? ` (${num})` : ''}.${extension}` - - const maxSafePrefixBytes = maxSafeBytes - suffix.length - const filenameBuf = Buffer.from(path.basename(withoutExt)) - - if (filenameBuf.byteLength > maxSafePrefixBytes) { - const truncated = filenameBuf.slice(0, maxSafePrefixBytes).toString() - - withoutExt = path.join(path.dirname(withoutExt), truncated) - } - - const fullPath = [withoutExt, suffix].join('') - - debug('ensureSafePath %o', { withoutExt, extension, num, maxSafeBytes, maxSafePrefixBytes }) - - return fs.pathExists(fullPath) - .then((found) => { - if (found && !overwrite) { - return ensureSafePath(withoutExt, extension, overwrite, num + 1) - } - - // path does not exist, attempt to create it to check for an ENAMETOOLONG error - 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 - - return ensureSafePath(withoutExt, extension, overwrite, num) - } - - throw err - }) - }) -} - -const sanitizeToString = (title) => { - // test titles may be values which aren't strings like - // null or undefined - so convert before trying to sanitize - return sanitize(_.toString(title)) -} - -const getPath = function (data, ext, screenshotsFolder, overwrite) { - let names - const specNames = (data.specName || '') - .split(pathSeparatorRe) - - if (data.name) { - names = data.name.split(pathSeparatorRe).map(sanitize) - } else { - names = _ - .chain(data.titles) - .map(sanitizeToString) - .join(RUNNABLE_SEPARATOR) - .concat([]) - .value() - } - - const index = names.length - 1 - - // append (failed) to the last name - if (data.testFailure) { - names[index] = `${names[index]} (failed)` - } - - if (data.testAttemptIndex > 0) { - names[index] = `${names[index]} (attempt ${data.testAttemptIndex + 1})` - } - - const withoutExt = path.join(screenshotsFolder, ...specNames, ...names) - - return ensureSafePath(withoutExt, ext, overwrite) -} - const getPathToScreenshot = function (data, details, screenshotsFolder) { const ext = mime.getExtension(getType(details)) diff --git a/packages/server/lib/util/fs.ts b/packages/server/lib/util/fs.ts index 5846acecc837..fb67a8f1e080 100644 --- a/packages/server/lib/util/fs.ts +++ b/packages/server/lib/util/fs.ts @@ -2,6 +2,20 @@ import Bluebird from 'bluebird' import fsExtra from 'fs-extra' +import sanitize from 'sanitize-filename' +import path from 'path' +import _ from 'lodash' + +const RUNNABLE_SEPARATOR = ' -- ' +const pathSeparatorRe = /[\\\/]/g + +// 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 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 type Promisified any> = (...params: Parameters) => Bluebird> @@ -12,6 +26,88 @@ interface PromisifiedFsExtra { readFileAsync: Promisified writeFileAsync: Promisified pathExistsAsync: Promisified + outputFileAsync: Promisified } +type PathOptions = { + specName?: string + name: string + testFailure: boolean + testAttemptIndex: number + titles: Array +}; + export const fs = Bluebird.promisifyAll(fsExtra) as PromisifiedFsExtra & typeof fsExtra + +const ensureSafePath = async function (withoutExt: string, extension: string, overwrite: boolean, num: number = 0): Promise { + const suffix = `${(num && !overwrite) ? ` (${num})` : ''}.${extension}` + + const maxSafePrefixBytes = maxSafeBytes - suffix.length + const filenameBuf = Buffer.from(path.basename(withoutExt)) + + if (filenameBuf.byteLength > maxSafePrefixBytes) { + const truncated = filenameBuf.slice(0, maxSafePrefixBytes).toString() + + withoutExt = path.join(path.dirname(withoutExt), truncated) + } + + const fullPath = [withoutExt, suffix].join('') + + return fs.pathExists(fullPath) + .then((found) => { + if (found && !overwrite) { + return ensureSafePath(withoutExt, extension, overwrite, num + 1) + } + + // 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, overwrite, num) + } + + throw err + }) + }) +} + +const sanitizeToString = (title: any, idx: number, arr: Array) => { + // test titles may be values which aren't strings like + // null or undefined - so convert before trying to sanitize + return sanitize(_.toString(title)) +} + +export const getPath = async function (data: PathOptions, ext: string, screenshotsFolder: string, overwrite: boolean): Promise { + let names + const specNames = (data.specName || '') + .split(pathSeparatorRe) + + if (data.name) { + names = data.name.split(pathSeparatorRe).map(sanitizeToString) + } else { + // we put this in array so to match with type of the if branch above + names = [_ + .chain(data.titles) + .map(sanitizeToString) + .join(RUNNABLE_SEPARATOR) + .value()] + } + + const index = names.length - 1 + + // append '(failed)' to the last name + if (data.testFailure) { + names[index] = `${names[index]} (failed)` + } + + if (data.testAttemptIndex > 0) { + names[index] = `${names[index]} (attempt ${data.testAttemptIndex + 1})` + } + + const withoutExt = path.join(screenshotsFolder, ...specNames, ...names) + + return await ensureSafePath(withoutExt, ext, overwrite) +} diff --git a/system-tests/README.md b/system-tests/README.md index 1d9d5b278cc0..78eb297a6d62 100644 --- a/system-tests/README.md +++ b/system-tests/README.md @@ -10,23 +10,23 @@ These tests run in CI in Electron, Chrome, Firefox, and WebKit under the `system ## Running System Tests ```bash -yarn test # runs all tests +yarn test-system # runs all tests ## or use globbing to find spec in folders as defined in "glob-in-dir" param in package.json -yarn test screenshot*element # runs screenshot_element_capture_spec.js -yarn test screenshot # runs screenshot_element_capture_spec.js, screenshot_fullpage_capture_spec.js, ..., etc. +yarn test-system screenshot*element # runs screenshot_element_capture_spec.js +yarn test-system screenshot # runs screenshot_element_capture_spec.js, screenshot_fullpage_capture_spec.js, ..., etc. ``` To keep the browser open after a spec run (for easier debugging and iterating on specs), you can pass the `--no-exit` flag to the test command. Live reloading due to spec changes should also work: ```sh -yarn test go_spec.js --browser chrome --no-exit +yarn test-system go_spec.js --browser chrome --no-exit ``` To debug the Cypress process under test, you can pass `--cypress-inspect-brk`: ```sh -yarn test go_spec.js --browser chrome --no-exit --cypress-inspect-brk +yarn test-system go_spec.js --browser chrome --no-exit --cypress-inspect-brk ``` ## Developing Tests diff --git a/system-tests/projects/issue-8280-retain-video/cypress.config.js b/system-tests/projects/issue-8280-retain-video/cypress.config.js new file mode 100644 index 000000000000..56bd22847ce1 --- /dev/null +++ b/system-tests/projects/issue-8280-retain-video/cypress.config.js @@ -0,0 +1,6 @@ +module.exports = { + e2e: { + supportFile: false, + video: true, + }, +} diff --git a/system-tests/projects/issue-8280-retain-video/cypress/e2e/spec1.cy.js b/system-tests/projects/issue-8280-retain-video/cypress/e2e/spec1.cy.js new file mode 100644 index 000000000000..8908e841df82 --- /dev/null +++ b/system-tests/projects/issue-8280-retain-video/cypress/e2e/spec1.cy.js @@ -0,0 +1,13 @@ +// here the delays are just so there is something in the screenshots and recordings. + +describe('spec1', () => { + it('testCase1', () => { + cy.wait(500) + assert(false) + }) + + it('testCase2', () => { + cy.wait(500) + assert(true) + }) +}) diff --git a/system-tests/projects/issue-8280-retain-video/cypress/e2e/spec2.cy.js b/system-tests/projects/issue-8280-retain-video/cypress/e2e/spec2.cy.js new file mode 100644 index 000000000000..4a7edb96217c --- /dev/null +++ b/system-tests/projects/issue-8280-retain-video/cypress/e2e/spec2.cy.js @@ -0,0 +1,13 @@ +// here the delays are just so there is something in the screenshots and recordings. + +describe('spec2', () => { + it('testCase1', () => { + cy.wait(500) + assert(false) + }) + + it('testCase2', () => { + cy.wait(500) + assert(true) + }) +}) diff --git a/system-tests/test/issue_8280_spec.js b/system-tests/test/issue_8280_spec.js new file mode 100644 index 000000000000..a113ee843c44 --- /dev/null +++ b/system-tests/test/issue_8280_spec.js @@ -0,0 +1,91 @@ +const { fs } = require('@packages/server/lib/util/fs') +const Fixtures = require('../lib/fixtures') +const systemTests = require('../lib/system-tests').default + +const PROJECT_NAME = 'issue-8280-retain-video' + +describe('e2e issue 8280', () => { + systemTests.setup() + + // https://github.com/cypress-io/cypress/issues/8280 + + it('should retain the videos from previous runs if trashAssetsBeforeRuns=false', function () { + return systemTests.exec(this, { + project: PROJECT_NAME, + snapshot: false, + expectedExitCode: 2, + processEnv: { + 'CYPRESS_trashAssetsBeforeRuns': 'false', + }, + }) + .then(() => { + return systemTests.exec(this, { + project: PROJECT_NAME, + snapshot: false, + expectedExitCode: 2, + processEnv: { + 'CYPRESS_trashAssetsBeforeRuns': 'false', + }, + }) + }).then(() => { + return fs.readdir(Fixtures.projectPath(`${PROJECT_NAME}/cypress/screenshots/spec1.cy.js`)) + }).then((screenshots) => { + expect(screenshots.length).to.eq(2) + expect(screenshots).to.include('spec1 -- testCase1 (failed).png') + expect(screenshots).to.include('spec1 -- testCase1 (failed) (1).png') + }).then(() => { + return fs.readdir(Fixtures.projectPath(`${PROJECT_NAME}/cypress/screenshots/spec2.cy.js`)) + }).then((screenshots) => { + expect(screenshots.length).to.eq(2) + expect(screenshots).to.include('spec2 -- testCase1 (failed).png') + expect(screenshots).to.include('spec2 -- testCase1 (failed) (1).png') + }) + .then(() => { + return fs.readdir(Fixtures.projectPath(`${PROJECT_NAME}/cypress/videos`)) + }).then((videos) => { + expect(videos.length).to.eq(4) + expect(videos).to.include('spec1.cy.js.mp4') + expect(videos).to.include('spec1.cy.js (1).mp4') + expect(videos).to.include('spec2.cy.js.mp4') + expect(videos).to.include('spec2.cy.js (1).mp4') + }) + }) + + // if trash assets = true, then there will be no retention of screenshots or videos + it('should not retain the videos from previous runs if trashAssetsBeforeRuns=true', function () { + return systemTests.exec(this, { + project: PROJECT_NAME, + snapshot: false, + expectedExitCode: 2, + processEnv: { + 'CYPRESS_trashAssetsBeforeRuns': 'true', + }, + }) + .then(() => { + return systemTests.exec(this, { + project: PROJECT_NAME, + snapshot: false, + expectedExitCode: 2, + processEnv: { + 'CYPRESS_trashAssetsBeforeRuns': 'true', + }, + }) + }).then(() => { + return fs.readdir(Fixtures.projectPath(`${PROJECT_NAME}/cypress/screenshots/spec1.cy.js`)) + }).then((screenshots) => { + expect(screenshots.length).to.eq(1) + expect(screenshots).to.include('spec1 -- testCase1 (failed).png') + }).then(() => { + return fs.readdir(Fixtures.projectPath(`${PROJECT_NAME}/cypress/screenshots/spec2.cy.js`)) + }).then((screenshots) => { + expect(screenshots.length).to.eq(1) + expect(screenshots).to.include('spec2 -- testCase1 (failed).png') + }).then(() => { + return fs.readdir(Fixtures.projectPath(`${PROJECT_NAME}/cypress/videos`)) + }).then((videos) => { + expect(videos.length).to.eq(2) + expect(videos).to.include('spec1.cy.js.mp4') + expect(videos).to.include('spec2.cy.js.mp4') + }) + }) +})