fix: prevent overwriting of video files

cli/CHANGELOG.md

@@ -6,6 +6,7 @@ _Released 1/28/2025 (PENDING)_
 **Bugfixes:**
 
 - Fixed an issue where Cypress would incorrectly navigate to `about:blank` when test isolation was disabled and the last test would fail and then retry. Fixes [#28527](https://github.com/cypress-io/cypress/issues/28527).
+- Fixed an issue where the configuration setting `trashAssetsBeforeRuns=false` was ignored for assets in the `videosfolder` and these assets were incorrectly deleted before running tests with `cypress run`. Addresses [#8280](https://github.com/cypress-io/cypress/issues/8280).
 
 **Misc:**
 

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, type Data } from '../util/fs'
 import runEvents from '../plugins/run_events'
 import env from '../util/env'
 import trash from '../util/trash'
@@ -224,15 +224,25 @@ async function trashAssets (config: Cfg) {
   }
 }
 
-async function startVideoRecording (options: { previous?: VideoRecording, project: Project, spec: SpecWithRelativeRoot, videosFolder: string }): Promise<VideoRecording> {
+async function startVideoRecording (options: { previous?: VideoRecording, project: Project, spec: SpecWithRelativeRoot, videosFolder: string, overwrite: boolean }): Promise<VideoRecording> {
   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
+    // tslint:disable-next-line
+    const data: 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 +343,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
   }
 
@@ -947,7 +964,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' })
 

packages/server/lib/screenshots.ts

@@ -1,54 +1,20 @@
 import _ from 'lodash'
 import Debug from 'debug'
 import mime from 'mime'
-import path from 'path'
 import Promise from 'bluebird'
 import dataUriToBuffer from 'data-uri-to-buffer'
 import Jimp from 'jimp'
 import sizeOf from 'image-size'
 import colorString from 'color-string'
-import sanitize from 'sanitize-filename'
 import * as plugins from './plugins'
-import { fs } from './util/fs'
+import { fs, getPath } from './util/fs'
+import type { Data, ScreenshotsFolder } from './util/fs'
 
 let debug = Debug('cypress:server:screenshot')
-const RUNNABLE_SEPARATOR = ' -- '
-const pathSeparatorRe = /[\\\/]/g
 
 // internal id incrementor
 let __ID__: string | null = null
 
-type ScreenshotsFolder = string | false | undefined
-
-interface Clip {
-  x: number
-  y: number
-  width: number
-  height: number
-}
-
-// TODO: This is likely not representative of the entire Type and should be updated
-interface Data {
-  specName: string
-  name: string
-  startTime: Date
-  viewport: {
-    width: number
-    height: number
-  }
-  titles?: string[]
-  testFailure?: boolean
-  overwrite?: boolean
-  simple?: boolean
-  current?: number
-  total?: number
-  testAttemptIndex?: number
-  appOnly?: boolean
-  hideRunnerUi?: boolean
-  clip?: Clip
-  userClip?: Clip
-}
-
 // TODO: This is likely not representative of the entire Type and should be updated
 interface Details {
   image: any
@@ -70,14 +36,6 @@ interface SavedDetails {
   path?: string
 }
 
-// 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
@@ -361,92 +319,6 @@ const getDimensions = function (details) {
   return pick(details.image.bitmap)
 }
 
-const ensureSafePath = function (withoutExt: string, extension: string, overwrite: Data['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
-    // @ts-expect-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: string | null | undefined) => {
-  // 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: Data, ext, screenshotsFolder: ScreenshotsFolder, overwrite: Data['overwrite']) {
-  let names
-  const specNames = (data.specName || '')
-  .split(pathSeparatorRe)
-
-  if (data.name) {
-    // @ts-expect-error
-    names = data.name.split(pathSeparatorRe).map(sanitize)
-  } else {
-    names = _
-    .chain(data.titles)
-    .map(sanitizeToString)
-    .join(RUNNABLE_SEPARATOR)
-    // @ts-expect-error - this shouldn't be necessary, but it breaks if you remove it
-    .concat([])
-    .value()
-  }
-
-  const index = names.length - 1
-
-  // append (failed) to the last name
-  if (data.testFailure) {
-    names[index] = `${names[index]} (failed)`
-  }
-
-  if (data.testAttemptIndex && data.testAttemptIndex > 0) {
-    names[index] = `${names[index]} (attempt ${data.testAttemptIndex + 1})`
-  }
-
-  let withoutExt
-
-  if (screenshotsFolder) {
-    withoutExt = path.join(screenshotsFolder, ...specNames, ...names)
-  } else {
-    withoutExt = path.join(...specNames, ...names)
-  }
-
-  return ensureSafePath(withoutExt, ext, overwrite)
-}
-
 const getPathToScreenshot = function (data: Data, details: Details, screenshotsFolder: ScreenshotsFolder) {
   const ext = mime.getExtension(getType(details))
 

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<T extends (...args: any) => any>
   = (...params: Parameters<T>) => Bluebird<ReturnType<T>>
@@ -12,6 +26,117 @@ interface PromisifiedFsExtra {
   readFileAsync: Promisified<typeof fsExtra.readFileSync>
   writeFileAsync: Promisified<typeof fsExtra.writeFileSync>
   pathExistsAsync: Promisified<typeof fsExtra.pathExistsSync>
+  outputFileAsync: Promisified<typeof fsExtra.outputFileSync>
+}
+
+interface Clip {
+  x: number
+  y: number
+  width: number
+  height: number
+}
+
+export type ScreenshotsFolder = string | false | undefined
+
+// TODO: This is likely not representative of the entire Type and should be updated
+export interface Data {
+  name: string
+  startTime: Date
+  specName?: string
+  viewport?: {
+    width: number
+    height: number
+  }
+  titles?: string[]
+  testFailure?: boolean
+  overwrite?: boolean
+  simple?: boolean
+  current?: number
+  total?: number
+  testAttemptIndex?: number
+  appOnly?: boolean
+  hideRunnerUi?: boolean
+  clip?: Clip
+  userClip?: Clip
 }
 
 export const fs = Bluebird.promisifyAll(fsExtra) as PromisifiedFsExtra & typeof fsExtra
+
+const ensureSafePath = async function (withoutExt: string, extension: string | null, overwrite: boolean | undefined, num: number = 0): Promise<string> {
+  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<string>) => {
+  // 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: Data, ext: string | null, screenshotsFolder: ScreenshotsFolder, overwrite: boolean | undefined): Promise<string> {
+  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 && data.testAttemptIndex > 0) {
+    names[index] = `${names[index]} (attempt ${data.testAttemptIndex + 1})`
+  }
+
+  let withoutExt
+
+  if (screenshotsFolder) {
+    withoutExt = path.join(screenshotsFolder, ...specNames, ...names)
+  } else {
+    withoutExt = path.join(...specNames, ...names)
+  }
+
+  return await ensureSafePath(withoutExt, ext, overwrite)
+}