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

breaking: default video configuration option to false #27008

Merged
merged 16 commits into from
Jun 20, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 0 additions & 1 deletion .circleci/workflows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2069,7 +2069,6 @@ jobs:
CYPRESS_PROJECT_ID=$TEST_KITCHENSINK_PROJECT_ID \
CYPRESS_RECORD_KEY=$TEST_KITCHENSINK_RECORD_KEY \
CYPRESS_INTERNAL_ENV=staging \
CYPRESS_video=false \
yarn cypress:run --project /tmp/cypress-example-kitchensink --record
- store-npm-logs

Expand Down
3 changes: 2 additions & 1 deletion cli/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
<!-- See the ../guides/writing-the-cypress-changelog.md for details on writing the changelog. -->
## 13.0.0

_Released 03/1/2023 (PENDING)_
_Released 8/1/2023 (PENDING)_

**Breaking Changes:**

- The [`cy.readFile()`](/api/commands/readfile) command is now retry-able as a [query command](https://on.cypress.io/retry-ability). This should not affect any tests using it; the functionality is unchanged. However, it can no longer be overwritten using [`Cypress.Commands.overwrite()`](/api/cypress-api/custom-commands#Overwrite-Existing-Commands). Addressed in [#25595](https://github.com/cypress-io/cypress/pull/25595).
- the [video](https://docs.cypress.io/guides/references/configuration#Videos) configuration option will now default to `false`. Addresses [#26157](https://github.com/cypress-io/cypress/issues/26157).

## 12.14.1

Expand Down
2 changes: 1 addition & 1 deletion cli/types/cypress.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3013,7 +3013,7 @@ declare namespace Cypress {
videoCompression: number | boolean
/**
* Whether Cypress will record a video of the test run when running headlessly.
Copy link
Member

Choose a reason for hiding this comment

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

Should this be calling out cypress run mode? do we really only recorded when headless or just run mode? haven't seen this blurb in our docs anywhere if it is only headless mode.

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 think you're right. recording happens in run mode regardless of whether the browser is headed or not. I'll push an update to call out run mode specifically

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hows 4ef0364 look?

* @default true
* @default false
*/
video: boolean
/**
Expand Down
1 change: 0 additions & 1 deletion npm/grep/cypress.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,4 @@ module.exports = defineConfig({
specPattern: '**/spec.js',
},
fixturesFolder: false,
video: false,
})
1 change: 0 additions & 1 deletion npm/react/cypress.config.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
module.exports = {
'viewportWidth': 400,
'viewportHeight': 400,
'video': false,
'projectId': 'z9dxah',
'env': {
'reactDevtools': true,
Expand Down
1 change: 0 additions & 1 deletion npm/vue/cypress.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { defineConfig } from 'cypress'
export default defineConfig({
'viewportWidth': 500,
'viewportHeight': 500,
'video': false,
'responseTimeout': 2500,
'projectId': '134ej7',
'experimentalFetchPolyfill': true,
Expand Down
3 changes: 0 additions & 3 deletions npm/webpack-preprocessor/test/e2e/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,6 @@ exports.runTest = async (options = {}) => {
spec: opts.spec,
browser: opts.browser,
exit: opts.exit,
config: {
video: false,
},
dev: true,
})
.finally(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@
"field": "userAgent"
},
{
"value": true,
"value": false,
"from": "default",
"field": "video"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
"field": "userAgent"
},
{
"value": true,
"value": false,
"from": "default",
"field": "video"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,7 @@
"field": "userAgent"
},
{
"value": true,
"value": false,
"from": "default",
"field": "video"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@
"field": "userAgent"
},
{
"value": true,
"value": false,
"from": "default",
"field": "video"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
"field": "userAgent"
},
{
"value": true,
"value": false,
"from": "default",
"field": "video"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1656,7 +1656,7 @@
"field": "userAgent"
},
{
"value": true,
"value": false,
"from": "default",
"field": "video"
},
Expand Down
4 changes: 2 additions & 2 deletions packages/config/__snapshots__/index.spec.ts.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ exports['config/src/index .getDefaultValues returns list of public config keys 1
'testIsolation': true,
'trashAssetsBeforeRuns': true,
'userAgent': null,
'video': true,
'video': false,
'videoCompression': 32,
'videosFolder': 'cypress/videos',
'videoUploadOnPasses': true,
Expand Down Expand Up @@ -163,7 +163,7 @@ exports['config/src/index .getDefaultValues returns list of public config keys f
'testIsolation': true,
'trashAssetsBeforeRuns': true,
'userAgent': null,
'video': true,
'video': false,
'videoCompression': 32,
'videosFolder': 'cypress/videos',
'videoUploadOnPasses': true,
Expand Down
2 changes: 1 addition & 1 deletion packages/config/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ const driverConfigOptions: Array<DriverConfigOption> = [
requireRestartOnChange: 'browser',
}, {
name: 'video',
defaultValue: true,
defaultValue: false,
validation: validate.isBoolean,
}, {
name: 'videoCompression',
Expand Down
8 changes: 4 additions & 4 deletions packages/config/test/project/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -779,8 +779,8 @@ describe('config/src/project/utils', () => {
return this.defaults('animationDistanceThreshold', 5)
})

it('video=true', function () {
return this.defaults('video', true)
it('video=false', function () {
return this.defaults('video', false)
})

it('videoCompression=32', function () {
Expand Down Expand Up @@ -1094,7 +1094,7 @@ describe('config/src/project/utils', () => {
testIsolation: { value: true, from: 'default' },
trashAssetsBeforeRuns: { value: true, from: 'default' },
userAgent: { value: null, from: 'default' },
video: { value: true, from: 'default' },
video: { value: false, from: 'default' },
videoCompression: { value: 32, from: 'default' },
videosFolder: { value: 'cypress/videos', from: 'default' },
videoUploadOnPasses: { value: true, from: 'default' },
Expand Down Expand Up @@ -1213,7 +1213,7 @@ describe('config/src/project/utils', () => {
testIsolation: { value: true, from: 'default' },
trashAssetsBeforeRuns: { value: true, from: 'default' },
userAgent: { value: null, from: 'default' },
video: { value: true, from: 'default' },
video: { value: false, from: 'default' },
videoCompression: { value: 32, from: 'default' },
videosFolder: { value: 'cypress/videos', from: 'default' },
videoUploadOnPasses: { value: true, from: 'default' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ const resolvedOptions: Array<ResolvedConfigOption> = [
canUpdateDuringTestTime: false,
}, {
name: 'video',
defaultValue: true,
defaultValue: false,
canUpdateDuringTestTime: false,
}, {
name: 'videoCompression',
Expand Down
2 changes: 1 addition & 1 deletion packages/frontend-shared/cypress/fixtures/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@
"field": "userAgent"
},
{
"value": true,
"value": false,
"from": "default",
"field": "video"
},
Expand Down
5 changes: 3 additions & 2 deletions packages/server/lib/modes/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ async function waitForTestsToFinishRunning (options: { project: Project, screens

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

Choose a reason for hiding this comment

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

why did we remove results.video = null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because video is still be captured, it just isn't uploaded to the cloud or compressed, meaning that we still want to print video: true in the terminal but want to return null when the function ends, which is handled here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ultimately it doesn't matter too much, since this gets ripped out in #27010

results.videoCompression = false
}

if (!quiet && !skippedSpec) {
Expand Down Expand Up @@ -723,7 +723,8 @@ async function waitForTestsToFinishRunning (options: { project: Project, screens
printResults.printVideoPath(videoName)
}

if (videoCaptureFailed || videoCompressionFailed) {
// if capture or compressing has failed, or the test passed and videoUploadOnPasses=false
if (videoCaptureFailed || videoCompressionFailed || !shouldUploadVideo) {
results.video = null
}

Expand Down
2 changes: 1 addition & 1 deletion packages/server/test/integration/cypress_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1042,7 +1042,7 @@ describe('lib/cypress', () => {

expect(chromeBrowser._navigateUsingCRI).to.have.been.calledOnce
expect(chromeBrowser._setAutomation).to.have.been.calledOnce
expect(chromeBrowser._recordVideo).to.have.been.calledOnce
expect(chromeBrowser._recordVideo).not.to.have.been.called

expect(BrowserCriClient.create).to.have.been.calledOnce
expect(browserCriClient.attachToTargetUrl).to.have.been.calledOnce
Expand Down
7 changes: 1 addition & 6 deletions system-tests/__snapshots__/async_timeouts_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ exports['e2e async timeouts / failing1'] = `
│ Pending: 0 │
│ Skipped: 0 │
│ Screenshots: 2 │
│ Video: true
│ Video: false
│ Duration: X seconds │
│ Spec Ran: async_timeouts.cy.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
Expand All @@ -61,11 +61,6 @@ exports['e2e async timeouts / failing1'] = `
cypress command (failed).png


(Video)

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


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

(Run Finished)
Expand Down
14 changes: 2 additions & 12 deletions system-tests/__snapshots__/base_url_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,12 @@ exports['e2e baseUrl / https / passes'] = `
│ Pending: 0 │
│ Skipped: 0 │
│ Screenshots: 0 │
│ Video: true
│ Video: false
│ Duration: X seconds │
│ Spec Ran: base_url.cy.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Video)

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


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

(Run Finished)
Expand Down Expand Up @@ -93,17 +88,12 @@ exports['e2e baseUrl / http / passes'] = `
│ Pending: 0 │
│ Skipped: 0 │
│ Screenshots: 0 │
│ Video: true
│ Video: false
│ Duration: X seconds │
│ Spec Ran: base_url.cy.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Video)

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


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

(Run Finished)
Expand Down
Loading