-
Notifications
You must be signed in to change notification settings - Fork 887
fix: Test flake when installing yarn dependencies on MacOS #436
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
Codecov Report
@@ Coverage Diff @@
## main #436 +/- ##
==========================================
+ Coverage 67.99% 68.26% +0.26%
==========================================
Files 156 159 +3
Lines 9046 9226 +180
Branches 73 73
==========================================
+ Hits 6151 6298 +147
- Misses 2289 2308 +19
- Partials 606 620 +14
Continue to review full report at Codecov.
|
Thanks for the quick review @kylecarbs ! I'll test this out w/ the other dependabot PRs - and then move on to investigating the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job diagnosing and fixing this @bryphe-coder 🎉 !
Co-authored-by: G r e y <vapurrmaid@pm.me>
This fixes #433 - a test flake in E2E (intermittent
ESOCKETTIMEDOUT
errors on MacOS).The main issue is that, occasionally, for very large dependencies (like
@material-ui/icons
) - yarn can actually time out! We researched this in-depth in v1: https://github.com/coder/m/pull/10040 and fixed it successfully there, by increasing the timeout for yarn.However, this also highlighted the fact that our
node_modules
caching behavior wasn't correct - we should very rarely see a timeout issue like this, because@material-ui/icons
should be cached.It turns out that we weren't falling back to the latest cached
node_modules
if there was a miss - so anytime the lock file changed, we'd invalidate the cache, and not restore the previous one. This can be improved by using therestore-keys
parameter of the@actions/cache
... and in fact we already do this for thego
dependencies.So this fix does two things:
@material-ui/icons
(and other large dependencies)ESOCKETTIMEDOUT
errors