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

feat: allow users to pass true to videoCompression config and only a… #26810

Merged
merged 23 commits into from
Jun 8, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
11e723d
chore: allow users to pass true to videoCompression config and only a…
AtofStryker May 18, 2023
5f1e071
run ci
AtofStryker May 19, 2023
3b49bd3
Merge branch 'develop' of github.com:cypress-io/cypress into chore/up…
AtofStryker Jun 7, 2023
2826f16
chore: allow zero to be option for CRF as we will coerve it to false …
AtofStryker Jun 7, 2023
7805081
Update cli/CHANGELOG.md
AtofStryker Jun 7, 2023
0b9100c
chore: update videoCompression types
AtofStryker Jun 7, 2023
476b4d2
Merge branch 'chore/update_video_compression_defaults' of github.com:…
AtofStryker Jun 7, 2023
99a8cf2
chore: update changelog
AtofStryker Jun 7, 2023
d919367
Update cli/CHANGELOG.md
AtofStryker Jun 7, 2023
d6cc771
Update cli/types/cypress.d.ts
AtofStryker Jun 7, 2023
7341310
Update packages/config/src/validation.ts
AtofStryker Jun 7, 2023
6943671
chore: update config snapshots
AtofStryker Jun 7, 2023
fb38288
Update packages/config/test/validation.spec.ts
AtofStryker Jun 7, 2023
b653921
chore: add system test on videoCompression=true coersion
AtofStryker Jun 8, 2023
c0fe652
Merge branch 'chore/update_video_compression_defaults' of github.com:…
AtofStryker Jun 8, 2023
53a07c9
chore: document 0 as being false and not a valid CRF option for cypre…
AtofStryker Jun 8, 2023
554177a
Merge branch 'develop' into chore/update_video_compression_defaults
AtofStryker Jun 8, 2023
0292aa5
chore: fix types
AtofStryker Jun 8, 2023
09bad26
Merge branch 'chore/update_video_compression_defaults' of github.com:…
AtofStryker Jun 8, 2023
7b3bde9
chore: fix types
AtofStryker Jun 8, 2023
bd7e247
Merge branch 'develop' of github.com:cypress-io/cypress into chore/up…
AtofStryker Jun 8, 2023
2eb4885
chore: fix change to log as feature and not bugfix
AtofStryker Jun 8, 2023
e86dcb1
chore: fix server side unit tests
AtofStryker Jun 8, 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
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ _Released 06/07/2023_

- Fixed an issue to now correctly detect Angular 16 dependencies during Component Testing onboarding. Addresses [#26852](https://github.com/cypress-io/cypress/issues/26852)
- Ensures Git-related messages on the [Runs page](https://docs.cypress.io/guides/core-concepts/cypress-app#Runs) remain dismissed. Addresses [#26808](https://github.com/cypress-io/cypress/issues/26808)
- The `videoCompression` option now only accepts a boolean or a valid CRF options between 0-51 (inclusively). In addition, `true` is now a valid option, which will default to 32 CRF. Addresses [#26658](https://github.com/cypress-io/cypress/issues/26658).
Copy link
Member

@emilyrohrbough emilyrohrbough Jun 7, 2023

Choose a reason for hiding this comment

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

Suggested change
- The `videoCompression` option now only accepts a boolean or a valid CRF options between 0-51 (inclusively). In addition, `true` is now a valid option, which will default to 32 CRF. Addresses [#26658](https://github.com/cypress-io/cypress/issues/26658).
- The [`videoCompression`](https://docs.cypress.io/guides/references/configuration#Videos) configuration option now accepts both a boolean or a Constant Rate Factor (CRF) number between `0` and `51`. The `videoCompression` default value is still `32` CRF and when `videoCompression` is set to `true` the default of `32` CRF will be used. Addresses [#26658](https://github.com/cypress-io/cypress/issues/26658).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in 53a07c9


**Dependency Updates:**

Expand Down
260 changes: 136 additions & 124 deletions packages/config/__snapshots__/validation.spec.ts.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
exports['missing https protocol'] = {
"key": "clientCertificates[0].url",
"value": "http://url.com",
"type": "an https protocol"
'key': 'clientCertificates[0].url',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the husky hook did it I swear 😅

'value': 'http://url.com',
'type': 'an https protocol',
}

exports['invalid url'] = {
"key": "clientCertificates[0].url",
"value": "not *",
"type": "a valid URL"
'key': 'clientCertificates[0].url',
'value': 'not *',
'type': 'a valid URL',
}

exports['undefined browsers'] = `
Expand All @@ -19,208 +19,220 @@ Expected at least one browser
`

exports['browsers list with a string'] = {
"key": "name",
"value": "foo",
"type": "a non-empty string",
"list": "browsers"
'key': 'name',
'value': 'foo',
'type': 'a non-empty string',
'list': 'browsers',
}

exports['invalid retry value'] = {
"key": "mockConfigKey",
"value": "1",
"type": "a positive number or null or an object with keys \"openMode\" and \"runMode\" with values of numbers or nulls"
'key': 'mockConfigKey',
'value': '1',
'type': 'a positive number or null or an object with keys "openMode" and "runMode" with values of numbers or nulls',
}

exports['invalid retry object'] = {
"key": "mockConfigKey",
"value": {
"fakeMode": 1
'key': 'mockConfigKey',
'value': {
'fakeMode': 1,
},
"type": "a positive number or null or an object with keys \"openMode\" and \"runMode\" with values of numbers or nulls"
'type': 'a positive number or null or an object with keys "openMode" and "runMode" with values of numbers or nulls',
}

exports['not qualified url'] = {
"key": "mockConfigKey",
"value": "url.com",
"type": "a fully qualified URL (starting with `http://` or `https://`)"
'key': 'mockConfigKey',
'value': 'url.com',
'type': 'a fully qualified URL (starting with `http://` or `https://`)',
}

exports['empty string'] = {
"key": "mockConfigKey",
"value": "",
"type": "a fully qualified URL (starting with `http://` or `https://`)"
'key': 'mockConfigKey',
'value': '',
'type': 'a fully qualified URL (starting with `http://` or `https://`)',
}

exports['not string or array'] = {
"key": "mockConfigKey",
"value": null,
"type": "a string or an array of strings"
'key': 'mockConfigKey',
'value': null,
'type': 'a string or an array of strings',
}

exports['array of non-strings'] = {
"key": "mockConfigKey",
"value": [
'key': 'mockConfigKey',
'value': [
1,
2,
3
3,
],
"type": "a string or an array of strings"
'type': 'a string or an array of strings',
}

exports['not one of the strings error message'] = {
"key": "test",
"value": "nope",
"type": "one of these values: \"foo\", \"bar\""
'key': 'test',
'value': 'nope',
'type': 'one of these values: "foo", "bar"',
}

exports['number instead of string'] = {
"key": "test",
"value": 42,
"type": "one of these values: \"foo\", \"bar\""
'key': 'test',
'value': 42,
'type': 'one of these values: "foo", "bar"',
}

exports['null instead of string'] = {
"key": "test",
"value": null,
"type": "one of these values: \"foo\", \"bar\""
'key': 'test',
'value': null,
'type': 'one of these values: "foo", "bar"',
}

exports['not one of the numbers error message'] = {
"key": "test",
"value": 4,
"type": "one of these values: 1, 2, 3"
'key': 'test',
'value': 4,
'type': 'one of these values: 1, 2, 3',
}

exports['string instead of a number'] = {
"key": "test",
"value": "foo",
"type": "one of these values: 1, 2, 3"
'key': 'test',
'value': 'foo',
'type': 'one of these values: 1, 2, 3',
}

exports['null instead of a number'] = {
"key": "test",
"value": null,
"type": "one of these values: 1, 2, 3"
'key': 'test',
'value': null,
'type': 'one of these values: 1, 2, 3',
}

exports['config/src/validation .isValidClientCertificatesSet returns error message for certs not passed as an array array 1'] = {
"key": "mockConfigKey",
"value": "1",
"type": "a positive number or null or an object with keys \"openMode\" and \"runMode\" with values of numbers or nulls"
'key': 'mockConfigKey',
'value': '1',
'type': 'a positive number or null or an object with keys "openMode" and "runMode" with values of numbers or nulls',
}

exports['config/src/validation .isValidClientCertificatesSet returns error message for certs object without url 1'] = {
"key": "clientCertificates[0].url",
"type": "a URL matcher"
'key': 'clientCertificates[0].url',
'type': 'a URL matcher',
}

exports['config/src/validation .isValidBrowser passes valid browsers and forms error messages for invalid ones isValidBrowser 1'] = {
"name": "isValidBrowser",
"behavior": [
'name': 'isValidBrowser',
'behavior': [
{
"given": {
"name": "Chrome",
"displayName": "Chrome Browser",
"family": "chromium",
"path": "/path/to/chrome",
"version": "1.2.3",
"majorVersion": 1
'given': {
'name': 'Chrome',
'displayName': 'Chrome Browser',
'family': 'chromium',
'path': '/path/to/chrome',
'version': '1.2.3',
'majorVersion': 1,
},
"expect": true
'expect': true,
},
{
"given": {
"name": "FF",
"displayName": "Firefox",
"family": "firefox",
"path": "/path/to/firefox",
"version": "1.2.3",
"majorVersion": "1"
'given': {
'name': 'FF',
'displayName': 'Firefox',
'family': 'firefox',
'path': '/path/to/firefox',
'version': '1.2.3',
'majorVersion': '1',
},
"expect": true
'expect': true,
},
{
"given": {
"name": "Electron",
"displayName": "Electron",
"family": "chromium",
"path": "",
"version": "99.101.3",
"majorVersion": 99
'given': {
'name': 'Electron',
'displayName': 'Electron',
'family': 'chromium',
'path': '',
'version': '99.101.3',
'majorVersion': 99,
},
"expect": true
'expect': true,
},
{
"given": {
"name": "No display name",
"family": "chromium"
'given': {
'name': 'No display name',
'family': 'chromium',
},
"expect": {
"key": "displayName",
"value": {
"name": "No display name",
"family": "chromium"
'expect': {
'key': 'displayName',
'value': {
'name': 'No display name',
'family': 'chromium',
},
"type": "a non-empty string"
}
'type': 'a non-empty string',
},
},
{
"given": {
"name": "bad family",
"displayName": "Bad family browser",
"family": "unknown family"
'given': {
'name': 'bad family',
'displayName': 'Bad family browser',
'family': 'unknown family',
},
"expect": {
"key": "family",
"value": {
"name": "bad family",
"displayName": "Bad family browser",
"family": "unknown family"
'expect': {
'key': 'family',
'value': {
'name': 'bad family',
'displayName': 'Bad family browser',
'family': 'unknown family',
},
"type": "either chromium, firefox or webkit"
}
}
]
'type': 'either chromium, firefox or webkit',
},
},
],
}

exports['config/src/validation .isPlainObject returns error message when value is a not an object 1'] = {
"key": "mockConfigKey",
"value": 1,
"type": "a plain object"
'key': 'mockConfigKey',
'value': 1,
'type': 'a plain object',
}

exports['config/src/validation .isNumber returns error message when value is a not a number 1'] = {
"key": "mockConfigKey",
"value": "string",
"type": "a number"
'key': 'mockConfigKey',
'value': 'string',
'type': 'a number',
}

exports['config/src/validation .isNumberOrFalse returns error message when value is a not number or false 1'] = {
"key": "mockConfigKey",
"value": null,
"type": "a number or false"
'key': 'mockConfigKey',
'value': null,
'type': 'a number or false',
}

exports['config/src/validation .isBoolean returns error message when value is a not a string 1'] = {
"key": "mockConfigKey",
"value": 1,
"type": "a string"
'key': 'mockConfigKey',
'value': 1,
'type': 'a string',
}

exports['config/src/validation .isString returns error message when value is a not a string 1'] = {
"key": "mockConfigKey",
"value": 1,
"type": "a string"
'key': 'mockConfigKey',
'value': 1,
'type': 'a string',
}

exports['config/src/validation .isArray returns error message when value is a non-array 1'] = {
"key": "mockConfigKey",
"value": 1,
"type": "an array"
'key': 'mockConfigKey',
'value': 1,
'type': 'an array',
}

exports['config/src/validation .isStringOrFalse returns error message when value is neither string nor false 1'] = {
"key": "mockConfigKey",
"value": null,
"type": "a string or false"
'key': 'mockConfigKey',
'value': null,
'type': 'a string or false',
}

exports['invalid lower bound'] = {
'key': 'test',
'value': -1,
'type': 'a valid CRF number between 0 & 51 (inclusively) or a boolean',
}

exports['invalid upper bound'] = {
'key': 'test',
'value': 52,
'type': 'a valid CRF number between 0 & 51 (inclusively) or a boolean',
}
2 changes: 1 addition & 1 deletion packages/config/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ const driverConfigOptions: Array<DriverConfigOption> = [
}, {
name: 'videoCompression',
defaultValue: 32,
validation: validate.isNumberOrFalse,
validation: validate.isValidCrfOrBoolean,
}, {
name: 'videosFolder',
defaultValue: 'cypress/videos',
Expand Down
10 changes: 10 additions & 0 deletions packages/config/src/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,16 @@ export function isNumberOrFalse (key: string, value: any): ErrResult | true {
return errMsg(key, value, 'a number or false')
}

export function isValidCrfOrBoolean (key: string, value: any): ErrResult | true {
// a valid number that is between 1-51 including 1 or 51
// or a boolean. false disables compression and true sets compression to 32 CRF by default.
if (_.isBoolean(value) || (_.isNumber(value) && _.inRange(value, 0, 52))) {
return true
}

return errMsg(key, value, 'a valid CRF number between 0 & 51 (inclusively) or a boolean')
AtofStryker marked this conversation as resolved.
Show resolved Hide resolved
}

export function isStringOrFalse (key: string, value: any): ErrResult | true {
if (_.isString(value) || isFalse(value)) {
return true
Expand Down
Loading