Skip to content
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

Improve make coverage target #11484

Closed
mhdawson opened this issue Feb 21, 2017 · 11 comments
Closed

Improve make coverage target #11484

mhdawson opened this issue Feb 21, 2017 · 11 comments
Labels
build Issues and PRs related to build files or the CI.

Comments

@mhdawson
Copy link
Member

  • Version: master
  • Platform: all
  • Subsystem: test

The "make coverage" target was recently added in #10856. There is still some additional work that is needed to complete this effort including:

  1. Platform support, allow the new target to work/run across platforms. Next target is to make it run on OS X
  2. Summary generation. have the target generate the initial summary/history page like we see on coverage.nodejs.org so that if you do multiple runs you can more easily see/compare results
  3. add some additional targets so that you can run just javascript or just c++ coverage
  4. remove need to patch some of the underlying tools
@addaleax addaleax added the build Issues and PRs related to build files or the CI. label Feb 21, 2017
@addaleax
Copy link
Member

Also, maybe we should just add everything that make coverage pulls from the nodejs/testing repository to tools?

@joyeecheung
Copy link
Member

On a side note, is it ready to be documented in the guides? Should be quite useful to contributors, although the "not working on OS X" part needs to be pointed out at the moment.

@gibfahn
Copy link
Member

gibfahn commented Feb 22, 2017

On a side note, is it ready to be documented in the guides?

+1 to a guide-to-coverage.md in doc/guides/

@Fishrock123
Copy link
Contributor

Note: make coverage works on OS X today, but ./configure --coverage does not.

That is to say: you can run coverage just fine for JS but not for C++.

@joyeecheung
Copy link
Member

Note: make coverage works on OS X today, but ./configure --coverage does not.
That is to say: you can run coverage just fine for JS but not for C++.

Yep, though I am consistently getting errors when running on 15.6.0 Darwin Kernel/OS X El Capitan 10.11.6 (15G1217), is that a known issue?

Click to see errors
/usr/local/opt/python/bin/python2.7 tools/test.py --mode=release -J \
		addons doctool inspector known_issues message pseudo-tty parallel sequential
=== release test-repl-tab-complete ===
Path: parallel/test-repl-tab-complete
assert.js:42
function fail(actual,expected,message,operator,stackStartFunction){++cov_1bytdz4cpa.f[4];++cov_1bytdz4cpa.s[21];throw new assert.AssertionError({message:message,actual:actual,expected:expected,operator:operator,stackStartFunction:stackStartFunction});}// EXTENSION! allows for well behaved errors defined elsewhere.
                                                                                                                ^
AssertionError: [ [ 'co' ], 'co' ] deepStrictEqual [ [ 'con' ], 'co' ]
    at editor.completer.common.mustCall (/Users/joyee/projects/node/test/parallel/test-repl-tab-complete.js:376:10)
    at /Users/joyee/projects/node/test/common.js:453:15
    at repl.js:158:1428
    at completionGroupsLoaded (repl.js:157:245)
    at REPLServer.complete (repl.js:140:525)
    at REPLServer.completer (repl.js:71:241)
    at Object. (/Users/joyee/projects/node/test/parallel/test-repl-tab-complete.js:375:8)
    at Module._compile (module.js:98:802)
    at Object.Module._extensions..js (module.js:99:198)
    at Module.load (module.js:81:659)
Command: out/Release/node /Users/joyee/projects/node/test/parallel/test-repl-tab-complete.js
[02:58|%  95|+ 1336|-   1]: release core_line_numbers match failed
line=0
expect=^punycode\.js\:42$
actual=punycode.js:11
=== release core_line_numbers ===
Path: message/core_line_numbers
punycode.js:11
 */function error(type){++cov_2lb1jwotzl.f[0];++cov_2lb1jwotzl.s[16];throw new RangeError(errors[type]);}/**
                                                                     ^
RangeError: Invalid input
    at error (punycode.js:11:76)
    at Object.decode (punycode.js:95:202)
    at Object. (/Users/joyee/projects/node/test/message/core_line_numbers.js:11:10)
    at Module._compile (module.js:98:802)
    at Object.Module._extensions..js (module.js:99:198)
    at Module.load (module.js:81:659)
    at tryModuleLoad (module.js:79:1369)
    at Function.Module._load (module.js:79:1132)
    at Module.runMain (module.js:103:8)
    at run (bootstrap_node.js:75:1)
Command: out/Release/node /Users/joyee/projects/node/test/message/core_line_numbers.js
[02:58|%  95|+ 1336|-   2]: release error_exit match failed
line=2
expect=^\ \ throw\ new\ assert\.AssertionError\(\{$
actual=function fail(actual,expected,message,operator,stackStartFunction){++cov_1bytdz4cpa.f[4];++cov_1bytdz4cpa.s[21];throw new assert.AssertionError({message:message,actual:actual,expected:expected,operator:operator,stackStartFunction:stackStartFunction});}// EXTENSION! allows for well behaved errors defined elsewhere.
=== release error_exit ===
Path: message/error_exit
Exiting with code=1

assert.js:42
function fail(actual,expected,message,operator,stackStartFunction){++cov_1bytdz4cpa.f[4];++cov_1bytdz4cpa.s[21];throw new assert.AssertionError({message:message,actual:actual,expected:expected,operator:operator,stackStartFunction:stackStartFunction});}// EXTENSION! allows for well behaved errors defined elsewhere.
^
AssertionError: 1 === 2
at Object. (/Users/joyee/projects/node/test/message/error_exit.js:9:8)
at Module._compile (module.js:98:802)
at Object.Module._extensions..js (module.js:99:198)
at Module.load (module.js:81:659)
at tryModuleLoad (module.js:79:1369)
at Function.Module._load (module.js:79:1132)
at Module.runMain (module.js:103:8)
at run (bootstrap_node.js:75:1)
at startup (bootstrap_node.js:34:215)
at bootstrap_node.js:78:3947
Command: out/Release/node /Users/joyee/projects/node/test/message/error_exit.js

@Fishrock123
Copy link
Contributor

Fishrock123 commented Feb 23, 2017

Yeah I think some of the /message/ tests and one repl test always fail under it even on linux.

@mhdawson
Copy link
Member Author

@CurryKitten is prioritizing making it work on OS X so we might want to tie the guide update to that.

@addaleax in terms of pulling in the other components into tools, did check if the licenses for those components are compatible with pulling in ?

@mhdawson
Copy link
Member Author

@joyeecheung there are failing tests when run under coverage and we allows them to run anyway as excluding reduces the test coverage. You can validate if the specific ones are expected by comparing to the failures in the nightly runs: https://ci.nodejs.org/job/node-test-commit-linux-coverage/172/

@addaleax
Copy link
Member

@addaleax in terms of pulling in the other components into tools, did check if the licenses for those components are compatible with pulling in ?

I was mainly thinking about things like https://github.com/nodejs/testing/blob/master/coverage/generate-index-html.py. Otherwise, no, I haven’t really done that.

@Trott
Copy link
Member

Trott commented Jul 26, 2017

Should this be closed? Split up into narrower individual issues? Left alone?

@Trott
Copy link
Member

Trott commented Oct 9, 2018

Closing due to inactivity, but feel free to re-open if you think that's the wrong move!

@Trott Trott closed this as completed Oct 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

No branches or pull requests

6 participants