-
Notifications
You must be signed in to change notification settings - Fork 30k
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
build: fix coverage generation #23769
Conversation
Coverage CI run to validate: https://ci.nodejs.org/view/Node.js%20Daily/job/node-test-commit-linux-coverage/806/ |
Coverage CI was good and I'd like to fast track this to get our coverage builds green again |
Regular CI run: https://ci.nodejs.org/job/node-test-pull-request/18012/ |
/CC @nodejs/build-files |
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. Maybe we can consider installing nyc in a directory under tools with a package.json to pin the version? (like what we do with lint-md-build
)
Failure was known issue seen before, resumed build here: https://ci.nodejs.org/job/node-test-pull-request/18045/ |
Changes in command line options for nyc resulted in the coverage target no longer working. Pin the major version of nyc and update the options to get it working again. PR-URL: nodejs#23769 Fixes: nodejs#23690 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: George Adams <george.adams@uk.ibm.com>
Landed as 9464e43 |
Changes in command line options for nyc resulted in the coverage target no longer working. Pin the major version of nyc and update the options to get it working again. PR-URL: #23769 Fixes: #23690 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: George Adams <george.adams@uk.ibm.com>
Arg, noticed I jumped the gun and the resume failed as well. Build of master to see if fails there consistently as well: https://ci.nodejs.org/job/node-test-commit-linux/22568/ |
This is the issue for the faliure -> #16601 here: test-fs-readfile-tostring-fail If the run in master fails, I'll look at creating a PR which marks the test as flaky. |
Build in master was ok. So I'm assuming that if I had resumed again, or rebased and run a new CI it would have passed. Sorry for having jumped the gun, but I think I'm ok to close this now. |
Changes in command line options for nyc resulted in the coverage target no longer working. Pin the major version of nyc and update the options to get it working again. PR-URL: #23769 Fixes: #23690 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: George Adams <george.adams@uk.ibm.com>
This lands cleanly on v10.x but requires a backport for 8.x |
Changes in command line options for nyc resulted in the coverage target no longer working. Pin the major version of nyc and update the options to get it working again. PR-URL: #23769 Fixes: #23690 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: George Adams <george.adams@uk.ibm.com>
Changes in command line options for nyc resulted in the coverage target no longer working. Pin the major version of nyc and update the options to get it working again. PR-URL: #23769 Fixes: #23690 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: George Adams <george.adams@uk.ibm.com>
Changes in command line options for nyc resulted in the coverage target no longer working. Pin the major version of nyc and update the options to get it working again. PR-URL: #23769 Fixes: #23690 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: George Adams <george.adams@uk.ibm.com>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesbuild: fix coverage generation
Changes in command line options for nyc resulted in the
coverage target no longer working.
Pin the major version of nyc and update the options to
get it working again.
Fixes: #23690