Skip to content

Commit

Permalink
test: switch to native v8 coverage
Browse files Browse the repository at this point in the history
PR-URL: #25157
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
  • Loading branch information
bcoe committed Jan 20, 2019
1 parent fac11b0 commit d1dee49
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 89 deletions.
12 changes: 9 additions & 3 deletions BUILDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,15 @@ $ CI_JS_SUITES=child-process CI_NATIVE_SUITES= make coverage
The above command executes tests for the `child-process` subsystem and
outputs the resulting coverage report.

The `make coverage` command downloads some tools to the project root directory
and overwrites the `lib/` directory. To clean up after generating the coverage
reports:
Alternatively, for the JavaScript test suite, you can use the `CI_JS_SUITES`
variable to run tests in isolation, outputting reports:

```text
$ CI_JS_SUITES=fs CI_NATIVE_SUITES= make coverage-run-js

This comment has been minimized.

Copy link
@thefourtheye

thefourtheye Jan 21, 2019

Contributor

I thought we decided to include a note about CI_NATIVE_SUITES=

This comment has been minimized.

Copy link
@bcoe

bcoe Jan 21, 2019

Author Contributor

@thefourtheye that copy got lost in the shuffle of a bunch of other copy edits requested in the code review. What felt a bit weird, is we use the exact same approach directly above in the stanza:

CI_JS_SUITES=child-process CI_NATIVE_SUITES= make coverage

Maybe we could move this note up a bit in the text:

To collect coverage for a subset of tests you can set the CI_JS_SUITES and CI_NATIVE_SUITES variables:

to:

To collect coverage for a subset of tests you can set the CI_JS_SUITES and CI_NATIVE_SUITES variables (alternatively, you can unset either variable to run specific suites in isolation):

This comment has been minimized.

Copy link
@refack

refack Jan 21, 2019

Contributor

I believe it was I who asked for copy that is more consistent with the previous paragraph.
image
Sorry @thefourtheye for overriding you.

This comment has been minimized.

Copy link
@thefourtheye

thefourtheye Jan 22, 2019

Contributor

Ah, okay. No problem @refack. I didn't go through all the comments in the original PR. I like the suggestion by @bcoe. Perhaps we can get it done in a separate PR. I thought it would be helpful for the first time users who read the docs to understand that space after the = means we are skipping those tests

```

The `make coverage` command downloads some tools to the project root directory.
To clean up after generating the coverage reports:

```console
$ make coverage-clean
Expand Down
50 changes: 27 additions & 23 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ STAGINGSERVER ?= node-www
LOGLEVEL ?= silent
OSTYPE := $(shell uname -s | tr '[A-Z]' '[a-z]')
COVTESTS ?= test-cov
COV_SKIP_TESTS ?= core_line_numbers.js,testFinalizer.js,test_function/test.js
GTEST_FILTER ?= "*"
GNUMAKEFLAGS += --no-print-directory
GCOV ?= gcov
Expand Down Expand Up @@ -181,7 +182,6 @@ coverage-clean:
$(RM) -r node_modules
$(RM) -r gcovr build
$(RM) -r out/$(BUILDTYPE)/.coverage
$(RM) -r .cov_tmp
$(RM) out/$(BUILDTYPE)/obj.target/node/gen/*.gcda
$(RM) out/$(BUILDTYPE)/obj.target/node/src/*.gcda
$(RM) out/$(BUILDTYPE)/obj.target/node/src/tracing/*.gcda
Expand All @@ -201,55 +201,51 @@ coverage: coverage-test ## Run the tests and generate a coverage report.

.PHONY: coverage-build
coverage-build: all
mkdir -p node_modules
if [ ! -d node_modules/nyc ]; then \
$(NODE) ./deps/npm install nyc@13 --no-save --no-package-lock; fi
-$(MAKE) coverage-build-js
if [ ! -d gcovr ]; then git clone -b 3.4 --depth=1 \
--single-branch https://github.com/gcovr/gcovr.git; fi
if [ ! -d build ]; then git clone --depth=1 \
--single-branch https://github.com/nodejs/build.git; fi
if [ ! -f gcovr/scripts/gcovr.orig ]; then \
(cd gcovr && patch -N -p1 < \
"$(CURDIR)/build/jenkins/scripts/coverage/gcovr-patches-3.4.diff"); fi
if [ -d lib_ ]; then $(RM) -r lib; mv lib_ lib; fi
mv lib lib_
NODE_DEBUG=nyc $(NODE) ./node_modules/.bin/nyc instrument --extension .js \
--extension .mjs --exit-on-error lib_/ lib/
$(MAKE)

.PHONY: coverage-build-js
coverage-build-js:
mkdir -p node_modules
if [ ! -d node_modules/c8 ]; then \
$(NODE) ./deps/npm install c8@next --no-save --no-package-lock;\
fi

.PHONY: coverage-test
coverage-test: coverage-build
$(RM) -r out/$(BUILDTYPE)/.coverage
$(RM) -r .cov_tmp
$(RM) out/$(BUILDTYPE)/obj.target/node/gen/*.gcda
$(RM) out/$(BUILDTYPE)/obj.target/node/src/*.gcda
$(RM) out/$(BUILDTYPE)/obj.target/node/src/tracing/*.gcda
$(RM) out/$(BUILDTYPE)/obj.target/node_lib/gen/*.gcda
$(RM) out/$(BUILDTYPE)/obj.target/node_lib/src/*.gcda
$(RM) out/$(BUILDTYPE)/obj.target/node_lib/src/tracing/*.gcda
-$(MAKE) $(COVTESTS)
mv lib lib__
mv lib_ lib
mkdir -p coverage .cov_tmp
$(NODE) ./node_modules/.bin/nyc merge 'out/Release/.coverage' \
.cov_tmp/libcov.json
(cd lib && .$(NODE) ../node_modules/.bin/nyc report \
--temp-dir "$(CURDIR)/.cov_tmp" \
--report-dir "$(CURDIR)/coverage" \
--reporter html)
-NODE_V8_COVERAGE=out/$(BUILDTYPE)/.coverage $(MAKE) $(COVTESTS)
$(MAKE) coverage-report-js
-(cd out && "../gcovr/scripts/gcovr" --gcov-exclude='.*deps' \
--gcov-exclude='.*usr' -v -r Release/obj.target \
--html --html-detail -o ../coverage/cxxcoverage.html \
--gcov-executable="$(GCOV)")
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'

.PHONY: coverage-report-js
coverage-report-js:
$(NODE) ./node_modules/.bin/c8 report --reporter=html \
--temp-directory=out/$(BUILDTYPE)/.coverage --omit-relative=false \
--resolve=./lib --exclude="deps/" --exclude="test/" --exclude="tools/" \
--wrapper-length=0

.PHONY: cctest
# Runs the C++ tests using the built `cctest` executable.
cctest: all
Expand All @@ -276,6 +272,14 @@ jstest: build-addons build-js-native-api-tests build-node-api-tests ## Runs addo
$(CI_JS_SUITES) \
$(CI_NATIVE_SUITES)

.PHONY: coverage-run-js
coverage-run-js:
$(RM) -r out/$(BUILDTYPE)/.coverage
$(MAKE) coverage-build-js
-NODE_V8_COVERAGE=out/$(BUILDTYPE)/.coverage CI_SKIP_TESTS=$(COV_SKIP_TESTS) \
$(MAKE) jstest
$(MAKE) coverage-report-js

.PHONY: test
# This does not run tests of third-party libraries inside deps.
test: all ## Runs default tests, linters, and builds docs.
Expand All @@ -300,7 +304,7 @@ test-cov: all
$(MAKE) build-js-native-api-tests
$(MAKE) build-node-api-tests
# $(MAKE) cctest
CI_SKIP_TESTS=core_line_numbers.js $(MAKE) jstest
CI_SKIP_TESTS=$(COV_SKIP_TESTS) $(MAKE) jstest

test-parallel: all
$(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) parallel
Expand Down
18 changes: 1 addition & 17 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -318,26 +318,10 @@ function startup() {
}
});

// Set up coverage exit hooks.
let originalReallyExit = process.reallyExit;
// Core coverage generation using nyc instrumented lib/ files.
// See `make coverage-build`. This does not affect user land.
// TODO(joyeecheung): this and `with_instrumentation.js` can be
// removed in favor of NODE_V8_COVERAGE once we switch to that
// in https://coverage.nodejs.org/
if (global.__coverage__) {
const {
writeCoverage
} = NativeModule.require('internal/coverage-gen/with_instrumentation');
process.on('exit', writeCoverage);
originalReallyExit = process.reallyExit = (code) => {
writeCoverage();
originalReallyExit(code);
};
}
// User-facing NODE_V8_COVERAGE environment variable that writes
// ScriptCoverage to a specified file.
if (process.env.NODE_V8_COVERAGE) {
const originalReallyExit = process.reallyExit;
const cwd = NativeModule.require('internal/process/execution').tryGetCwd();
const { resolve } = NativeModule.require('path');
// Resolve the coverage directory to an absolute path, and
Expand Down
36 changes: 0 additions & 36 deletions lib/internal/coverage-gen/with_instrumentation.js

This file was deleted.

1 change: 0 additions & 1 deletion node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@
'lib/internal/console/global.js',
'lib/internal/console/inspector.js',
'lib/internal/coverage-gen/with_profiler.js',
'lib/internal/coverage-gen/with_instrumentation.js',
'lib/internal/crypto/certificate.js',
'lib/internal/crypto/cipher.js',
'lib/internal/crypto/diffiehellman.js',
Expand Down
9 changes: 1 addition & 8 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,6 @@ function platformTimeout(ms) {
if (process.features.debug)
ms = multipliers.two * ms;

if (global.__coverage__)
ms = multipliers.four * ms;

if (isAIX)
return multipliers.two * ms; // default localhost speed is slower on AIX

Expand Down Expand Up @@ -299,11 +296,7 @@ function leakedGlobals() {
}
}

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

process.on('exit', function() {
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-bootstrap-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ const common = require('../common');
const assert = require('assert');

const isMainThread = common.isMainThread;
const kMaxModuleCount = isMainThread ? 63 : 85;
const kCoverageModuleCount = process.env.NODE_V8_COVERAGE ? 1 : 0;
const kMaxModuleCount = (isMainThread ? 63 : 85) + kCoverageModuleCount;

assert(list.length <= kMaxModuleCount,
`Total length: ${list.length}\n` + list.join('\n')
Expand Down

0 comments on commit d1dee49

Please sign in to comment.