From d32f769d75dc31773ad34b5c28ddefd2dbd84a57 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 30 Dec 2018 19:44:13 -0500 Subject: [PATCH 1/3] coverage: use process._rawDebug() during setup console is not ready to use at this point in the bootstrapping process, so switch to process._rawDebug() instead. PR-URL: https://github.com/nodejs/node/pull/25289 Fixes: https://github.com/nodejs/node/issues/25287 Reviewed-By: Anna Henningsen Reviewed-By: Gus Caplan Reviewed-By: Yuta Hiroto Reviewed-By: Joyee Cheung Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- lib/internal/process/coverage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/process/coverage.js b/lib/internal/process/coverage.js index 5c5d0c2b6171f3..c84181db933334 100644 --- a/lib/internal/process/coverage.js +++ b/lib/internal/process/coverage.js @@ -53,7 +53,7 @@ exports.writeCoverage = writeCoverage; function setup() { const { Connection } = internalBinding('inspector'); if (!Connection) { - console.warn('inspector not enabled'); + process._rawDebug('inspector not enabled'); return; } @@ -80,7 +80,7 @@ function setup() { coverageDirectory = process.env.NODE_V8_COVERAGE = resolve(process.env.NODE_V8_COVERAGE); } catch (err) { - console.error(err); + process._rawDebug(err.toString()); } } From a779ee4fad71b22a35ff4fd8176e36f98ac2e684 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 30 Dec 2018 19:47:47 -0500 Subject: [PATCH 2/3] coverage: pass cwd to path.resolve() in setup During coverage setup, path.resolve() is called. path.resolve() can potentially call process.cwd(), which hasn't been bootstrapped yet. This commit passes the current working directory directly so that path.resolve() doesn't attempt to compute it. PR-URL: https://github.com/nodejs/node/pull/25289 Fixes: https://github.com/nodejs/node/issues/25287 Reviewed-By: Anna Henningsen Reviewed-By: Gus Caplan Reviewed-By: Yuta Hiroto Reviewed-By: Joyee Cheung Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- lib/internal/process/coverage.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/process/coverage.js b/lib/internal/process/coverage.js index c84181db933334..ef37deba8a5f58 100644 --- a/lib/internal/process/coverage.js +++ b/lib/internal/process/coverage.js @@ -76,9 +76,10 @@ function setup() { })); try { + const { cwd } = internalBinding('process_methods'); const { resolve } = require('path'); coverageDirectory = process.env.NODE_V8_COVERAGE = - resolve(process.env.NODE_V8_COVERAGE); + resolve(cwd(), process.env.NODE_V8_COVERAGE); } catch (err) { process._rawDebug(err.toString()); } From 8881a5ac416118ce56413d7ce4c9bc75238a31ec Mon Sep 17 00:00:00 2001 From: cjihrig Date: Mon, 31 Dec 2018 11:10:01 -0500 Subject: [PATCH 3/3] test: make test-v8-coverage.js more strict Update the coverage test to verify that nothing is printed to stderr (which happens when coverage errors happen). Also add a test case to verify that non-absolute coverage paths work. PR-URL: https://github.com/nodejs/node/pull/25289 Fixes: https://github.com/nodejs/node/issues/25287 Reviewed-By: Anna Henningsen Reviewed-By: Gus Caplan Reviewed-By: Yuta Hiroto Reviewed-By: Joyee Cheung Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- test/parallel/test-v8-coverage.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/parallel/test-v8-coverage.js b/test/parallel/test-v8-coverage.js index 75a6aa03fd8761..e1520a04115e42 100644 --- a/test/parallel/test-v8-coverage.js +++ b/test/parallel/test-v8-coverage.js @@ -23,6 +23,7 @@ function nextdir() { require.resolve('../fixtures/v8-coverage/basic') ], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } }); assert.strictEqual(output.status, 0); + assert.strictEqual(output.stderr.toString(), ''); const fixtureCoverage = getFixtureCoverage('basic.js', coverageDirectory); assert.ok(fixtureCoverage); // first branch executed. @@ -38,6 +39,7 @@ function nextdir() { require.resolve('../fixtures/v8-coverage/exit-1') ], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } }); assert.strictEqual(output.status, 1); + assert.strictEqual(output.stderr.toString(), ''); const fixtureCoverage = getFixtureCoverage('exit-1.js', coverageDirectory); assert.ok(fixtureCoverage, 'coverage not found for file'); // first branch executed. @@ -55,6 +57,7 @@ function nextdir() { if (!common.isWindows) { assert.strictEqual(output.signal, 'SIGINT'); } + assert.strictEqual(output.stderr.toString(), ''); const fixtureCoverage = getFixtureCoverage('sigint.js', coverageDirectory); assert.ok(fixtureCoverage); // first branch executed. @@ -70,6 +73,7 @@ function nextdir() { require.resolve('../fixtures/v8-coverage/spawn-subprocess') ], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } }); assert.strictEqual(output.status, 0); + assert.strictEqual(output.stderr.toString(), ''); const fixtureCoverage = getFixtureCoverage('subprocess.js', coverageDirectory); assert.ok(fixtureCoverage); @@ -87,6 +91,7 @@ function nextdir() { require.resolve('../fixtures/v8-coverage/worker') ], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } }); assert.strictEqual(output.status, 0); + assert.strictEqual(output.stderr.toString(), ''); const fixtureCoverage = getFixtureCoverage('subprocess.js', coverageDirectory); assert.ok(fixtureCoverage); @@ -103,6 +108,7 @@ function nextdir() { require.resolve('../fixtures/v8-coverage/spawn-subprocess-no-cov') ], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } }); assert.strictEqual(output.status, 0); + assert.strictEqual(output.stderr.toString(), ''); const fixtureCoverage = getFixtureCoverage('subprocess.js', coverageDirectory); assert.strictEqual(fixtureCoverage, undefined); @@ -115,6 +121,7 @@ function nextdir() { require.resolve('../fixtures/v8-coverage/async-hooks') ], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } }); assert.strictEqual(output.status, 0); + assert.strictEqual(output.stderr.toString(), ''); const fixtureCoverage = getFixtureCoverage('async-hooks.js', coverageDirectory); assert.ok(fixtureCoverage); @@ -122,6 +129,27 @@ function nextdir() { assert.strictEqual(fixtureCoverage.functions[1].ranges[0].count, 1); } +// Outputs coverage when the coverage directory is not absolute. +{ + const coverageDirectory = nextdir(); + const absoluteCoverageDirectory = path.join(tmpdir.path, coverageDirectory); + const output = spawnSync(process.execPath, [ + require.resolve('../fixtures/v8-coverage/basic') + ], { + cwd: tmpdir.path, + env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } + }); + assert.strictEqual(output.status, 0); + assert.strictEqual(output.stderr.toString(), ''); + const fixtureCoverage = getFixtureCoverage('basic.js', + absoluteCoverageDirectory); + assert.ok(fixtureCoverage); + // first branch executed. + assert.strictEqual(fixtureCoverage.functions[1].ranges[0].count, 1); + // second branch did not execute. + assert.strictEqual(fixtureCoverage.functions[1].ranges[1].count, 0); +} + // Extracts the coverage object for a given fixture name. function getFixtureCoverage(fixtureFile, coverageDirectory) { const coverageFiles = fs.readdirSync(coverageDirectory);