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

coverage.nodejs.org misreporting #19912

Closed
chrismilleruk opened this issue Apr 10, 2018 · 12 comments
Closed

coverage.nodejs.org misreporting #19912

chrismilleruk opened this issue Apr 10, 2018 · 12 comments
Labels
build Issues and PRs related to build files or the CI.

Comments

@chrismilleruk
Copy link
Contributor

  • Version: v10.0.0-pre
  • Platform: tbc
  • Subsystem: test, coverage

https://coverage.nodejs.org/ is reporting a significant drop in code coverage in the most recent nightly.

JS Coverage: 94.42% -> 28.80%
C++ Coverage: 92.50% -> 31.10%

Note that my local coverage results seem OK so this looks like a build / job failure.

screenshot 2018-04-10 11 06 45

@addaleax addaleax added the build Issues and PRs related to build files or the CI. label Apr 10, 2018
@addaleax
Copy link
Member

/cc @nodejs/build @mhdawson

@rvagg
Copy link
Member

rvagg commented Apr 10, 2018

9th: https://ci.nodejs.org/job/node-test-commit-linux-coverage/600/nodes=benchmark/console
10th: https://ci.nodejs.org/job/node-test-commit-linux-coverage/601/nodes=benchmark/console

Does the output help? it has %'s that match what we're seeing on the table.

Maybe we just added a ton of code yesterday and didn't catch up with enough new tests ...

@chrismilleruk
Copy link
Contributor Author

chrismilleruk commented Apr 10, 2018

Those look really useful, thanks.

I had found these:
9th: https://ci.nodejs.org/job/node-test-commit-linux-coverage/600/
10th: https://ci.nodejs.org/job/node-test-commit-linux-coverage/601/
... but yours are better.

I noticed from the job status pages there that job 601 took just 16 mins instead of the usual ~31 mins.

@chrismilleruk
Copy link
Contributor Author

I reviewed the diff of the two outputs. Aside from the obvious size difference (601 is a shorter log), there are fewer files covered and the coverage for each file is generally worse.

It seems to point towards some missing files (both from the test suite and the source files).

Is it possible to re-run the job on the CI server?

@rvagg
Copy link
Member

rvagg commented Apr 11, 2018

maybe we just wait for today's run and see how it goes, perhaps we should delete yesterday's record if it's a simple anomaly

@rvagg
Copy link
Member

rvagg commented Apr 11, 2018

Today's run has the same numbers as yesterdays .. something's up with the codebase I think.
I just checked on the machine and the last login was Apr 3 so I don't think this is about a software update or anything on the system.

@chrismilleruk
Copy link
Contributor Author

Trying to repro the issue locally is proving difficult. I'm not familiar with the codebase and build setup so any pointers appreciated. I found some commands in the build repo but I can't confirm that these are the same as used by node-test-commit-linux-coverage job.
https://github.com/nodejs/build/tree/master/jenkins/scripts/coverage#coverage-job

./configure --coverage
make coverage-clean
NODE_TEST_DIR=${HOME}/node-tmp PYTHON=python COVTESTS=test-ci make coverage -j $(getconf _NPROCESSORS_ONLN)

The above fails locally (macOS 10.13.4) with the following error:
error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: unknown option character `-' in: --coverage

@chrismilleruk
Copy link
Contributor Author

These are the commits between the last known good and the first known failure:
244af7a...d1156da

The OpenSSL changes look like a sensible suspect, there were some CI failures that have already been reviewed and discounted #19794 (comment) but it might be worth a second look?

@Trott
Copy link
Member

Trott commented Apr 11, 2018

@nodejs/testing

@addaleax
Copy link
Member

Looking at the console outputs, it seems like the issue is that make test seems to fail during/after addon compilation. This is the original error message from https://ci.nodejs.org/job/node-test-commit-linux-coverage/601/nodes=benchmark/consoleFull:

npm ERR! weird error structured-stack:1
npm ERR! weird error (function (){cov_12264jx1gp.f[26]++;cov_12264jx1gp.s[202]++;Error.prepareStackTrace=function(err,trace){cov_12264jx1gp.f[27]++;cov_12264jx1gp.s[203]++;err.stack=trace;};cov_12264jx1gp.s[204]++;Error.stackTraceLimit=Infinity;cov_12264jx1gp.s[205]++;return function structuredStack(){cov_12264jx1gp.f[28]++;cov_12264jx1gp.s[206]++;// eslint-disable-next-line no-restricted-syntax
npm ERR! weird error              ^
npm ERR! weird error 
npm ERR! weird error ReferenceError: cov_12264jx1gp is not defined
npm ERR! weird error     at structured-stack:1:14
npm ERR! weird error     at structured-stack:2:30
npm ERR! weird error     at Script.runInContext (vm.js:24:1196)
npm ERR! weird error     at Script.runInNewContext (vm.js:24:1416)
npm ERR! weird error     at runInNewContext (vm.js:28:1085)
npm ERR! weird error     at isInsideNodeModules (internal/util.js:36:44)
npm ERR! weird error     at showFlaggedDeprecation (buffer.js:26:744)
npm ERR! weird error     at new Buffer (buffer.js:41:94)
npm ERR! weird error     at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-coverage/nodes/benchmark/deps/npm/node_modules/mississippi/node_modules/duplexify/index.js:6:20)
npm ERR! weird error     at Module._compile (internal/modules/cjs/loader.js:109:892)

@addaleax
Copy link
Member

(i.e. this points to #19524 being at fault for this)

addaleax added a commit to addaleax/node that referenced this issue Apr 14, 2018
Taking the source code of a function and running it in another
context does not play well with coverage instrumentation.
For now, do the simple thing and just write the source code as
a string literal.

Fixes: nodejs#19912
Refs: nodejs#19524
@addaleax
Copy link
Member

Aaand https://coverage.nodejs.org/ is back to normal! 🎉

jasnell pushed a commit that referenced this issue Apr 16, 2018
Taking the source code of a function and running it in another
context does not play well with coverage instrumentation.
For now, do the simple thing and just write the source code as
a string literal.

Fixes: #19912
Refs: #19524

PR-URL: #20035
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
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

Successfully merging a pull request may close this issue.

4 participants