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: add v8 snapshot usage to cypress in cypress testing #24860

Merged
merged 36 commits into from Dec 8, 2022

Conversation

ryanthemanuel
Copy link
Collaborator

@ryanthemanuel ryanthemanuel commented Nov 28, 2022

User facing changelog

n/a

Additional details

This adds support for utilizing the v8 snapshot in cypress in cypress testing. In order to do this, we need to ensure that when the child process is started it is done via the spawn command since electron has special logic in its fork command which effectively disables the snapshot. This has the implication of having an additional Cypress icon in the dock on Macs, but that will only happen during development and is a small inconvenience given that we will be able to more accurately simulate prod during cypress in cypress tests.

In addition, as just general cleanup, I cleaned up the bytenode logic and was able to make it a dev dependency by bundling it into the electron entry point

Steps to test

Running the app and launchpad tests is the best way to see this in action

How has the user experience changed?

n/a

PR Tasks

  • Have tests been added/updated?
  • [na] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • [na] Has a PR for user-facing changes been opened in cypress-documentation?
  • [na] Have API changes been updated in the type definitions?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 28, 2022

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Nov 28, 2022



Test summary

428 0 18 0Flakiness 5


Run details

Project cypress
Status Passed
Commit 9ba8ca8
Started Dec 8, 2022 5:51 AM
Ended Dec 8, 2022 6:07 AM
Duration 15:50 💡
OS Windows 10.0.17763
Browser Chrome 108

View run in Cypress Dashboard ➡️


Flakiness

cypress\e2e\specs.cy.ts Flakiness
1 ... > opens config file in ide from specPattern text description
cypress\e2e\specs_list_latest_runs.cy.ts Flakiness
1 App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs
cypress\e2e\subscriptions\configChange-subscription.cy.ts Flakiness
1 configChange subscription > on runner page > responds to configChange event and re-runs spec
cypress\e2e\cypress-origin-communicator.cy.ts Flakiness
1 Cypress In Cypress Origin Communicator > cy.origin passivity with app interactions > passes upon test reload mid test execution
cypress\e2e\cypress-in-cypress-run-mode.cy.ts Flakiness
1 Cypress In Cypress - run mode > e2e run mode spec runner header is correct

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@ryanthemanuel ryanthemanuel marked this pull request as ready for review December 5, 2022 21:21
@@ -21,7 +21,7 @@
"@babel/parser": "^7",
"@babel/plugin-syntax-typescript": "^7",
"@babel/plugin-transform-typescript": "^7",
"@babel/traverse": "^7",
"@babel/traverse": "7.15.4",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated this to match the rest of the repo.

@AtofStryker AtofStryker self-requested a review December 6, 2022 18:25
Copy link
Contributor

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

Great, I have a question, but no blocker.

@@ -310,6 +319,17 @@ export class ProjectConfigIpc extends EventEmitter {
debug(`no typescript found, just use regular Node.js`)
}

if (process.env.CYPRESS_INTERNAL_E2E_TESTING_SELF_PARENT_PROJECT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Bit of a shame we need to ship test code to prod :'(

This is "the change" that lets v8 snapshots run in cy-in-cy?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there anyway to verify it now uses snapshots in the tests? How do we know it works (and catch it if stops?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah it's unfortunate, but I'm not sure of a clean way to do it otherwise with how everything is structured.

And, I like your idea. I added a check to ensure that we fail in cypress in cypress if the snapshot is unavailable.

@ryanthemanuel ryanthemanuel merged commit c540284 into develop Dec 8, 2022
@ryanthemanuel ryanthemanuel deleted the ryanm/fix/cy-in-cy-and-v8-snapshots branch December 8, 2022 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants