-
Notifications
You must be signed in to change notification settings - Fork 370
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
fix(deps): regenerate lockfile to reduce dependencies #4083
Conversation
The Windows CI failure seems to happen on main sporadically too. BTW, the E2E tests are pretty nice, I can see a few good warnings :) |
I'm already curating a list with all flaky builds as I want to reduce them sadly our tests are not written that well so they use the file system and depend heavily on it and on API calls this makes it even more likely to be flaky. But this topic has my full attention as it slows us really down. |
@erezrokah I'm retriggering now for the 4th time this PR and still not getting green always a different test that is flaky! We need to take some action on this. |
I suggest to disable the flaky tests for now, as they are not helpful. We can work on them as a separate issue and enable when we stabilize them. |
I say better trigger renovate to update build, config etc, and ping me to redo this PR one last time :) |
@XhmikosR could you give me some insights on how you cleaned it up? (I would love to understand the resolution of a shrinkwrap file more) |
Just remove the lock file, But in this particular case I've kept back 2 packages because they are deprecated (the new superagent and supertest versions). |
It seems the size benchmark doesn't run here (probably because I'm the patch author) so here are the results:
I can't keep rebasing this forever so I'd say first land #4072 and #4090 and then this one. :) |
still flaky windows tests 😢 |
Have you guys tried to reduce ava concurrency? I know that the Actions Windows env is not exactly the same as vanilla Windows, but whenever I run the cli tests on my Windows VM with the default scripts, I was getting hundreds of the permissions errors. If I use Generally, I'm not sure if spawning so many processes is good regardless of OS. You can see some tests take minute(s) to finish, not because they are slow, but probably because the system is being bombarded with many simultaneous tasks. EDIT: OK, I give up :/ The |
Ava defaults to concurrency 2 in CI: I believe this number only impacts the number of specs to run in parallel (a spec maps out to a file), and not number of tests in parallel within a single spec. If that's the case, we might benefit from splitting specs with many tests to reduce the number of processes. |
That's a good idea worth trying at least! 💡 |
Weird, the CI time did differ a lot...
Anyway, I hope you guys can improve this too later :) |
This PR currently has a merge conflict. Please resolve this and then re-add the |
This is moot now since this was done in another commit. Similarly, #4072 is moot too. |
@XhmikosR sorry can you please resolve the conflicts? Seems like the disabled windows tests where solving the flakyness |
I can't rebase because I dropped the branch since you included these changes in #3904 |
Then I will close the PR. I try to open up the regeneration in in a new one :) |
Shouldn't really help anymore :) |
yea I already did it on a PR that got merged so I hope we are fine now :) Thx for your continues effort on this topic @XhmikosR 🙏 we are so glad you helped with your expertise! I'm saying this in behalf of the whole team @netlify/build-services :) |
I've added most of my suggestions in #3941. It's a lot, I know, but I've already submitted quite a few upstream PRs. Personally, I'd freeze adding new packages for a while and address some of those issues. Unsure if this branch even works (due to the listr2 switch)n but the results are promising:
|
🎉 Thanks for submitting a pull request! 🎉
Summary
Fixes #<replace_with_issue_number>
For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)