feat: set up auto prs for snapshot metafile changes #25052

ryanthemanuel · 2022-12-08T05:29:11Z

Closes Automate the regeneration of V8 snapshot cache files #24339

User facing changelog

n/a

Additional details

This PR offers several quality of life improvements for local development with V8 snapshots

A GitHub action has been created that will automatically update the snapshot cache file
- This action runs by default on develop and release/** branches and will issue a PR against the respective branches
- This action can be manually triggered against any branch providing several options
  - The branch name to run against
  - Whether or not to generate the dependencies from scratch (this takes some time)
  - Whether or not to commit directly to the branch or issue a PR to the branch
- This action will be scheduled to run weekly on wednesday at midnight UTC. It will generate the cache file from scratch.
The cache files for dev and prod have been merged down into one file. dev was always a subset of prod anyway, so I updated logic to read only node_modules from the file if we're in dev mode and the whole file if we're in prod mode
The cache file will now only be updated if V8_UPDATE_METAFILE is set to true and we're running in prod mode. This happens automatically on the GitHub action. This should limit the day to day concerns that people have with respect to the cache file (i.e. you won't ever have to check this file in unless you are explicitly wanting to)

Steps to test

You can trigger the workflow and run it against a branch to test it:

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 · 2022-12-08T05:29:14Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

ryanthemanuel · 2022-12-08T17:10:12Z

.github/workflows/update_v8_snapshot_cache.yml

-on: [workflow_dispatch]
+on:
+  schedule:
+    # Run every Wednesday at 00:00 UTC


I chose this so it'll be after the release. I'm ensuring to run from scratch then, so any changes will be caught early rather than right before the release.

cypress · 2022-12-08T19:48:25Z

Test summary

4762 • 0 • 336 • 0 • 8

Run details


Project	cypress
Status	Passed
Commit	`8bd54e4`
Started	Dec 13, 2022 4:03 AM
Ended	Dec 13, 2022 4:18 AM
Duration	15:35 💡
OS	Linux Debian -
Browser	Electron 106

View run in Cypress Dashboard ➡️

Flakiness

	user_agent_override.cy.ts	1
1	user agent override > persists modified user agent after cy.go
	cookie_behavior.cy.ts	4
1	... > same site / cross origin > XMLHttpRequest > sets cookie on same-site request if withCredentials is true, and attaches to same-site request if withCredentials is true
2	... > same site / cross origin > fetch > sets same-site cookies if "include" credentials option is specified from request, but does not attach same-site cookies to request by default (same-origin)
3	... > same site / cross origin > XMLHttpRequest > sets cookie on same-site request if withCredentials is true, and attaches to same-site request if withCredentials is true
4	... > same site / cross origin > fetch > sets same-site cookies if "include" credentials option is specified from request, but does not attach same-site cookies to request by default (same-origin)
This comment includes only the first 5 flaky tests. See all 8 flaky tests in the Cypress Dashboard.

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

Co-authored-by: cypress-bot[bot] <2f0651858c6e38e0+cypress-bot[bot]@users.noreply.github.com> Co-authored-by: Ryan Manuel <[email protected]>

BlueWinds · 2022-12-08T22:53:38Z

This action will be scheduled to run weekly on wednesday at midnight UTC. It will generate the cache file from scratch.

What's the advantage in generating this from scratch in the job, rather than updating the existing one?

ryanthemanuel · 2022-12-08T22:57:32Z

This action will be scheduled to run weekly on wednesday at midnight UTC. It will generate the cache file from scratch.

What's the advantage in generating this from scratch in the job, rather than updating the existing one?

Mainly performance improvements. Generally the way this works is that dependencies can go to a "worse" category, but can't go back the other way. Thus, over time, there may end up being more and more dependencies that could be optimized better but are not. This is a challenging problem to do iteratively, and I have not been able to figure out a way to solve it other than regenerating from scratch periodically.

BlueWinds · 2022-12-08T23:06:04Z

Looks like this adds several empty yarn.lock files? (because they're empty, I can't comment on them directly from the files tab).

BlueWinds · 2022-12-08T22:55:27Z

.github/workflows/update_v8_snapshot_cache.yml

+        type: boolean
+        default: false
+      commit_directly_to_branch:
+        description: 'Commit directly to branch'


When would we want to set this true?

If you're wanting to commit something to your feature branch but don't want to go through the rigmarole of a PR to your feature branch.

BlueWinds · 2022-12-08T23:01:42Z

tooling/v8-snapshot/cache/darwin/snapshot-meta.json

@@ -3930,8 +3930,8 @@
    "./packages/types/src/util.ts",
    "./packages/types/src/video.ts",
    "./packages/types/src/warning.ts",
-    "./tooling/v8-snapshot/cache/prod-darwin/snapshot-entry.js"
+    "./tooling/v8-snapshot/cache/darwin/snapshot-entry.js"


Love to see the reduced duplication. 👍

BlueWinds · 2022-12-08T23:08:26Z

tooling/v8-snapshot/src/generator/snapshot-generator.ts

      forceNoRewrite: this.forceNoRewrite.size,
      auxiliaryData: auxiliaryDataKeys,
    })
  }

  private _addGitignore () {
-    const gitignore = 'snapshot.js\nbase.snapshot.js.map\nprocessed.snapshot.js.map\nesbuild-meta.json\nsnapshot-meta.json\nsnapshot-entry.js\n'
+    const gitignore = 'snapshot.js\nbase.snapshot.js.map\nprocessed.snapshot.js.map\nesbuild-meta.json\nsnapshot-entry.js\n'


This should limit the day to day concerns that people have with respect to the cache file (i.e. you won't ever have to check this file in unless you are explicitly wanting to)

I'm not sure I follow this. When you say won't have to check it in - will the cache file still show up in the git diff output? I haven't been clear on when I should be checking in changes or not, and therefor also not sure how this PR changes it.

The key bit happens here. Basically when you're developing locally, V8_UPDATE_METAFILE won't be set and so the cache files won't be written. The only time they will be written is with the github action (or if you explicitly set that environment variable locally for some reason).

lmiller1990

Looks good, please verify that we are not using a soon-to-be-deprecated CircleCI API. My other comments are just ideas/questions, not blockers.

lmiller1990 · 2022-12-08T23:25:28Z

.github/workflows/update_v8_snapshot_cache.yml

+    - cron: '0 0 * * 3'
+  push:
+    branches:
+      - ryanm/feature/v8-snapshots-auto-pr


Just to check is this a scheduled PIPELINE or a scheduled WORKFLOW? The reason I ask https://circleci.com/docs/configuration-reference/#schedule

The scheduled workflows feature is set to be deprecated. Using scheduled pipeline.

This is in a workflows file and called schedule - just want to verify we are not using a soon-to-be-deprecated feature.

Ah yeah. That's circle ci. I'm using it in a github action where it is not deprecated.

lmiller1990 · 2022-12-08T23:26:24Z

.github/workflows/update_v8_snapshot_cache.yml

+              branchName: '${{ env.BRANCH_NAME }}',
+              description: 'Update v8 snapshot cache',
+              body: 'This PR was automatically generated by the [update-v8-snapshot-cache](https://github.com/cypress-io/cypress/actions/workflows/update_v8_snapshot_cache.yml) github action.',
+              reviewers: ['ryanthemanuel']


lmiller1990 · 2022-12-08T23:29:44Z

scripts/unit/github-actions/create-pull-request-spec.js

@@ -0,0 +1,86 @@
+const { expect } = require('chai')


Random q, I wonder if a better way to test if this is working would be to just have a local check in the monorepo. Eg, when you run yarn install, we would (somehow) check the last time the snapshot metadata was updated. If it's been more than , we could either error or warn "snapshot hasn't been updated in a week; is the automation working?"

Just an idea - if this GHA stopped working, or we changed to use another VCS service, we'd need to remember to do this. Not a requirement for this PR, specifically - just floating an idea to make the monorepo more self maintaining.

lmiller1990 · 2022-12-08T23:30:40Z

tooling/v8-snapshot/src/doctor/determine-deferred.ts

-      hashFilePath != null,
-      `Unable to find hash file inside ${projectBaseDir}`,
-    )
+  healthy.forEach((dependency) => {


Going to admit I don't fully grok the finer details of some parts of this PR (like this). I get the general idea though, and it looks like this makes sense.

mschile · 2022-12-08T23:31:27Z

.github/workflows/update_v8_snapshot_cache.yml

+    - cron: '0 0 * * 3'
+  push:
+    branches:
+      - ryanm/feature/v8-snapshots-auto-pr


Remove before merging?

I could. But I also sort of see this as similar to what we do in the circle ci file. If you're working on it on your branch, update it with your branch, and so on and so on.

…napshots-auto-pr # Conflicts: # tooling/v8-snapshot/cache/darwin/snapshot-meta.json

Co-authored-by: cypress-bot[bot] <2f0651858c6e38e0+cypress-bot[bot]@users.noreply.github.com>

Co-authored-by: cypress-bot[bot] <2f0651858c6e38e0+cypress-bot[bot]@users.noreply.github.com> Co-authored-by: Ryan Manuel <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

ryanthemanuel added 16 commits December 5, 2022 16:52

feat: set up auto prs for snapshot metafile changes

360fd38

fix build

3b4756d

cleanup

50e7f87

cleanup

8dccd36

fix windows

a5ce82d

fix windows

2b0a400

fix windows

4395113

refactoring

3e3a745

fix build

d32457a

fix build

00209b9

fix build

29188b3

fix tests

a091afc

do a schedule

393fa12

do a schedule

a477fe5

refactor a few things

add79e3

fix unit test

b9ddb82

ryanthemanuel added 5 commits December 7, 2022 23:37

try to assign me as a reviewer

d351ba5

fix always run generate from scratch

56d11ad

try to assign me as a reviewer

d62a961

request me as reviewer

328d897

fix build

ee13c3a

ryanthemanuel commented Dec 8, 2022

View reviewed changes

ryanthemanuel added 4 commits December 8, 2022 11:43

further refactoring

26dc547

merge develop

4f0770b

make this closer to what it will look like in prod

8810000

auto merge

c526471

ryanthemanuel marked this pull request as ready for review December 8, 2022 19:17

Merge branch 'develop' into ryanm/feature/v8-snapshots-auto-pr

494db80

ryanthemanuel and others added 3 commits December 8, 2022 14:30

docs and minor improvements

ce152c4

chore: Update v8 snapshot cache (#25069)

7fc0a2b

Co-authored-by: cypress-bot[bot] <2f0651858c6e38e0+cypress-bot[bot]@users.noreply.github.com> Co-authored-by: Ryan Manuel <[email protected]>

fix from scratch

111bfc1

BlueWinds reviewed Dec 8, 2022

View reviewed changes

BlueWinds approved these changes Dec 8, 2022

View reviewed changes

lmiller1990 reviewed Dec 8, 2022

View reviewed changes

mschile reviewed Dec 8, 2022

View reviewed changes

mschile approved these changes Dec 8, 2022

View reviewed changes

fix committing directly to branch

177990f

lmiller1990 self-requested a review December 10, 2022 12:42

lmiller1990 approved these changes Dec 10, 2022

View reviewed changes

ryanthemanuel and others added 2 commits December 12, 2022 19:23

Merge remote-tracking branch 'origin/develop' into ryanm/feature/v8-s…

54ad887

…napshots-auto-pr # Conflicts: # tooling/v8-snapshot/cache/darwin/snapshot-meta.json

chore: Update v8 snapshot cache (#25121)

8bd54e4

Co-authored-by: cypress-bot[bot] <2f0651858c6e38e0+cypress-bot[bot]@users.noreply.github.com>

ryanthemanuel merged commit 56bebb1 into develop Dec 13, 2022

ryanthemanuel deleted the ryanm/feature/v8-snapshots-auto-pr branch December 13, 2022 14:29

cypress-bot bot mentioned this pull request Dec 13, 2022

Automate the regeneration of V8 snapshot cache files #24339

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: set up auto prs for snapshot metafile changes #25052

feat: set up auto prs for snapshot metafile changes #25052

ryanthemanuel commented Dec 8, 2022 •

edited

Loading

cypress-bot bot commented Dec 8, 2022

ryanthemanuel Dec 8, 2022

cypress bot commented Dec 8, 2022 •

edited

Loading

BlueWinds commented Dec 8, 2022

ryanthemanuel commented Dec 8, 2022

BlueWinds commented Dec 8, 2022

BlueWinds Dec 8, 2022

ryanthemanuel Dec 9, 2022

BlueWinds Dec 8, 2022

BlueWinds Dec 8, 2022

ryanthemanuel Dec 9, 2022

lmiller1990 left a comment

lmiller1990 Dec 8, 2022

ryanthemanuel Dec 9, 2022

lmiller1990 Dec 8, 2022

lmiller1990 Dec 8, 2022

lmiller1990 Dec 8, 2022

mschile Dec 8, 2022

ryanthemanuel Dec 9, 2022

feat: set up auto prs for snapshot metafile changes #25052

feat: set up auto prs for snapshot metafile changes #25052

Conversation

ryanthemanuel commented Dec 8, 2022 • edited Loading

User facing changelog

Additional details

Steps to test

How has the user experience changed?

PR Tasks

cypress-bot bot commented Dec 8, 2022

Choose a reason for hiding this comment

cypress bot commented Dec 8, 2022 • edited Loading

Test summary

Run details

Flakiness

BlueWinds commented Dec 8, 2022

ryanthemanuel commented Dec 8, 2022

BlueWinds commented Dec 8, 2022

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

lmiller1990 left a 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

Choose a reason for hiding this comment

ryanthemanuel commented Dec 8, 2022 •

edited

Loading

cypress bot commented Dec 8, 2022 •

edited

Loading