breaking: default video configuration option to false #27008
Conversation
AtofStryker
force-pushed
the
feat/set_video_false
branch
2 times, most recently
from
cdeee0e to
93584bc
Compare
AtofStryker
changed the title
AtofStryker
changed the title
AtofStryker
force-pushed
the
feat/set_video_false
branch
from
93584bc to
ead8ed1
Compare
cli/types/cypress.d.ts
Outdated
| @@ -3013,7 +3013,7 @@ declare namespace Cypress { | |||
| videoCompression: number | boolean | |||
| /** | |||
| * Whether Cypress will record a video of the test run when running headlessly. | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
hows 4ef0364 look?
| @@ -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 | |||
There was a problem hiding this comment.
why did we remove results.video = null?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
ultimately it doesn't matter too much, since this gets ripped out in #27010
|
@AtofStryker On the issue we called out:
What was the execution time and resource usage improvement in our CI runs by disabling videos by default? Would we expect to be able to lower our CI machine resources with this change? |
Co-authored-by: Emily Rohrbough <[email protected]>
I didn't measure it, but I know @mjhenkes is doing measurements on the v13 branch while mixing/matching video configurations. I am not sure if we can lower CI resources since protocol might make up for the performance difference, which seems to be somewhat what @mjhenkes was saying, assuming I am articulating that correctly. |
|
Currently we're running protocol and video on, with this change i'd expect our resource consumption with just protocol to drop back to video + compression levels. So we will reduce somewhat in this branch, but not overall. |
3 flaky tests on run #47901 ↗︎
Details:
|
|||||||||||||||||||||||||||
| Test | Artifacts | |
|---|---|---|
| ... > correctly returns currentRetry |
Output
|
|
| ... > correctly returns currentRetry |
Output
|
|
| ... > correctly returns currentRetry |
Output
|
|
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.
AtofStryker
force-pushed
the
feat/set_video_false
branch
from
052bebb to
24ee9f5
Compare
AtofStryker
force-pushed
the
feat/set_video_false
branch
from
24ee9f5 to
129a152
Compare
AtofStryker
force-pushed
the
feat/set_video_false
branch
from
129a152 to
a66b1fe
Compare
…as the CI tests run faster without video on
…eat/set_video_false
…almsot always, but sometimes throws an error
|
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
For Cypress
v13, video will be set tofalseby default. This PR implements those changes.Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation? BREAKING: update documentation by setting default video: false for v13.0.0 cypress-documentation#5300type definitions?