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

feat: enable env-setup caching #48

Merged
merged 15 commits into from
Oct 14, 2024
Merged

feat: enable env-setup caching #48

merged 15 commits into from
Oct 14, 2024

Conversation

getlarge
Copy link
Collaborator

Description

This PR introduces caching for the entire env-setup (in the pb-ve-env-setup target created in e2e projects.

Screenshot 2024-10-11 at 12 41 02

Can be tested by:

  1. running nx run utils-e2e:e2e --verbose
  2. modifying the end 2 end test (examples/e2e/utils-e2e/test/utils-sort-user-list.spec.ts)
  3. running nx run utils-e2e:e2e --verbose again

Related to #33

Copy link
Collaborator Author

@getlarge getlarge left a comment

Choose a reason for hiding this comment

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

I'd like to add some optimizations:

  • ensure verdaccio is stopped in case of executor error
  • only use async methods in NPM-related executors

nx.json Outdated Show resolved Hide resolved
nx.json Outdated Show resolved Hide resolved
Copy link
Contributor

@BioPhoton BioPhoton left a comment

Choose a reason for hiding this comment

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

Excited for this PR!! I left some comments. Mostly questions.
The PR comment from code-pushup is not running anymore. To be able to produce benchmarks this is important to get green.

Other than that, this PR ships the main impact for the idea. :) Thanks.

nx.json Show resolved Hide resolved
@getlarge getlarge requested a review from BioPhoton October 13, 2024 14:40
@getlarge
Copy link
Collaborator Author

getlarge commented Oct 14, 2024

@BioPhoton, Could you please have a look at the failing unit test, please?
You can scroll doooown to find the logs in the job. BTW, why are there some many vitest logs?

Screenshot 2024-10-14 at 07 14 51

Copy link

github-actions bot commented Oct 14, 2024

Code PushUp

🤨 Code PushUp report has both improvements and regressions – compared target commit f90cbb3 with source commit cba2e67.

🏷️ Categories

🏷️ Category ⭐ Previous score ⭐ Current score 🔄 Score change
Performance 🟡 67 🟡 58 ↓ −9
👍 2 audits improved, 👎 6 audits regressed, 2 audits changed without impacting score

🛡️ Audits

🔌 Plugin 🛡️ Audit 📏 Previous value 📏 Current value 🔄 Value change
Nx Performance Checks [Graph Time] task graph 🟩 3.27 ms 🟥 3090.04 ms ↑ +94281 %
Nx Performance Checks [Task Time] nx-verdaccio-e2e:nxv-e2e 🟥 40.80 s 🟥 41.10 s ↑ +1 %
Nx Performance Checks [Task Time] models-e2e:e2e 🟨 10.38 s 🟨 10.51 s ↑ +1 %
Nx Performance Checks [Task Time] models-e2e:nxv-e2e 🟨 10.81 s 🟨 10.76 s ↓ +0 %
Nx Performance Checks [Task Time] models-e2e:nxv-env-setup 🟨 9.10 s 🟨 9.15 s ↑ +1 %
Nx Performance Checks [Task Time] models-e2e:nxv-env-bootstrap 🟩 3.60 s 🟩 3.61 s ↑ +0 %
Nx Performance Checks [Task Time] models-e2e:nxv-env-teardown 🟩 483 ms 🟩 475 ms ↓ −2 %
Nx Performance Checks [Cache Size] nx-verdaccio-e2e:nxv-env-setup 🟥 167.1 MB 🟥 167.1 MB ↑ +0 %
Nx Performance Checks [Task Time] cli-e2e-original:original-e2e 🟥 151.66 s 🟥 149.96 s ↓ −1 %
Nx Performance Checks [Graph Time] project graph 🟥 2777.99 ms 🟥 3078.41 ms ↑ +11 %

1 other audit is unchanged.

@getlarge
Copy link
Collaborator Author

@BioPhoton, Could you please have a look at the failing unit test, please? You can scroll doooown to find the logs in the job. BTW, why are there some many vitest logs?

Seems like this test is flaky, it passed now without changing the code.

Copy link
Contributor

@BioPhoton BioPhoton left a comment

Choose a reason for hiding this comment

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

Nice! Lets ship this core functionality

@getlarge getlarge merged commit 8509ca4 into main Oct 14, 2024
6 checks passed
@getlarge getlarge deleted the feat-enable-setup-caching branch October 14, 2024 15:02
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.

2 participants