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

chore: upgrade lerna to 6, cache build step #26913

Merged
merged 19 commits into from
Aug 10, 2023
Merged

Conversation

jordanpowell88
Copy link
Contributor

@jordanpowell88 jordanpowell88 commented Jun 1, 2023

Additional details

This PR does the following:

  • Upgrades Lerna from 5 => 6
  • Adds build as cacheable step

It allows for the build steps for the following packages to be cached locally and in the distributed cache (using Nx Cloud):

  • cypress (cli/)
  • @cypress/angular
  • create-cypress-tests
  • @cypress/schematic
  • @cypress/mount-utils
  • @cypress/react
  • @cypress/react18
  • @cypress/svelte
  • @cypress/vite-dev-server
  • @cypress/vite-plugin-cypress-esm
  • @cypress/vue
  • @cypress/vue2
  • @cypress/webpack-dev-server
  • @cypress/webpack-preprocessor
  • @packages/electron
  • @packages/errors
  • @packages/example
  • @packages/extension
  • @packages/icons
  • @packages/packherd-require
  • @packages/runner
  • @packages/telemetry
  • @packages/v8-snapshot-require
  • @tooling/electron-mksnapshot
  • @tooling/packherd
  • @tooling/v8-snapshot

Rationale

Most of the time when we work on the Cypress repo, we are only editing files in a few packages. If we've already built the source code for the other packages that we did not edit, then we shouldn't waste time and compute resources building all of those packages again.

We can use Lerna (using Nx under the hood) to cache builds when there have been no source code changes in the package. This way, when you run yarn or yarn build, Lerna only builds the packages that we haven't already built with that exact source code in the past.

As of right now we are being cautious to avoid accidental false positive cache hits, so we have configured Nx to invalidate the build cache for a package when any source files in the package change. In the future, and as we cache more steps, we should be able to fine-tune this so that we can exclude files that don't affect build outputs (*.md files, .eslintignore, etc.).

You can tell if Nx built your package from cache or not based on the output. To the right of the command, you will see some information in brackets like this

> @cypress/react18:build  [existing outputs match the cache, left as is]

This tells us that Nx did not actually build the @cypress/react18 package, but it pulled the build output from cache, because we've built @cypress/react18 with this exact source code before.

To better understand Nx features related to caching, you can read the following

Escape hatches

If you are building the Cypress monorepo and Nx is incorrectly pulling a build from cache, you can run the command with the --skip-nx-cache flag, or set NX_SKIP_NX_CACHE=true in your environment to force Nx to build everything from source and to ignore the cache. This normally should not happen, and if it does, then it means that we need to update the Nx configuration in the repo.

Going forward

We still have a bunch of packages that are built and tasks that are run using Gulp. These can't be orchestrated or cached by Nx. There are lots of performance gains to be made by moving these tasks out of Gulp and executing them via yarn scripts with Nx. In the future, we will slowly move these tasks out of Gulp and use Nx to orchestrate and cache them.

Steps to test

Run yarn locally on this branch. A lot of the packages that we build during the postinstall step should be taken from the remote cache if you haven't changed any code since CI has already built these packages and uploaded the results to Nx Cloud.

Make a change to the source code of a package from the list above, then run yarn again, and verify that the package was not built from cache but was actually built with your new source code changes.

How has the user experience changed?

n/a

PR Tasks

@cypress
Copy link

cypress bot commented Jun 1, 2023

4 failed and 35 flaky tests on run #49712 ↗︎

4 27875 1347 0 Flakiness 35

Details:

Merge branch 'develop' into jordanpowell88/lerna-6
Project: cypress Commit: 1e00362bf2
Status: Errored Duration: 23:34 💡
Started: Aug 10, 2023 3:12 PM Ended: Aug 10, 2023 3:36 PM
Failed  user_agent_override.cy.ts • 1 failed test • 5x-driver-electron

View Output Video

Test Artifacts
user agent override > persists modified user agent after cy.go Output Video
Failed  commands/navigation.cy.ts • 1 failed test • 5x-driver-chrome

View Output Video

Test Artifacts
cy.origin navigation > #consoleProps > .go() Output Video
Flakiness  specs_list_latest_runs.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs Output Screenshots Video
Flakiness  cypress-in-cypress-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
Cypress In Cypress CT > default config > redirects to the specs list with error if a spec is not found Output Screenshots Video
Flakiness  cypress-origin-communicator.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
Cypress In Cypress Origin Communicator > cy.origin passivity with app interactions > passes upon test reload mid test execution Output Screenshots Video
Flakiness  commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-firefox

View Output Video

Test Artifacts
network stubbing > intercepting request > can delay and throttle a StaticResponse Output
Flakiness  e2e/origin/commands/assertions.cy.ts • 1 flaky test • 5x-driver-firefox

View Output Video

Test Artifacts
cy.origin assertions > #consoleProps > .should() and .and() Output

The first 5 flaky specs are shown, see all 23 specs in Cypress Cloud.

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

Base automatically changed from jordanpowell88/add-linting to develop June 2, 2023 20:47
@jordanpowell88 jordanpowell88 marked this pull request as draft June 5, 2023 16:40
@astone123 astone123 marked this pull request as ready for review July 28, 2023 16:20
@astone123 astone123 changed the title chore: update lerna to 6 chore: upgrade lerna to 6 Jul 28, 2023
@lmiller1990
Copy link
Contributor

Looks good, just need a rebase and one more +1.

@astone123 astone123 changed the title chore: upgrade lerna to 6 chore: upgrade lerna to 6, cache build step Aug 7, 2023
@@ -1913,7 +1914,7 @@ jobs:
- restore_cached_workspace
- run:
name: Build
command: yarn workspace @cypress/webpack-preprocessor build
command: yarn lerna run build --scope @cypress/webpack-preprocessor
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order for these jobs to leverage caching we need to change them from yarn workspace commands to yarn lerna run commands

"targets": {
"build": {
"outputs": [
"{projectRoot}/types",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to specify these outputs because we generate /types in CI. By default, Nx looks in the following directories so deviations from those standards need to be specified.

"{workspaceRoot}/scripts"
],
"outputs": [
"{workspaceRoot}/cli/angular"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we copy these assets to the /cli directory we need to specify them here.

"targets": {
"build": {
"inputs": [
"{workspaceRoot}/scripts"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We specify /scripts because we still rely upon many node scripts for handling building (amongst other things). Any changes to a script should invalidate this lib. Eventually we can become more targeted and stricter in the future but we want to be more conservative to start off

@@ -23,14 +22,15 @@
"commander": "6.2.1",
"find-up": "5.0.0",
"fs-extra": "^9.1.0",
"inquirer": "7.3.3",
"glob": "^7.1.6",
"inquirer": "8.2.4",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a required change as Lerna 6 depends upon [email protected]+

"targets": {
"build": {
"outputs": [
"{projectRoot}/src/**/*.js",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to specify these like this because we currently do not place build assets in an outDir and get placed next to the ts files

Copy link
Contributor

@AtofStryker AtofStryker left a comment

Choose a reason for hiding this comment

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

Changeset looks good to me. Do we need to change the CONTRIBUTING guide to omit build-prod? https://github.com/cypress-io/cypress/blob/jordanpowell88/lerna-6/CONTRIBUTING.md#package-level-scripts

@astone123
Copy link
Contributor

astone123 commented Aug 9, 2023

@AtofStryker we could do that. I'm not sure why it's in there anyway, no one should be running build-prod during development. Although, I'm not sure if we need to update it

@AtofStryker
Copy link
Contributor

@astone123 didn't we remove those build steps entirely though? It a) doesn't make sense why they are there and b) since we removed the steps, shouldn't we just update the contributing doc to remove them as well? otherwise the scripts would just fail, right?

@astone123
Copy link
Contributor

@AtofStryker no, there are still a few packages that use build-prod, so that script still exists. I just removed the ones that were

"build-prod": "yarn build"

because they were exactly the same and we can cache them if they're called as build instead

@AtofStryker
Copy link
Contributor

@astone123 OK that makes sense. So in CI when we run the unit-tests build-prod will also call build as well to check the TS compilation or is that different now?

@astone123
Copy link
Contributor

@AtofStryker yeah that's right, I added a call to build here to make sure things are built correctly before we run the tests

Copy link
Contributor

@AtofStryker AtofStryker left a comment

Choose a reason for hiding this comment

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

OK makes sense to me!

@nagash77 nagash77 merged commit 9e60aeb into develop Aug 10, 2023
2 of 3 checks passed
@nagash77 nagash77 deleted the jordanpowell88/lerna-6 branch August 10, 2023 15:12
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 15, 2023

Released in 12.17.4.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v12.17.4, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Aug 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade Lerna to latest version
5 participants