fix: Treat Video compression 0 as false. #26503

mjhenkes · 2023-04-14T14:19:14Z

Closes When passing "videoCompression":0, the output video does not work #5191
Closes Videos not viewable on Cypress Dashboard when videoCompression is set to 0 #24595

Additional details

This Pull request also disables video compression for our tests in CI

The following table is a comparison between a develop ci run and a ci run in this branch, since it is not averaged over time the number just represent a snapshot in time and the real performance difference is probably somewhere in the middle.

CI Job	Before	After	Difference
driver-integration-tests-chrome	10:26	9:32	-0:54
driver-integration-tests-chrome-beta	10:25	9:10	-1:15
driver-integration-tests-electron	11:14	9:57	-1:17
driver-integration-tests-firefox	8:48	10	+1:12
driver-integration-tests-webkit	12:41	11:08	-1:33
run-app-component-tests-chrome	4:06	4:02	-0:04
run-app-integration-tests-chrome	9:22	9:22	+/-0
run-frontend-shared-component-tests-chrome	5:01	3:37	-1:24
run-launchpad-component-tests-chrome	2:20	2:34	-0:14
run-launchpad-integration-tests-chrome	8	7:41	-0:19
run-reporter-component-tests-chrome	1:55	2:07	+0:12
system-tests-chrome	15:25	13:37	-1:48
system-tests-electron	16:25	16:09	-0:14
system-tests-firefox	11:28	12:05	+0:37
system-tests-webkit	8:22	8: 09	-0:13

Honeycomb metrics:
Comparing average job duration between develop and this branch
Average time spent in the video:post:procoessing span

Steps to test

Set video compression to zero, watch the video.

How has the user experience changed?

PR Tasks

[ x ] Have tests been added/updated?
Has a PR for user-facing changes been opened in cypress-documentation?
Have API changes been updated in the type definitions?

mjhenkes · 2023-04-14T14:20:50Z

This still needs a Changlog entry, but i'll add that after the next release.

mjhenkes · 2023-04-14T14:21:56Z

.circleci/workflows.yml

@@ -493,7 +493,7 @@ commands:
              # internal PR
              CYPRESS_RECORD_KEY=$MAIN_RECORD_KEY \
              CYPRESS_INTERNAL_ENABLE_TELEMETRY="true" \
-              OTEL_RESOURCE_ATTRIBUTES="ci.branch=$CIRCLE_BRANCH,ci.job=$CIRCLE_JOB,ci.node-index=$CIRCLE_NODE_INDEX,ci.circle=$CIRCLECI,ci.build-url=$CIRCLE_BUILD_URL,ci.build-number=$CIRCLE_BUILD_NUM" \
+              OTEL_RESOURCE_ATTRIBUTES="ci.branch=$CIRCLE_BRANCH,ci.job=$CIRCLE_JOB,ci.node-index=$CIRCLE_NODE_INDEX,ci.circle=$CIRCLECI,ci.build-url=$CIRCLE_BUILD_URL,ci.build-number=$CIRCLE_BUILD_NUM,SHA1=$CIRCLE_SHA1,PR_NUMBER=$CIRCLE_PR_NUMBER" \


adding to telemetry, i want to turn this into a custom detector, but thats for another pr

mjhenkes · 2023-04-14T14:22:20Z

packages/server/lib/cypress.js

-  telemetry.getSpan('cypress')?.end()
+  const span = telemetry.getSpan('cypress')
+
+  span?.setAttribute('exitCode', code)


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

mjhenkes · 2023-04-14T14:22:34Z

packages/server/lib/modes/run.ts

  // 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) {


This is the bug fix.

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

i'd rather be explicit

mjhenkes · 2023-04-14T14:23:19Z

packages/server/lib/modes/run.ts

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

      span?.setAttributes({
        videoName,
-        videoCompression,
+        videoCompressionString: videoCompression.toString(),


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

mjhenkes · 2023-04-14T14:25:32Z

system-tests/lib/system-tests.ts

@@ -342,7 +342,7 @@ const isVideoSnapshotError = (err: Error) => {
  _.pull(added, sometimesAddedVideoSnapshotLine, sometimesAddedSpacingLine)
  _.pull(deleted, sometimesDeletedVideoSnapshotLine, sometimesAddedSpacingLine)

-  return _.isEqual(added, expectedAddedVideoSnapshotLines) && _.isEqual(deleted, expectedDeletedVideoSnapshotLines)
+  return _.isEqual(added, expectedAddedVideoSnapshotLines) && (deleted.length === 0 || _.isEqual(deleted, expectedDeletedVideoSnapshotLines))


This is modifying an existing workaround for the firefox video error... previously when video was being compressed there were lines that were missing from the saved snapshot... since with compression off, the video name isn't logged, now there are not deleted lines.

mjhenkes · 2023-04-14T14:26:09Z

system-tests/test/video_compression_spec.js

@@ -101,3 +102,15 @@ describe('e2e video compression', () => {
    })
  })
 })
+
+describe('video compression 0', () => {


a new test for video compression 0

mjhenkes · 2023-04-14T14:27:14Z

I was honestly hoping to see better performance returns from system tests.

mschile · 2023-04-14T14:33:29Z

packages/frontend-shared/cypress.config.ts

@@ -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


Surprised this wasn't a typescript error.

cypress · 2023-04-14T14:50:25Z

3 flaky tests on run #45607 ↗︎

0	5193	77	0	3

Details:

Merge branch 'develop' into matth/chore/disable-compression
Project: cypress	Commit: `144df27edd`
Status: Passed	Duration: 13:17 💡
Started: Apr 17, 2023 9:22 PM	Ended: Apr 17, 2023 9:36 PM

cypress/e2e/cypress/cypress.cy.js • 3 flaky tests • 5x-driver-electron

View Output Video

Test		Artifacts
... > correctly returns currentRetry		`Output` `Video`
... > correctly returns currentRetry		`Output` `Video`
... > correctly returns currentRetry		`Output` `Video`

_{This comment has been generated by cypress-bot as a result of this
project's GitHub integration settings.}

system-tests/test/video_compression_spec.js

* chore: disable video compression * system test updates * moar system test fixes * a couple of telemetry tweaks * more intelligent defaults, don't touch firefox. * trying this * probably finally fix firefox system tests * unformat plz * ugh, also add a test * plz stop formatting that file * re-enable firefox tests * Change log * don't save videos to artifacts * this will be fixed in another pr * quoth the raven, quotes matter don't mess them up

Co-authored-by: Matt Henkes <mjhenkes@gmail.com> Co-authored-by: cypress-bot[bot] <+cypress-bot[bot]@users.noreply.github.com> Co-authored-by: Ryan Manuel <ryanm@cypress.io> Co-authored-by: Leosvel Pérez Espinosa <leosvel.perez.espinosa@gmail.com> Co-authored-by: astone123 <adams@cypress.io> Co-authored-by: Mark Noonan <mark@cypress.io> Co-authored-by: Mike Plummer <mike-plummer@users.noreply.github.com> Co-authored-by: Mike Plummer <mikep@cypress.io> Co-authored-by: Lachlan Miller <lachlan.miller.1990@outlook.com> Co-authored-by: Stokes Player <stokes.player@gmail.com> Co-authored-by: Stokes Player <stokes@cypress.io> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Adam Stone-Lord <adams@cypress.io> Co-authored-by: Ben M <benm@cypress.io> Co-authored-by: Jordan <jordan@jpdesigning.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Barthélémy Ledoux <bart@cypress.io> fix: Treat Video compression 0 as false. (#26503) fix: don't display run passing status if Cloud org is over run limit (#26533) fix: unify cdp approach to fix devtools in electron (#26573)

* feat/protocol: refactor: migrate from windi to tailwind (#26516) chore: update v8 generation vars so that from scratch depends implies updating the metafile (#26472) chore: Update Vite to 4.3.0 (#26553) fix: unify cdp approach to fix devtools in electron (#26573) dependency(deps): update dependency deps-ok to v1.4.1 🌟 (#26612) chore: update 12.11.0 release date (#26587) chore: 12.11.0 release (#26582) chore: implement experimental ESM stub/spy for Vite (#26536) chore: try triggering mouseleave on buttons to ensure that tooltips aren't showing (#26524) chore: add support for Angular 16 (#26052) chore: upgrade Vue to 3.2.47 (#26555) chore: Update v8 snapshot cache (#26537) chore: add missing utm parameters for cloud links to Debug page (#26556) chore: update stalebot to respect new labels and up process rate (#26552) fix: don't display run passing status if Cloud org is over run limit (#26533) chore: update vm2 to 3.9.17 (#26534) feat: display a limit warning on the run navigation component when there are 100 total runs (#26523) chore: Update v8 snapshot cache (#26476) chore: upgrade vm2 (#26495) fix: Treat Video compression 0 as false. (#26503)

mjhenkes added 11 commits April 12, 2023 10:47

chore: disable video compression

afff623

system test updates

3a812cc

moar system test fixes

80a5190

a couple of telemetry tweaks

7e1883d

more intelligent defaults, don't touch firefox.

13cab12

trying this

f297e42

probably finally fix firefox system tests

9fd896e

unformat plz

77578b6

ugh, also add a test

a1dc72b

plz stop formatting that file

602f7c3

Merge branch 'develop' into matth/chore/disable-compression

6a2b9b0

mjhenkes marked this pull request as ready for review April 14, 2023 14:20

mjhenkes commented Apr 14, 2023

View reviewed changes

mschile reviewed Apr 14, 2023

View reviewed changes

system-tests/test/video_compression_spec.js Outdated Show resolved Hide resolved

mschile approved these changes Apr 14, 2023

View reviewed changes

re-enable firefox tests

2621ece

emilyrohrbough approved these changes Apr 14, 2023

View reviewed changes

mjhenkes added 4 commits April 17, 2023 11:45

Merge branch 'develop' into matth/chore/disable-compression

e6596bf

Change log

dff448a

don't save videos to artifacts

a7b6168

this will be fixed in another pr

20baf66

mjhenkes added 2 commits April 17, 2023 14:27

quoth the raven, quotes matter don't mess them up

a96d2f2

Merge branch 'develop' into matth/chore/disable-compression

144df27

mjhenkes merged commit 3d0a2b4 into develop Apr 17, 2023

mjhenkes deleted the matth/chore/disable-compression branch April 17, 2023 21:33

AtofStryker mentioned this pull request May 22, 2023

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

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Treat Video compression 0 as false. #26503

fix: Treat Video compression 0 as false. #26503

mjhenkes commented Apr 14, 2023

mjhenkes commented Apr 14, 2023

mjhenkes Apr 14, 2023

mjhenkes Apr 14, 2023

mjhenkes Apr 14, 2023

emilyrohrbough Apr 14, 2023

mjhenkes Apr 14, 2023

mjhenkes Apr 14, 2023

mjhenkes Apr 14, 2023

mjhenkes Apr 14, 2023

mjhenkes commented Apr 14, 2023

mschile Apr 14, 2023

mjhenkes Apr 14, 2023

cypress bot commented Apr 14, 2023 •

edited

Loading

fix: Treat Video compression 0 as false. #26503

fix: Treat Video compression 0 as false. #26503

Conversation

mjhenkes commented Apr 14, 2023

Additional details

Steps to test

How has the user experience changed?

PR Tasks

mjhenkes commented Apr 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjhenkes commented Apr 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress bot commented Apr 14, 2023 • edited Loading

3 flaky tests on run #45607 ↗︎

cypress bot commented Apr 14, 2023 •

edited

Loading