From 7ee0cea0283b6e18d81e509d5ed809647fb6f09c Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sat, 3 Nov 2018 15:42:13 -0700 Subject: [PATCH] lib: make coverage work for Node.js PR-URL: https://github.com/nodejs/node/pull/23941 Reviewed-By: James M Snell Reviewed-By: Yang Guo --- lib/internal/bootstrap/loaders.js | 6 ++ lib/internal/bootstrap/node.js | 4 +- lib/internal/process/coverage.js | 90 +++++++++++++++-------- test/fixtures/v8-coverage/async-hooks.js | 11 +++ test/parallel/test-heapdump-inspector.js | 47 ++++++++---- test/parallel/test-v8-coverage.js | 14 ++++ test/sequential/test-inspector-enabled.js | 5 +- 7 files changed, 129 insertions(+), 48 deletions(-) create mode 100644 test/fixtures/v8-coverage/async-hooks.js diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index dff6ce661a4640..aceb15cc63fc69 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -362,6 +362,12 @@ NativeModule._cache[this.id] = this; }; + // coverage must be turned on early, so that we can collect + // it for Node.js' own internal libraries. + if (process.env.NODE_V8_COVERAGE) { + NativeModule.require('internal/process/coverage').setup(); + } + // This will be passed to the bootstrapNodeJSCore function in // bootstrap/node.js. return loaderExports; diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 1ba427a572e3f7..6b5f1f7f8e561c 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -103,9 +103,7 @@ NativeModule.require('internal/process/write-coverage').setup(); if (process.env.NODE_V8_COVERAGE) { - const { resolve } = NativeModule.require('path'); - process.env.NODE_V8_COVERAGE = resolve(process.env.NODE_V8_COVERAGE); - NativeModule.require('internal/process/coverage').setup(); + NativeModule.require('internal/process/coverage').setupExitHooks(); } if (process.config.variables.v8_enable_inspector) { diff --git a/lib/internal/process/coverage.js b/lib/internal/process/coverage.js index df45285baaee02..9c9b2b47119ea7 100644 --- a/lib/internal/process/coverage.js +++ b/lib/internal/process/coverage.js @@ -1,23 +1,19 @@ 'use strict'; -const path = require('path'); -const { mkdirSync, writeFileSync } = require('fs'); -const hasInspector = process.config.variables.v8_enable_inspector === 1; -let inspector = null; -if (hasInspector) inspector = require('inspector'); - -let session; +let coverageConnection = null; +let coverageDirectory; function writeCoverage() { - if (!session) { + if (!coverageConnection && coverageDirectory) { return; } + const { join } = require('path'); + const { mkdirSync, writeFileSync } = require('fs'); const { threadId } = require('internal/worker'); const filename = `coverage-${process.pid}-${Date.now()}-${threadId}.json`; try { - // TODO(bcoe): switch to mkdirp once #22302 is addressed. - mkdirSync(process.env.NODE_V8_COVERAGE); + mkdirSync(coverageDirectory, { recursive: true }); } catch (err) { if (err.code !== 'EEXIST') { console.error(err); @@ -25,41 +21,73 @@ function writeCoverage() { } } - const target = path.join(process.env.NODE_V8_COVERAGE, filename); - + const target = join(coverageDirectory, filename); try { - session.post('Profiler.takePreciseCoverage', (err, coverageInfo) => { - if (err) return console.error(err); - try { - writeFileSync(target, JSON.stringify(coverageInfo)); - } catch (err) { - console.error(err); - } - }); + disableAllAsyncHooks(); + let msg; + coverageConnection._coverageCallback = function(_msg) { + msg = _msg; + }; + coverageConnection.dispatch(JSON.stringify({ + id: 3, + method: 'Profiler.takePreciseCoverage' + })); + const coverageInfo = JSON.parse(msg).result; + writeFileSync(target, JSON.stringify(coverageInfo)); } catch (err) { console.error(err); } finally { - session.disconnect(); - session = null; + coverageConnection.disconnect(); + coverageConnection = null; } } +function disableAllAsyncHooks() { + const { getHookArrays } = require('internal/async_hooks'); + const [hooks_array] = getHookArrays(); + hooks_array.forEach((hook) => { hook.disable(); }); +} + exports.writeCoverage = writeCoverage; function setup() { - if (!hasInspector) { - console.warn('coverage currently only supported in main thread'); + const { Connection } = process.binding('inspector'); + if (!Connection) { + console.warn('inspector not enabled'); return; } - session = new inspector.Session(); - session.connect(); - session.post('Profiler.enable'); - session.post('Profiler.startPreciseCoverage', { callCount: true, - detailed: true }); + coverageConnection = new Connection((res) => { + if (coverageConnection._coverageCallback) { + coverageConnection._coverageCallback(res); + } + }); + coverageConnection.dispatch(JSON.stringify({ + id: 1, + method: 'Profiler.enable' + })); + coverageConnection.dispatch(JSON.stringify({ + id: 2, + method: 'Profiler.startPreciseCoverage', + params: { + callCount: true, + detailed: true + } + })); - const reallyReallyExit = process.reallyExit; + try { + const { resolve } = require('path'); + coverageDirectory = process.env.NODE_V8_COVERAGE = + resolve(process.env.NODE_V8_COVERAGE); + } catch (err) { + console.error(err); + } +} +exports.setup = setup; + +function setupExitHooks() { + const reallyReallyExit = process.reallyExit; process.reallyExit = function(code) { writeCoverage(); reallyReallyExit(code); @@ -68,4 +96,4 @@ function setup() { process.on('exit', writeCoverage); } -exports.setup = setup; +exports.setupExitHooks = setupExitHooks; diff --git a/test/fixtures/v8-coverage/async-hooks.js b/test/fixtures/v8-coverage/async-hooks.js new file mode 100644 index 00000000000000..855f41ed3b3449 --- /dev/null +++ b/test/fixtures/v8-coverage/async-hooks.js @@ -0,0 +1,11 @@ +const async_hooks = require('async_hooks'); +const common = require('../../common'); + +const hook = async_hooks.createHook({ + init: common.mustNotCall(), + before: common.mustNotCall(), + after: common.mustNotCall(), + destroy: common.mustNotCall() +}); + +hook.enable(); diff --git a/test/parallel/test-heapdump-inspector.js b/test/parallel/test-heapdump-inspector.js index 3d031d87eb1a99..963b85ac3a029a 100644 --- a/test/parallel/test-heapdump-inspector.js +++ b/test/parallel/test-heapdump-inspector.js @@ -7,17 +7,38 @@ common.skipIfInspectorDisabled(); const { validateSnapshotNodes } = require('../common/heap'); const inspector = require('inspector'); -const session = new inspector.Session(); -validateSnapshotNodes('Node / JSBindingsConnection', []); -session.connect(); -validateSnapshotNodes('Node / JSBindingsConnection', [ - { - children: [ - { node_name: 'Node / InspectorSession', edge_name: 'session' }, - { node_name: 'Connection', edge_name: 'wrapped' }, - (edge) => edge.name === 'callback' && - (edge.to.type === undefined || // embedded graph - edge.to.type === 'closure') // snapshot - ] +const snapshotNode = { + children: [ + { node_name: 'Node / InspectorSession', edge_name: 'session' } + ] +}; + +// starts with no JSBindingsConnection (or 1 if coverage enabled). +{ + const expected = []; + if (process.env.NODE_V8_COVERAGE) { + expected.push(snapshotNode); + } + validateSnapshotNodes('Node / JSBindingsConnection', expected); +} + +// JSBindingsConnection should be added. +{ + const session = new inspector.Session(); + session.connect(); + const expected = [ + { + children: [ + { node_name: 'Node / InspectorSession', edge_name: 'session' }, + { node_name: 'Connection', edge_name: 'wrapped' }, + (edge) => edge.name === 'callback' && + (edge.to.type === undefined || // embedded graph + edge.to.type === 'closure') // snapshot + ] + } + ]; + if (process.env.NODE_V8_COVERAGE) { + expected.push(snapshotNode); } -]); + validateSnapshotNodes('Node / JSBindingsConnection', expected); +} diff --git a/test/parallel/test-v8-coverage.js b/test/parallel/test-v8-coverage.js index 16a55b0fc4b8e8..191835fe0f5af9 100644 --- a/test/parallel/test-v8-coverage.js +++ b/test/parallel/test-v8-coverage.js @@ -108,6 +108,20 @@ function nextdir() { assert.strictEqual(fixtureCoverage, undefined); } +// disables async hooks before writing coverage. +{ + const coverageDirectory = path.join(tmpdir.path, nextdir()); + const output = spawnSync(process.execPath, [ + require.resolve('../fixtures/v8-coverage/async-hooks') + ], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } }); + assert.strictEqual(output.status, 0); + const fixtureCoverage = getFixtureCoverage('async-hooks.js', + coverageDirectory); + assert.ok(fixtureCoverage); + // first branch executed. + assert.strictEqual(fixtureCoverage.functions[1].ranges[0].count, 1); +} + // extracts the coverage object for a given fixture name. function getFixtureCoverage(fixtureFile, coverageDirectory) { const coverageFiles = fs.readdirSync(coverageDirectory); diff --git a/test/sequential/test-inspector-enabled.js b/test/sequential/test-inspector-enabled.js index a7a0832793283c..f14e7c17d817c5 100644 --- a/test/sequential/test-inspector-enabled.js +++ b/test/sequential/test-inspector-enabled.js @@ -20,7 +20,10 @@ assert( `; const args = ['--inspect', '-e', script]; -const child = spawn(process.execPath, args, { stdio: 'inherit' }); +const child = spawn(process.execPath, args, { + stdio: 'inherit', + env: { ...process.env, NODE_V8_COVERAGE: '' } +}); child.on('exit', (code, signal) => { process.exit(code || signal); });