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

Build: Produce C/JS code coverage reports from make #10856

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 66 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ TEST_CI_ARGS ?=
STAGINGSERVER ?= node-www
LOGLEVEL ?= silent
OSTYPE := $(shell uname -s | tr '[A-Z]' '[a-z]')
COVTESTS ?= test

ifdef JOBS
PARALLEL_ARGS = -j $(JOBS)
Expand Down Expand Up @@ -113,6 +114,69 @@ distclean:

check: test

# Remove files generated by running coverage, put the non-instrumented lib back
# in place
coverage-clean:
if [ -d lib_ ]; then rm -rf lib; mv lib_ lib; fi
-rm -rf node_modules
-rm -rf gcovr testing
-rm -rf out/$(BUILDTYPE)/.coverage
-rm -rf .cov_tmp coverage
-rm -f out/$(BUILDTYPE)/obj.target/node/src/*.gcda
-rm -f out/$(BUILDTYPE)/obj.target/node/src/tracing/*.gcda
-rm -f out/$(BUILDTYPE)/obj.target/node/src/*.gcno
-rm -f out/$(BUILDTYPE)/obj.target/node/src/tracing*.gcno

# Build and test with code coverage reporting. Leave the lib directory
# instrumented for any additional runs the user may want to make.
# For C++ coverage reporting, this needs to be run in conjunction with configure
# --coverage. html coverage reports will be created under coverage/

coverage: coverage-test

coverage-build: all
mkdir -p node_modules
if [ ! -d node_modules/istanbul-merge ]; then \
$(NODE) ./deps/npm install istanbul-merge; fi
if [ ! -d node_modules/nyc ]; then $(NODE) ./deps/npm install nyc; fi
if [ ! -d gcovr ]; then git clone --depth=1 \
--single-branch git://github.com/gcovr/gcovr.git; fi
if [ ! -d testing ]; then git clone --depth=1 \
--single-branch https://github.com/nodejs/testing.git; fi
if [ ! -f gcovr/scripts/gcovr.orig ]; then \
(cd gcovr && patch -N -p1 < \
"$(CURDIR)/testing/coverage/gcovr-patches.diff"); fi
if [ -d lib_ ]; then rm -rf lib; mv lib_ lib; fi
mv lib lib_
$(NODE) ./node_modules/.bin/nyc instrument lib_/ lib/
$(MAKE)

coverage-test: coverage-build
-rm -rf out/$(BUILDTYPE)/.coverage
-rm -rf .cov_tmp
-rm -f out/$(BUILDTYPE)/obj.target/node/src/*.gcda
-rm -f out/$(BUILDTYPE)/obj.target/node/src/tracing/*.gcda
-$(MAKE) $(COVTESTS)
mv lib lib__
mv lib_ lib
mkdir -p coverage .cov_tmp
$(NODE) ./node_modules/.bin/istanbul-merge --out \
.cov_tmp/libcov.json 'out/Release/.coverage/coverage-*.json'
(cd lib && .$(NODE) ../node_modules/.bin/nyc report \
--temp-directory "$(CURDIR)/.cov_tmp" -r html \
--report-dir "../coverage")
-(cd out && "../gcovr/scripts/gcovr" --gcov-exclude='.*deps' \
--gcov-exclude='.*usr' -v -r Release/obj.target/node \
--html --html-detail -o ../coverage/cxxcoverage.html)
mv lib lib_
mv lib__ lib
@echo -n "Javascript coverage %: "
@grep -B1 Lines coverage/index.html | head -n1 \
| sed 's/<[^>]*>//g'| sed 's/ //g'
@echo -n "C++ coverage %: "
@grep -A3 Lines coverage/cxxcoverage.html | grep style \
| sed 's/<[^>]*>//g'| sed 's/ //g'

cctest: all
@out/$(BUILDTYPE)/$@

Expand Down Expand Up @@ -781,4 +845,5 @@ endif
bench-all bench bench-misc bench-array bench-buffer bench-net \
bench-http bench-fs bench-tls cctest run-ci test-v8 test-v8-intl \
test-v8-benchmarks test-v8-all v8 lint-ci bench-ci jslint-ci doc-only \
$(TARBALL)-headers test-ci test-ci-native test-ci-js build-ci clear-stalled
$(TARBALL)-headers test-ci test-ci-native test-ci-js build-ci clear-stalled \
coverage-clean coverage-build coverage-test coverage
2 changes: 2 additions & 0 deletions lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
NativeModule.require('internal/process/stdio').setup();
_process.setupKillAndExit();
_process.setupSignalHandlers();
if (global.__coverage__)
NativeModule.require('internal/process/write-coverage').setup();

// Do not initialize channel in debugger agent, it deletes env variable
// and the main thread won't see it.
Expand Down
46 changes: 46 additions & 0 deletions lib/internal/process/write-coverage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
'use strict';
const process = require('process');
const path = require('path');
const fs = require('fs');
const mkdirSync = fs.mkdirSync;
const writeFileSync = fs.writeFileSync;

var isWritingCoverage = false;
function writeCoverage() {
if (isWritingCoverage || !global.__coverage__) {
return;
}
isWritingCoverage = true;

const dirname = path.join(path.dirname(process.execPath), '.coverage');
const filename = `coverage-${process.pid}-${Date.now()}.json`;
try {
mkdirSync(dirname);
} catch (err) {
if (err.code !== 'EEXIST') {
console.error(err);
return;
}
}

const target = path.join(dirname, filename);
const coverageInfo = JSON.stringify(global.__coverage__);
try {
writeFileSync(target, coverageInfo);
} catch (err) {
console.error(err);
}
}

function setup() {
const reallyReallyExit = process.reallyExit;

process.reallyExit = function(code) {
writeCoverage();
reallyReallyExit(code);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, not 100% sold on this.

Is process.really a JS function? If so, can we put this hook into it directly somehow?

Also, could you point to a test that hits this but not the exit event?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

process.reallyExit is a C++ function.

Also, could you point to a test that hits this but not the exit event?

I put this in there to try to be really sure that writeCoverage() is called, especially since we have code that tests process.exit() itself… I can try to run coverage locally later without this, and see if there’s any difference


process.on('exit', writeCoverage);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth running the contents of setup() only conditionally, e.g. when global.__coverage__ is set (I think that test should work)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've put the check for global.__coverage__ into lib/internal/bootstrap_node.js so the require for internal/process/write-coverage isn't called unless coverage is being used


exports.setup = setup;
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
'lib/internal/process/warning.js',
'lib/internal/process.js',
'lib/internal/querystring.js',
'lib/internal/process/write-coverage.js',
'lib/internal/readline.js',
'lib/internal/repl.js',
'lib/internal/socket_list.js',
Expand Down
9 changes: 8 additions & 1 deletion test/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,9 @@ exports.platformTimeout = function(ms) {
if (process.config.target_defaults.default_configuration === 'Debug')
ms = 2 * ms;

if (global.__coverage__)
ms = 4 * ms;

if (exports.isAix)
return 2 * ms; // default localhost speed is slower on AIX

Expand Down Expand Up @@ -384,7 +387,11 @@ function leakedGlobals() {
if (!knownGlobals.includes(global[val]))
leaked.push(val);

return leaked;
if (global.__coverage__) {
return leaked.filter((varname) => !/^(cov_|__cov)/.test(varname));
} else {
return leaked;
}
}
exports.leakedGlobals = leakedGlobals;

Expand Down