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.

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

fix: Treat Video compression 0 as false. #26503

Merged
merged 18 commits into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 0 additions & 2 deletions .circleci/workflows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -605,8 +605,6 @@ commands:
fi
- store_test_results:
path: /tmp/cypress
- store_artifacts:
path: ./packages/<<parameters.package>>/cypress/videos
- store-npm-logs

run-system-tests:
Expand Down
4 changes: 4 additions & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

_Released 04/25/2023 (PENDING)_

**Bugfixes:**

- Fixed an issue where setting `videoCompression` to `0` would cause the video output to be broken. `0` is now treated as false. Addresses [#5191](https://github.com/cypress-io/cypress/issues/5191) and [#24595](https://github.com/cypress-io/cypress/issues/24595).

## 12.10.0

_Released 04/17/2023_
Expand Down
1 change: 1 addition & 0 deletions packages/driver/cypress.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export default defineConfig({
reporterOptions: {
configFile: '../../mocha-reporter-config.json',
},
videoCompression: false, // turn off video compression for CI
e2e: {
experimentalOriginDependencies: true,
experimentalModifyObstructiveThirdPartyCode: true,
Expand Down
2 changes: 1 addition & 1 deletion packages/frontend-shared/cypress.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ export default defineConfig({
reporter: '../../node_modules/cypress-multi-reporters/index.js',
reporterOptions: {
configFile: '../../mocha-reporter-config.json',
videoCompression: false, // turn off video compression for CI
Copy link
Contributor

Choose a reason for hiding this comment

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

Surprised this wasn't a typescript error.

Copy link
Member Author

Choose a reason for hiding this comment

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

same

},
videoCompression: false, // turn off video compression for CI
component: {
experimentalSingleTabRunMode: true,
devServer: {
Expand Down
2 changes: 1 addition & 1 deletion packages/launchpad/cypress.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ export default defineConfig({
reporter: '../../node_modules/cypress-multi-reporters/index.js',
reporterOptions: {
configFile: '../../mocha-reporter-config.json',
videoCompression: false, // turn off video compression for CI
},
videoCompression: false, // turn off video compression for CI
component: {
experimentalSingleTabRunMode: true,
supportFile: 'cypress/component/support/index.ts',
Expand Down
2 changes: 2 additions & 0 deletions packages/reporter/cypress.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ export default defineConfig({
openMode: 0,
},

videoCompression: false, // turn off video compression for CI

e2e: {
experimentalStudio: true,
baseUrl: 'http://localhost:5006',
Expand Down
5 changes: 4 additions & 1 deletion packages/server/lib/cypress.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ const exit = async (code = 0) => {
})
}

telemetry.getSpan('cypress')?.end()
const span = telemetry.getSpan('cypress')

span?.setAttribute('exitCode', code)
Copy link
Member Author

Choose a reason for hiding this comment

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

lets log out the exit code to know if jobs failed or not.

span?.end()

await telemetry.shutdown().catch((err) => {
debug('telemetry shutdown errored with: ', err)
Expand Down
8 changes: 4 additions & 4 deletions packages/server/lib/modes/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ async function startVideoRecording (options: { previous?: VideoRecording, projec

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

Expand All @@ -321,9 +321,9 @@ async function postProcessRecording (options: { quiet: boolean, videoCompression

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

Choose a reason for hiding this comment

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

This is the bug fix.

Copy link
Member

Choose a reason for hiding this comment

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

if (!options.videoCompress || !options.shouldUploadVideo) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

i'd rather be explicit

return
}

Expand Down Expand Up @@ -676,7 +676,7 @@ async function waitForTestsToFinishRunning (options: { project: Project, screens

span?.setAttributes({
videoName,
videoCompression,
videoCompressionString: videoCompression.toString(),
Copy link
Member Author

Choose a reason for hiding this comment

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

honeycomb decided this value was an int, so false === 0... i changed it to a string to get the real value.

compressedVideoName: videoRecording.api.compressedVideoName,
})

Expand Down
8 changes: 0 additions & 8 deletions system-tests/__snapshots__/async_timeouts_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,6 @@ exports['e2e async timeouts / failing1'] = `
cypress command (failed).png


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down
16 changes: 0 additions & 16 deletions system-tests/__snapshots__/base_url_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,6 @@ exports['e2e baseUrl / https / passes'] = `
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down Expand Up @@ -102,14 +94,6 @@ exports['e2e baseUrl / http / passes'] = `
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down
48 changes: 0 additions & 48 deletions system-tests/__snapshots__/browser_crash_handling_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,6 @@ https://on.cypress.io/renderer-process-crashed
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Video)

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

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


────────────────────────────────────────────────────────────────────────────────────────────────────

Running: simple.cy.js (2 of 2)
Expand All @@ -87,14 +79,6 @@ https://on.cypress.io/renderer-process-crashed
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down Expand Up @@ -167,14 +151,6 @@ https://on.cypress.io/renderer-process-crashed
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Video)

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

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


────────────────────────────────────────────────────────────────────────────────────────────────────

Running: simple.cy.js (2 of 2)
Expand All @@ -200,14 +176,6 @@ https://on.cypress.io/renderer-process-crashed
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down Expand Up @@ -271,14 +239,6 @@ This can happen for many different reasons:
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Video)

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

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


────────────────────────────────────────────────────────────────────────────────────────────────────

Running: simple.cy.js (2 of 2)
Expand All @@ -304,14 +264,6 @@ This can happen for many different reasons:
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down
8 changes: 0 additions & 8 deletions system-tests/__snapshots__/busted_support_file_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,6 @@ Fix the error in your code and re-run your tests.
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down
8 changes: 0 additions & 8 deletions system-tests/__snapshots__/cache_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,6 @@ exports['e2e cache passes 1'] = `
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down
32 changes: 0 additions & 32 deletions system-tests/__snapshots__/caught_uncaught_hook_errors_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,6 @@ Because this error occurred during a \`before all\` hook we are skipping the rem
before all hook (failed).png


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down Expand Up @@ -181,14 +173,6 @@ Because this error occurred during a \`before each\` hook we are skipping the re
before each hook (failed).png


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down Expand Up @@ -265,14 +249,6 @@ Because this error occurred during a \`before each\` hook we are skipping all of
efore each hook (failed).png


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down Expand Up @@ -359,14 +335,6 @@ Because this error occurred during a \`before each\` hook we are skipping the re
before each hook (failed).png


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down
24 changes: 0 additions & 24 deletions system-tests/__snapshots__/commands_outside_of_test_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,6 @@ exports['e2e commands outside of test / passes on passing assertions'] = `
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down Expand Up @@ -118,14 +110,6 @@ We dynamically generated a new test to display this failure.
aught error was detected outside of a test (failed).png


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down Expand Up @@ -209,14 +193,6 @@ https://on.cypress.io/cannot-execute-commands-outside-test
r was detected outside of a test (failed).png


(Video)

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

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


====================================================================================================

(Run Finished)
Expand Down
Loading