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

Jump to bottom

chore: rename video processing events to capture/compress #26800

Merged
merged 2 commits into from
May 19, 2023
Merged

chore: rename video processing events to capture/compress #26800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec16dea
chore: change processing nomenclature to compressing when printing th…
AtofStryker May 18, 2023
50bddeb
chore: rename 'capturing of' to 'capturing'
AtofStryker May 19, 2023
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
1 change: 1 addition & 0 deletions cli/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ _Released 05/23/2023 (PENDING)_
**Misc:**

- Updated styling & content of Cypress Cloud slideshows when not logged in or no runs have been recorded. Addresses [#26181](https://github.com/cypress-io/cypress/issues/26181).
- Changed the nomenclature of 'processing' to 'compressing' when terminal video output is printed during a run. Addresses [#26657](https://github.com/cypress-io/cypress/issues/26657).

## 12.12.0

Expand Down
4 changes: 2 additions & 2 deletions ...-html__/VIDEO_POST_PROCESSING_FAILED.html → ...snapshot-html__/VIDEO_CAPTURE_FAILED.html
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

44 changes: 44 additions & 0 deletions packages/errors/__snapshot-html__/VIDEO_COMPRESSION_FAILED.html
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 10 additions & 2 deletions packages/errors/src/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,17 @@ export const AllCypressErrors = {

${fmt.stackTrace(arg1)}`
},
VIDEO_POST_PROCESSING_FAILED: (arg1: Error) => {
VIDEO_CAPTURE_FAILED: (arg1: Error) => {
return errTemplate`\
Warning: We failed processing this video.
Warning: We failed capturing this video.

This error will not affect or change the exit code.

${fmt.stackTrace(arg1)}`
},
VIDEO_COMPRESSION_FAILED: (arg1: Error) => {
return errTemplate`\
Warning: We failed compressing this video.

This error will not affect or change the exit code.

Expand Down
9 changes: 8 additions & 1 deletion packages/errors/test/unit/visualSnapshotErrors_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,14 @@ describe('visual error templates', () => {
default: [err],
}
},
VIDEO_POST_PROCESSING_FAILED: () => {
VIDEO_CAPTURE_FAILED: () => {
const err = makeErr()

return {
default: [err],
}
},
VIDEO_COMPRESSION_FAILED: () => {
const err = makeErr()

return {
Expand Down
3 changes: 2 additions & 1 deletion packages/graphql/schemas/schema.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1236,7 +1236,8 @@ enum ErrorTypeEnum {
UNEXPECTED_INTERNAL_ERROR
UNEXPECTED_MUTATION_ERROR
UNSUPPORTED_BROWSER_VERSION
VIDEO_POST_PROCESSING_FAILED
VIDEO_CAPTURE_FAILED
VIDEO_COMPRESSION_FAILED
VIDEO_RECORDING_FAILED
}

Expand Down
44 changes: 26 additions & 18 deletions packages/server/lib/modes/run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,20 +310,28 @@ async function startVideoRecording (options: { previous?: VideoRecording, projec
return videoRecording
}

const warnVideoRecordingFailed = (err) => {
// log that post processing was attempted
const warnVideoCaptureFailed = (err) => {
// log that capturing video was attempted
// but failed and don't let this change the run exit code
errors.warning('VIDEO_POST_PROCESSING_FAILED', err)
errors.warning('VIDEO_CAPTURE_FAILED', err)
}

async function postProcessRecording (options: { quiet: boolean, videoCompression: number | boolean, shouldUploadVideo: boolean, processOptions: Omit<ProcessOptions, 'videoCompression'> }) {
const warnVideoCompressionFailed = (err) => {
// log that compression was attempted
// but failed and don't let this change the run exit code
errors.warning('VIDEO_COMPRESSION_FAILED', err)
}

async function compressRecording (options: { quiet: boolean, videoCompression: number | boolean, shouldUploadVideo: boolean, processOptions: Omit<ProcessOptions, 'videoCompression'> }) {
debug('ending the video recording %o', options)

// once this ended promises resolves
// then begin processing the file
// don't process anything if videoCompress is off
// then begin compressing the file
// don't compress anything if videoCompress is off
// or we've been told not to upload the video
if (options.videoCompression === false || options.videoCompression === 0 || options.shouldUploadVideo === false) {
debug('skipping compression')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a debug log if this condition is met since otherwise its hard to tell. I wonder if we want to add any of these options to the telemetry span in the future to determine if this is a no-op (we can do it in this PR as well just wanted to see if others thought it was a good idea).

Copy link
Member

@mjhenkes mjhenkes May 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

videoname, videoCompression and compressedVideoName are added to the span now. You could add 'should upload' if you want. (down on line 685)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I missed that. I think that should be enough for us to infer the behavior!


return
}

Expand All @@ -332,17 +340,17 @@ async function postProcessRecording (options: { quiet: boolean, videoCompression
videoCompression: Number(options.videoCompression),
}

function continueProcessing (onProgress?: (progress: number) => void) {
return videoCapture.process({ ...processOptions, onProgress })
function continueWithCompression (onProgress?: (progress: number) => void) {
return videoCapture.compress({ ...processOptions, onProgress })
}

if (options.quiet) {
return continueProcessing()
return continueWithCompression()
}

const { onProgress } = printResults.displayVideoProcessingProgress(processOptions)
const { onProgress } = printResults.displayVideoCompressionProgress(processOptions)

return continueProcessing(onProgress)
return continueWithCompression(onProgress)
}

function launchBrowser (options: { browser: Browser, spec: SpecWithRelativeRoot, setScreenshotMetadata: SetScreenshotMetadata, screenshots: ScreenshotMetadata[], projectRoot: string, shouldLaunchNewTab: boolean, onError: (err: Error) => void, videoRecording?: VideoRecording }) {
Expand Down Expand Up @@ -610,7 +618,7 @@ async function waitForTestsToFinishRunning (options: { project: Project, screens
debug('ended video capture')
} catch (err) {
videoCaptureFailed = true
warnVideoRecordingFailed(err)
warnVideoCaptureFailed(err)
}

telemetry.getSpan('video:capture')?.setAttributes({ videoCaptureFailed })?.end()
Expand All @@ -628,7 +636,7 @@ async function waitForTestsToFinishRunning (options: { project: Project, screens
if (!videoExists) {
// the video file no longer exists at the path where we expect it,
// possibly because the user deleted it in the after:spec event
debug(`No video found after spec ran - skipping processing. Video path: ${videoName}`)
debug(`No video found after spec ran - skipping compression. Video path: ${videoName}`)

results.video = null
}
Expand All @@ -641,7 +649,7 @@ async function waitForTestsToFinishRunning (options: { project: Project, screens
results.shouldUploadVideo = shouldUploadVideo

if (!shouldUploadVideo) {
debug(`Spec run had no failures and config.videoUploadOnPasses=false. Skip processing video. Video path: ${videoName}`)
debug(`Spec run had no failures and config.videoUploadOnPasses=false. Skip compressing video. Video path: ${videoName}`)
results.video = null
}

Expand All @@ -668,19 +676,19 @@ async function waitForTestsToFinishRunning (options: { project: Project, screens
}

if (videoExists && !skippedSpec && !videoCaptureFailed) {
const span = telemetry.startSpan({ name: 'video:post:processing' })
const span = telemetry.startSpan({ name: 'video:compression' })
const chaptersConfig = videoCapture.generateFfmpegChaptersConfig(results.tests)

try {
debug('post processing recording')
debug('compressing recording')

span?.setAttributes({
videoName,
videoCompressionString: videoCompression.toString(),
compressedVideoName: videoRecording.api.compressedVideoName,
})

await postProcessRecording({
await compressRecording({
shouldUploadVideo,
quiet,
videoCompression,
Expand All @@ -693,7 +701,7 @@ async function waitForTestsToFinishRunning (options: { project: Project, screens
})
} catch (err) {
videoCaptureFailed = true
warnVideoRecordingFailed(err)
warnVideoCompressionFailed(err)
}
span?.end()
}
Expand Down
8 changes: 4 additions & 4 deletions packages/server/lib/util/print-run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ function displayScreenshots (screenshots: Screenshot[] = []) {
console.log('')
}

export function displayVideoProcessingProgress (opts: { videoName: string, videoCompression: number | false }) {
export function displayVideoCompressionProgress (opts: { videoName: string, videoCompression: number | false }) {
console.log('')

terminal.header('Video', {
Expand All @@ -498,7 +498,7 @@ export function displayVideoProcessingProgress (opts: { videoName: string, video

table.push([
gray('-'),
gray('Started processing:'),
gray('Started compressing:'),
chalk.cyan(`Compressing to ${opts.videoCompression} CRF`),
])

Expand All @@ -515,7 +515,7 @@ export function displayVideoProcessingProgress (opts: { videoName: string, video
const dur = `${humanTime.long(finished)}`

const table = terminal.table({
colWidths: [3, 21, 61, 15],
colWidths: [3, 22, 60, 15],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

had to offset the 2nd column by one in order to not get finished compressin... and not get the g truncated. I took a character from the 3rd column as we have more than ample room to store the results of the compression, but means the snapshots will be missing a space in that column (hence the regen in the system tests)

colAligns: ['left', 'left', 'left', 'right'],
type: 'noBorder',
style: {
Expand All @@ -529,7 +529,7 @@ export function displayVideoProcessingProgress (opts: { videoName: string, video

table.push([
gray('-'),
gray('Finished processing:'),
gray('Finished compressing:'),
gray(dur),
])

Expand Down
6 changes: 3 additions & 3 deletions packages/server/lib/video_capture.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export type StartOptions = {
videoName: string
// If set, expect input frames as webm chunks.
webmInput?: boolean
// Callback for asynchronous errors in video processing/compression.
// Callback for asynchronous errors in video capturing/compression.
onError?: (err: Error, stdout: string, stderr: string) => void
}

Expand Down Expand Up @@ -279,14 +279,14 @@ export function start (options: StartOptions) {
})
}

export async function process (options: ProcessOptions) {
export async function compress (options: ProcessOptions) {
let total = null

const metaFileName = `${options.videoName}.meta`
const addChaptersMeta = options.chaptersConfig && await fs.writeFile(metaFileName, options.chaptersConfig).then(() => true)

return new Promise<void>((resolve, reject) => {
debug('processing video %o', options)
debug('compressing video %o', options)

const command = ffmpeg({
priority: 20,
Expand Down
4 changes: 2 additions & 2 deletions packages/server/test/support/fixtures/server/expected_stdout_bundle_failures.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ Fix the error in your code and re-run your tests.

(Video)

- Started processing: Compressing to 32 CRF
- Finished processing: /foo/bar/.projects/e2e/cypress/videos/abc123.mp4 (X seconds)
- Started compressing: Compressing to 32 CRF
- Finished compressing: /foo/bar/.projects/e2e/cypress/videos/abc123.mp4 (X seconds)


(Run Finished)
4 changes: 2 additions & 2 deletions packages/server/test/support/fixtures/server/expected_stdout_failures_outro.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

(Video)

- Started processing: Compressing to 32 CRF
- Finished processing: /foo/bar/.projects/e2e/cypress/videos/abc123.mp4 (X seconds)
- Started compressing: Compressing to 32 CRF
- Finished compressing: /foo/bar/.projects/e2e/cypress/videos/abc123.mp4 (X seconds)


(Run Finished)
4 changes: 2 additions & 2 deletions packages/server/test/support/fixtures/server/expected_stdout_passing.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@

(Video)

- Started processing: Compressing to 32 CRF
- Finished processing: /foo/bar/.projects/e2e/cypress/videos/abc123.mp4 (X seconds)
- Started compressing: Compressing to 32 CRF
- Finished compressing: /foo/bar/.projects/e2e/cypress/videos/abc123.mp4 (X seconds)


(Run Finished)
4 changes: 2 additions & 2 deletions system-tests/__snapshots__/browser_path_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ exports['e2e launching browsers by path works with an installed browser path 1']

(Video)

- Started processing: Compressing to 32 CRF
- Finished processing: /XXX/XXX/XXX/cypress/videos/simple.cy.js.mp4 (X second)
- Started compressing: Compressing to 32 CRF
- Finished compressing: /XXX/XXX/XXX/cypress/videos/simple.cy.js.mp4 (X second)
Comment on lines +42 to +43
Copy link
Member

Choose a reason for hiding this comment

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

you have no idea how lucky you are to make this change after i turned off video compression in snapshots. lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's just say there were a lot LESS snapshots that needed updated 🙏🏻



====================================================================================================
Expand Down
4 changes: 2 additions & 2 deletions system-tests/__snapshots__/plugins_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ exports['e2e plugins can modify config from plugins 1'] = `

(Video)

- Started processing: Compressing to 20 CRF
- Finished processing: X second(s)
- Started compressing: Compressing to 20 CRF
- Finished compressing: X second(s)

- Video output: /XXX/XXX/XXX/cypress/videos/app.cy.js.mp4

Expand Down
16 changes: 8 additions & 8 deletions system-tests/__snapshots__/record_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ Fix the error in your code and re-run your tests.

(Video)

- Started processing: Compressing to 32 CRF
- Finished processing: X second(s)
- Started compressing: Compressing to 32 CRF
- Finished compressing: X second(s)

- Video output: /XXX/XXX/XXX/cypress/videos/record_error.cy.js.mp4

Expand Down Expand Up @@ -124,8 +124,8 @@ Because this error occurred during a \`before each\` hook we are skipping the re

(Video)

- Started processing: Compressing to 32 CRF
- Finished processing: X second(s)
- Started compressing: Compressing to 32 CRF
- Finished compressing: X second(s)

- Video output: /XXX/XXX/XXX/cypress/videos/record_fail.cy.js.mp4

Expand Down Expand Up @@ -226,8 +226,8 @@ We dynamically generated a new test to display this failure.

(Video)

- Started processing: Compressing to 32 CRF
- Finished processing: X second(s)
- Started compressing: Compressing to 32 CRF
- Finished compressing: X second(s)

- Video output: /XXX/XXX/XXX/cypress/videos/record_uncaught.cy.js.mp4

Expand Down Expand Up @@ -2357,8 +2357,8 @@ Because this error occurred during a \`before each\` hook we are skipping the re

(Video)

- Started processing: Compressing to 32 CRF
- Finished processing: X second(s)
- Started compressing: Compressing to 32 CRF
- Finished compressing: X second(s)

- Video output: /XXX/XXX/XXX/cypress/videos/record_fail.cy.js.mp4

Expand Down
Loading