fix: prevent overwriting of video files

cli/CHANGELOG.md

@@ -6,6 +6,7 @@ _Released 4/22/2025 (PENDING)_
 **Bugfixes:**
 
 - Fixed an issue where auto scroll in the Cypress Command Log was not scrolling correctly. Fixes [#31530](https://github.com/cypress-io/cypress/issues/31530).
+- 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).
 
 ## 14.3.1
 

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'
@@ -23,6 +23,7 @@ import chromePolicyCheck from '../util/chrome_policy_check'
 import type { SpecWithRelativeRoot, SpecFile, TestingType, OpenProjectLaunchOpts, FoundBrowser, BrowserVideoController, VideoRecording, ProcessOptions, ProtocolManagerShape, AutomationCommands } from '@packages/types'
 import type { Cfg, ProjectBase } from '../project-base'
 import type { Browser } from '../browsers/types'
+import type { Data } from '../util/fs'
 import * as printResults from '../util/print-run'
 import { telemetry } from '@packages/telemetry'
 import { CypressRunResult, createPublicBrowser, createPublicConfig, createPublicRunResults, createPublicSpec, createPublicSpecResults } from './results'
@@ -224,15 +225,31 @@ 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,
+      startTime: new Date(), // needed for ts-lint
+      viewport: {
+        width: 0,
+        height: 0,
+      },
+      specName: '', // this is optional, the getPath will pick up from specPath
+      testFailure: false, //  this is only applicable for screenshot, not for video
+      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 +350,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
   }
 
@@ -949,7 +973,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
@@ -61,7 +27,10 @@ interface Details {
 interface SavedDetails {
   size?: string
   takenAt?: Date
-  dimensions?: string
+  dimensions?: {
+    width: number
+    height: number
+  }
   multipart?: any
   pixelRatio?: number
   name?: any
@@ -70,14 +39,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 +322,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))