build: fix coverage build#18409
Conversation
addaleax
left a comment
There was a problem hiding this comment.
Thanks for taking care of this!
|
updated the change based on @joyeecheung comment. Please kick off a new CI to verify it. not sure about the failure of fedora24. Let's see if it fails again in new CI. |
|
still got weird failures on |
|
Seems the debian failure is related to performance. That should not be related to this PR. For those failures in fedora24, I am going to verify that locally. At the same time, I rebased my branch and Please kick off another CI to verify it. |
|
Another CI run to see if the fedora24 failures re-occur: https://ci.nodejs.org/job/node-test-pull-request/12862/ |
mhdawson
left a comment
There was a problem hiding this comment.
LGTM provided CI run is green.
|
The failures in debian8-64 happened in other PR and should be irrelevant. |
|
@yhwang would you be so kind and rebase? :-) |
After adding the node_lib target in node.gyp, most of the node source files are moved to that target. When coverage option is enabled, corresponding cflags and ldflags are needed in that target as well. gcovr also needs to check .gcda data for both node and node_lib. Fixes: nodejs#18402 Signed-off-by: Yihong Wang <yh.wang@ibm.com>
|
@BridgeAR rebase is done. Please kick off a CI. |
|
only failed in |
|
All ARM failures look like infra issues, and there were no failures on earlier CI runs so I think we are good. Landing. |
|
Landed as a89d215 |
After adding the node_lib target in node.gyp, most of the node source files are moved to that target. When coverage option is enabled, corresponding cflags and ldflags are needed in that target as well. gcovr also needs to check .gcda data for both node and node_lib. PR-URL: #18409 Fixes: #18402 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
|
Should this be backported to |
|
@MylesBorins I guess code coverage is only for master build. right? if that's the case, then there is no need to backport. |
After adding the node_lib target in node.gyp, most of the node source files are moved to that target. When coverage option is enabled, corresponding cflags and ldflags are needed in that target as well. gcovr also needs to check .gcda data for both node and node_lib. PR-URL: nodejs#18409 Fixes: nodejs#18402 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
After adding the node_lib target in node.gyp, most of the node source files are moved to that target. When coverage option is enabled, corresponding cflags and ldflags are needed in that target as well. gcovr also needs to check .gcda data for both node and node_lib. PR-URL: nodejs#18409 Fixes: nodejs#18402 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
After adding the node_lib target in node.gyp, most of the node source
files are moved to that target. When coverage option is enabled,
corresponding cflags and ldflags are needed in that target as well.
gcovr also needs to check .gcda data for both node and node_lib.
Fixes: #18402
Signed-off-by: Yihong Wang yh.wang@ibm.com
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)