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: multiple measures to make e2e tests more reliable #4637

Merged
merged 2 commits into from
Jun 7, 2022

Conversation

danez
Copy link
Contributor

@danez danez commented May 27, 2022

Summary

Refs #4625

  • Cache the verdaccio storage between e2e test runs to speed them up.
  • Cache package information in verdaccio up to 1 day before rechecking npmjs.com
  • increase test timeout to 10minutes, I rather wait longer than have to restart the tests
  • Only write .npmrc file during publishing to verdaccio. For the installation this is not needed and the registry can be set by an environment variable.

@danez danez added type: chore work needed to keep the product and development running smoothly area: ci labels May 27, 2022
@danez danez self-assigned this May 27, 2022
@github-actions
Copy link

github-actions bot commented May 27, 2022

📊 Benchmark results

Comparing with 9240a0f

Package size: 294 MB

(no change)

^  287 MB  287 MB  287 MB  287 MB  287 MB  287 MB  287 MB  287 MB  287 MB  287 MB  287 MB  287 MB  294 MB 
│   ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

@danez danez force-pushed the verdaccio-memory branch from bd2a2e7 to e861a84 Compare May 27, 2022 18:44
@danez danez force-pushed the verdaccio-memory branch 3 times, most recently from 2e1ef4e to a9dcf11 Compare May 30, 2022 08:05
@danez danez marked this pull request as ready for review May 30, 2022 08:12
@danez danez requested review from lukasholzer and a team May 30, 2022 08:12
@danez danez force-pushed the verdaccio-memory branch from a9dcf11 to b8b7bf7 Compare May 30, 2022 08:15
@danez danez changed the title chore: use verdaccio memory plugin to speed up e2e tests chore: use verdaccio memory plugin to reduce timeouts in e2e tests May 30, 2022
Copy link
Contributor

@lukasholzer lukasholzer left a comment

Choose a reason for hiding this comment

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

I recall that I had used the verdaccio in memory plugin and it was slower on consecutive runs (as it always needed to download all node_modules) therefore I switched to a disk based solution as the artifacts (node_modules) can be cached by circlecis disk cache and persisted:

The evolution of this is in the ocean project [internal url]: https://github.com/netlify/ocean/blob/main/.circleci/config.yml#L126

      - save_cache:
          key: e2e-{{ arch }}-v1-{{ checksum "package.json" }}
          paths:
            - .verdaccio-cache

CleanShot 2022-05-30 at 10 54 00

Instead of leveraging the in memory version I would persist the cache as this is way faster on consecutive or similar runs

@danez danez force-pushed the verdaccio-memory branch from b8b7bf7 to 96912ee Compare June 1, 2022 16:26
@danez danez changed the title chore: use verdaccio memory plugin to reduce timeouts in e2e tests chore: multiple measures to speed up e2e tests Jun 1, 2022
@danez danez force-pushed the verdaccio-memory branch 2 times, most recently from 74def75 to 2c4a8bc Compare June 1, 2022 16:50
@danez danez changed the title chore: multiple measures to speed up e2e tests chore: multiple measures to make e2e tests more reliable Jun 1, 2022
@danez
Copy link
Contributor Author

danez commented Jun 1, 2022

Okay I reworked it, it is now caching the verdaccio storage between builds.

@danez danez requested a review from lukasholzer June 1, 2022 17:44
await rmdirRecursiveAsync(workspace)
}

env.npm_config_registry = url
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 replaces the writing of the npmrc file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work on linux as well? Had some issues on windows and linux when I recall correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

execa which we use to call the package managers, does automatically inherit all environment variables.

the weird part is: as we call the e2e tests through an npm script this environment variable is already set, but with the proper npmjs registry. This is why I guess in the past we had to use --registry when calling the package managers, as the environment variable always takes precedence over any other config format besides cli options.

lukasholzer
lukasholzer previously approved these changes Jun 7, 2022
Copy link
Contributor

@lukasholzer lukasholzer left a comment

Choose a reason for hiding this comment

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

Thanks for reworking looks good to me! 🐳

increase timeout to 10min
cache verdaccio storage in github actions
only write .npmrc during publish
cache packages in verdaccio up to 1 day
@@ -4,7 +4,6 @@ on:
push:
branches: [main]
pull_request:
types: [synchronize, opened, reopened, edited]
Copy link
Contributor Author

@danez danez Jun 7, 2022

Choose a reason for hiding this comment

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

I changed this, as the e2e tests would run whenever the main body or title was edited, which I don't think makes sense. I saw that edited is also triggered when the base branch changes, but I guess that is a less common scenario and in that case kodiak would merge anyway and rerun stuff.
The other 3 types are default anyway.

@danez danez requested a review from lukasholzer June 7, 2022 08:44
@danez danez added the automerge Add to Kodiak auto merge queue label Jun 7, 2022
@kodiakhq kodiakhq bot merged commit 77d387d into main Jun 7, 2022
@kodiakhq kodiakhq bot deleted the verdaccio-memory branch June 7, 2022 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ci automerge Add to Kodiak auto merge queue type: chore work needed to keep the product and development running smoothly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants