-
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
Add --coverage option to configure to produce C based coverage reports #9463
Conversation
'ldflags': [ '-Wl,-z,noexecstack', | ||
'-Wl,--whole-archive <(V8_BASE)', | ||
'-Wl,--no-whole-archive' ] | ||
}], | ||
[ '(OS=="freebsd" or OS=="linux") and node_shared=="false" and coverage=="true"', { |
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.
does generating coverage on macOS work? Does it need any changes like this?
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.
@evanlucas I can run coverage on mac, but as the basis of running the CI job is to restrict it to running on Linux-x86, I haven't made any additional changes to support it.
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, thank you!
Fun on Freebsd to confirm failure is unrelated: Unless there are any objections before then will plan to land tomorrow. |
As a note there are other pieces that are needed to generate code coverage results but this is a good first step in order to remove potential conflicts in the nightly runs. |
Add --coverage option to configure to support compiling for generation of C based coverage reports PR-URL: #9463 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@CurryKitten for future reference, the title must be limited to 52 characters and have an appropriate prefix. I updated the first commit line as part of landing. |
landed as 5d9d415 |
Add --coverage option to configure to support compiling for generation of C based coverage reports PR-URL: #9463 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Add --coverage option to configure to support compiling for generation of C based coverage reports PR-URL: #9463 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Add --coverage option to configure to support compiling for generation of C based coverage reports PR-URL: #9463 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Add --coverage option to configure to support compiling for generation of C based coverage reports PR-URL: #9463 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Add --coverage option to configure to support compiling for generation of C based coverage reports PR-URL: #9463 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This PR is to support the ongoing efforts of the Nodejs testing workgroup under the following issue nodejs/testing#36 (comment)
Code coverage has currently been implemented by patching files on the fly in order to add in additional compilation options, but this has obvious limitations should the affected riles change in master.
This initial PR implements a
--coverage
option for the configure script which will then add the necessary cflags/ldflags in node.gyp in order to build node with C++ code coverage on Linux. Obviously, node builds as per normal without the--coverage
option specified in configure.