inspector: expose precise coverage start to JS runtime · nodejs/node@8c989ec · GitHub
Skip to content

Commit 8c989ec

Browse files
Han5991sxa
Han5991
authored and
sxa
committed
inspector: expose precise coverage start to JS runtime
Add a `startCoverage` method on the `profiler` internal binding so that V8 precise coverage can be enabled after bootstrap. The method is idempotent against the existing bootstrap path (which creates a V8CoverageConnection when NODE_V8_COVERAGE or --experimental-test-coverage is set) and a no-op when the inspector is unavailable, e.g. in the parent process of `--test --test-isolation=process` where workers handle coverage and Environment::should_create_inspector() returns false. Refs: #60023 Signed-off-by: sangwook <rewq5991@gmail.com> PR-URL: #63079 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: Aviv Keller <me@aviv.sh>
1 parent 19e8ce1 commit 8c989ec

8 files changed

Lines changed: 150 additions & 1 deletion

File tree

‎lib/internal/test_runner/coverage.js‎

Lines changed: 2 additions & 0 deletions

‎lib/internal/test_runner/runner.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -839,6 +839,8 @@ function run(options = kEmptyObject) {
839839
coverageExcludeGlobs = [coverageExcludeGlobs];
840840
}
841841
validateStringArray(coverageExcludeGlobs, 'options.coverageExcludeGlobs');
842+
} else if (coverage) {
843+
coverageExcludeGlobs = [kDefaultPattern];
842844
}
843845
if (coverageIncludeGlobs != null) {
844846
if (!ArrayIsArray(coverageIncludeGlobs)) {

‎src/inspector_profiler.cc‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,30 @@ static void SetSourceMapCacheGetter(const FunctionCallbackInfo<Value>& args) {
548548
env->set_source_map_cache_getter(args[0].As<Function>());
549549
}
550550

551+
static void StartCoverage(const FunctionCallbackInfo<Value>& args) {
552+
Environment* env = Environment::GetCurrent(args);
553+
554+
Debug(env,
555+
DebugCategory::INSPECTOR_PROFILER,
556+
"StartCoverage, connection %s nullptr\n",
557+
env->coverage_connection() == nullptr ? "==" : "!=");
558+
559+
if (env->coverage_connection() != nullptr) {
560+
return;
561+
}
562+
563+
// The parent of `--test --test-isolation=process` intentionally has no
564+
// inspector (see Environment::should_create_inspector); workers handle
565+
// coverage themselves. Without an inspector, V8CoverageConnection would
566+
// get a null session and crash on the first DispatchMessage.
567+
if (!env->should_create_inspector()) {
568+
return;
569+
}
570+
571+
env->set_coverage_connection(std::make_unique<V8CoverageConnection>(env));
572+
env->coverage_connection()->Start();
573+
}
574+
551575
static void TakeCoverage(const FunctionCallbackInfo<Value>& args) {
552576
Environment* env = Environment::GetCurrent(args);
553577
V8CoverageConnection* connection = env->coverage_connection();
@@ -601,6 +625,7 @@ static void Initialize(Local<Object> target,
601625
SetMethod(context, target, "setCoverageDirectory", SetCoverageDirectory);
602626
SetMethod(
603627
context, target, "setSourceMapCacheGetter", SetSourceMapCacheGetter);
628+
SetMethod(context, target, "startCoverage", StartCoverage);
604629
SetMethod(context, target, "takeCoverage", TakeCoverage);
605630
SetMethod(context, target, "stopCoverage", StopCoverage);
606631
SetMethod(context, target, "endCoverage", EndCoverage);
@@ -609,6 +634,7 @@ static void Initialize(Local<Object> target,
609634
void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
610635
registry->Register(SetCoverageDirectory);
611636
registry->Register(SetSourceMapCacheGetter);
637+
registry->Register(StartCoverage);
612638
registry->Register(TakeCoverage);
613639
registry->Register(StopCoverage);
614640
registry->Register(EndCoverage);

‎test/fixtures/test-runner/coverage-isolation-none/runner.mjs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { run } from 'node:test';
2+
import { join } from 'node:path';
3+
4+
const stream = run({
5+
files: [join(import.meta.dirname, 'tests', 'foo.test.mjs')],
6+
coverage: true,
7+
isolation: 'none',
8+
cwd: import.meta.dirname,
9+
});
10+
stream.on('test:fail', () => process.exit(10));
11+
let summary;
12+
stream.on('test:coverage', (event) => { summary = event.summary; });
13+
for await (const _ of stream);
14+
if (!summary || summary.files.length === 0) process.exit(11);
15+
const hasSrc = summary.files.some((f) => f.path.endsWith('foo.mjs') && !f.path.endsWith('foo.test.mjs'));
16+
const hasTest = summary.files.some((f) => f.path.endsWith('foo.test.mjs'));
17+
if (!hasSrc) process.exit(12);
18+
if (hasTest) process.exit(13);

‎test/fixtures/test-runner/coverage-isolation-none/src/foo.mjs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
export function add(a, b) {
2+
return a + b;
3+
}
4+
5+
export function sub(a, b) {
6+
return a - b;
7+
}
8+
9+
export function unused() {
10+
return 'unused';
11+
}

‎test/fixtures/test-runner/coverage-isolation-none/tests/foo.test.mjs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import test from 'node:test';
2+
import assert from 'node:assert';
3+
import { add, sub } from '../src/foo.mjs';
4+
5+
test('add', () => {
6+
assert.strictEqual(add(2, 3), 5);
7+
});
8+
9+
test('sub', () => {
10+
assert.strictEqual(sub(5, 3), 2);
11+
});

‎test/parallel/test-runner-coverage-isolation-none-api.mjs‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import * as common from '../common/index.mjs';
2+
import { before, describe, it, run } from 'node:test';
3+
import assert from 'node:assert';
4+
import { spawnSync } from 'node:child_process';
5+
import { cp } from 'node:fs/promises';
6+
import { join, sep } from 'node:path';
7+
import tmpdir from '../common/tmpdir.js';
8+
import fixtures from '../common/fixtures.js';
9+
10+
const skipIfNoInspector = {
11+
skip: !process.features.inspector ? 'inspector disabled' : false,
12+
};
13+
14+
tmpdir.refresh();
15+
16+
async function setupFixtures() {
17+
const fixtureDir = fixtures.path('test-runner', 'coverage-isolation-none');
18+
await cp(fixtureDir, tmpdir.path, { recursive: true });
19+
}
20+
21+
describe('run() coverage with isolation: none', skipIfNoInspector, () => {
22+
before(async () => {
23+
await setupFixtures();
24+
});
25+
26+
for (const isolation of ['none', 'process']) {
27+
it(`reports src coverage and excludes test files by default (isolation=${isolation})`, async () => {
28+
const stream = run({
29+
files: [join(tmpdir.path, 'tests', 'foo.test.mjs')],
30+
coverage: true,
31+
isolation,
32+
cwd: tmpdir.path,
33+
});
34+
stream.on('test:fail', common.mustNotCall());
35+
36+
let summary;
37+
stream.on('test:coverage', common.mustCall(({ summary: s }) => {
38+
summary = s;
39+
}));
40+
// eslint-disable-next-line no-unused-vars
41+
for await (const _ of stream);
42+
43+
assert.ok(summary, 'test:coverage event must fire');
44+
const paths = summary.files.map((f) => f.path);
45+
assert.ok(
46+
paths.length > 0,
47+
`coverage files must be reported (isolation=${isolation}); got ${JSON.stringify(paths)}`,
48+
);
49+
assert.ok(
50+
paths.some((p) => p.endsWith(`src${sep}foo.mjs`)),
51+
`expected src/foo.mjs to be present (isolation=${isolation}); got ${JSON.stringify(paths)}`,
52+
);
53+
assert.ok(
54+
paths.every((p) => !p.endsWith('foo.test.mjs')),
55+
`expected foo.test.mjs to be excluded by default (isolation=${isolation}); got ${JSON.stringify(paths)}`,
56+
);
57+
});
58+
}
59+
60+
it('is idempotent when --experimental-test-coverage is also passed', async () => {
61+
const result = spawnSync(process.execPath, [
62+
'--experimental-test-coverage',
63+
join(tmpdir.path, 'runner.mjs'),
64+
], { cwd: tmpdir.path });
65+
assert.strictEqual(
66+
result.status,
67+
0,
68+
`exited with ${result.status}\nstderr: ${result.stderr}\nstdout: ${result.stdout}`,
69+
);
70+
});
71+
});

‎test/parallel/test-runner-run-coverage.mjs‎

Lines changed: 9 additions & 1 deletion

0 commit comments

Comments
 (0)