-
Notifications
You must be signed in to change notification settings - Fork 168
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: better skip logic #681
fix: better skip logic #681
Conversation
@mcollina PTAL |
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.
lgtm
What is the reason for remaining broken test? (it's broken locally as well) |
* ci: trigger * ci: trigger * ci: fail in templates-without-ts-esm now * ci: fail in template-ts-esm now * ci: pass now * ci: now everything works with direct config resolution * ci: now add back template test and fail * ci: point to good tsconfig and pass * fix: nl in config * ci: rename to unit:ts-esm * ci: split greedy glob into ts/js, enumerate all template tests separatelly * ci: all globs are quoted
So, those Very simple fix is to not point to templates/**/tsconfig.json, and try to resolve fastify-config from there, but to point to a copy of it which I've put in For even more clarity, I've refactored pkg.scripts that deal with tests into the following shape:
|
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.
lgtm
@mcollina you can just re-run this failed test. |
Looking good now |
This adds better logic for skipping certain tests that rely on
--no-coverage
flag to be provided.Ideally, all test suites I should be able to run with any of:
tap /pattern/
tap /pattern/ --no-coverage
c8 tap /pattern/
This PR gives me that flexibility for these 2 suites (
test/print-*.test.js
)BEFORE:
NOW:
Clarification:
tap
(16.x) without a--no-coverage
flag will have aNYC_PROCESS_ID
inprocess.env
c8
will assign a coverage/tmp dir into aNODE_V8_COVERAGE
env variableCI was not the reason those tests hanged.
It is
c8
intest
script that is being run by CI, on top of all unit tests with--no-coverage
flag, resulting in effectively undo-ing what that flag was doing for local testing anyway.Checklist
npm run test
andnpm run benchmark
and the Code of conduct