chore: add --changedSince to CI test runs on other Node.js versions #4661
Conversation
|
Thanks for the PR, @JoshuaKGoldberg! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day. |
|
✔️ Deploy Preview for typescript-eslint ready! 🔨 Explore the source changes: 8ddf512 🔍 Inspect the deploy log: https://app.netlify.com/sites/typescript-eslint/deploys/6229fc248e66e000080e81ca 😎 Browse the preview: https://deploy-preview-4661--typescript-eslint.netlify.app |
435a4cb to
cb73516
Compare
|
UPDATE: Sorry I initially misunderstood what this was doing, I see it is related to tests within a particular project - as you were 😅 UPDATE (2): This probably does require some broader thinking through, though, because conceptually the idea of the project graph is what is determining whether or not the test results for the project are restored from cache or not. If the eslint-plugin project was deemed to not be affected today, the results are ready in less than one second already. So this change is targeting a case where the project graph has determined the project is affected and the tests should therefore be run... Just thinking out loud at this stage but I think this would expose us to some new edge cases.
|
Codecov Report
@@ Coverage Diff @@
## main #4661 +/- ##
==========================================
- Coverage 94.25% 92.67% -1.59%
==========================================
Files 151 317 +166
Lines 7971 11299 +3328
Branches 2573 3337 +764
==========================================
+ Hits 7513 10471 +2958
- Misses 262 563 +301
- Partials 196 265 +69
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Makes sense. Since it's really just
Yes! I would love to see nx onboarded more fully -- and to learn more about adopting nx in general! Will file a followup task. |
|
Blurgh, needs more work on the Git checkout... https://github.com/typescript-eslint/typescript-eslint/runs/5495766395?check_suite_focus=true |
|
Speaking of those edge cases: I think this is also either blocked on us completely auto-generating |
|
(closing to keep my queue small) |
PR Checklist
Overview
Uses Jest's
--changedSinceagainst the target branch in a PR to cut down on the number of unnecessary tests run on other Node.js versions. This means we'll get reports of unit test failures ineslint-plugina minute or two sooner!Doesn't use this in the primary Node.js version because the test coverage report still needs to include all files.
Not that I think this is reachable, but if the
${{ github.event.pull_request.base.sha }}variable is blank/empty then all tests will be run.