Skip to content

Commit bac6828

Browse files
committed
fix: address review feedback on statement coverage
- Replace hardcoded kStatementTypes array with acorn-walk's generic "Statement" category visitor, which automatically covers all current and future ESTree statement types (including ClassDeclaration, FunctionDeclaration, and StaticBlock that were previously missing). BlockStatement and EmptyStatement are excluded via a small deny-set. - Simplify AST parsing to a single pass using sourceType: 'script' with permissive flags (allowReturnOutsideFunction, allowImportExportEverywhere, allowAwaitOutsideFunction). Script mode is non-strict, so it handles both ESM and legacy CJS (e.g. `with` statements) without needing a module→script fallback. - Flatten V8 coverage ranges before the statement matching loop, reducing nesting from three levels to two. - Add coverage-class.js fixture and test for ClassDeclaration and StaticBlock statement coverage.
1 parent 05a5e91 commit bac6828

3 files changed

Lines changed: 105 additions & 45 deletions

File tree

lib/internal/test_runner/coverage.js

Lines changed: 38 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,11 @@ const { Parser: AcornParser } =
4343
const { simple: acornWalkSimple } =
4444
require('internal/deps/acorn/acorn-walk/dist/walk');
4545

46-
const kStatementTypes = [
47-
'ExpressionStatement', 'ReturnStatement', 'ThrowStatement',
48-
'IfStatement', 'WhileStatement', 'DoWhileStatement',
49-
'ForStatement', 'ForInStatement', 'ForOfStatement',
50-
'SwitchStatement', 'TryStatement', 'BreakStatement',
51-
'ContinueStatement', 'VariableDeclaration', 'LabeledStatement',
52-
'WithStatement', 'DebuggerStatement',
53-
];
46+
// Statement types excluded from coverage: containers (BlockStatement)
47+
// and empty statements that carry no executable semantics.
48+
const kExcludedStatementTypes = new SafeSet([
49+
'BlockStatement', 'EmptyStatement',
50+
]);
5451

5552
const kCoverageFileRegex = /^coverage-(\d+)-(\d{13})-(\d+)\.json$/;
5653
const kIgnoreRegex = /\/\* node:coverage ignore next (?<count>\d+ )?\*\//;
@@ -98,47 +95,37 @@ class TestCoverage {
9895

9996
const statements = [];
10097

101-
// Build a visitor with one handler per concrete statement type.
102-
// acorn-walk does not fire a generic "Statement" visitor for concrete
103-
// node types, so each type must be registered individually.
98+
// acorn-walk's simple() fires a generic "Statement" visitor for every
99+
// node dispatched in a statement position (Program body, block bodies,
100+
// if/for/while bodies, etc.). This automatically covers all current and
101+
// future ESTree statement types without hardcoding a list.
104102
const visitor = { __proto__: null };
105-
const handler = (node) => {
103+
visitor.Statement = (node) => {
104+
if (kExcludedStatementTypes.has(node.type)) return;
106105
ArrayPrototypePush(statements, {
107106
__proto__: null,
108107
startOffset: node.start,
109108
endOffset: node.end,
110109
count: 0,
111110
});
112111
};
113-
for (let i = 0; i < kStatementTypes.length; ++i) {
114-
visitor[kStatementTypes[i]] = handler;
115-
}
116112

117-
// Try parsing as a module first, fall back to script for files that
118-
// use CommonJS syntax incompatible with module mode.
113+
// Parse as script with permissive flags — script mode is non-strict,
114+
// so it handles legacy CJS (e.g. `with` statements) while the
115+
// allow* flags enable ESM syntax (import/export/top-level await).
119116
let ast;
120117
try {
121118
ast = AcornParser.parse(source, {
122119
__proto__: null,
123120
ecmaVersion: 'latest',
124-
sourceType: 'module',
121+
sourceType: 'script',
125122
allowReturnOutsideFunction: true,
126123
allowImportExportEverywhere: true,
127124
allowAwaitOutsideFunction: true,
128125
});
129126
} catch {
130-
try {
131-
ast = AcornParser.parse(source, {
132-
__proto__: null,
133-
ecmaVersion: 'latest',
134-
sourceType: 'script',
135-
allowReturnOutsideFunction: true,
136-
allowAwaitOutsideFunction: true,
137-
});
138-
} catch {
139-
this.#sourceStatements.set(fileUrl, null);
140-
return null;
141-
}
127+
this.#sourceStatements.set(fileUrl, null);
128+
return null;
142129
}
143130

144131
acornWalkSimple(ast, visitor);
@@ -341,28 +328,34 @@ class TestCoverage {
341328
const statementReports = [];
342329

343330
if (statements) {
331+
// Flatten all V8 coverage ranges into a single array so
332+
// the statement loop only needs two levels of iteration.
333+
const allRanges = [];
334+
for (let fi = 0; fi < functions.length; ++fi) {
335+
const { ranges } = functions[fi];
336+
for (let ri = 0; ri < ranges.length; ++ri) {
337+
ArrayPrototypePush(allRanges, ranges[ri]);
338+
}
339+
}
340+
344341
for (let j = 0; j < statements.length; ++j) {
345342
const stmt = statements[j];
346343
let bestCount = 0;
347-
let bestRange = null;
348-
349-
for (let fi = 0; fi < functions.length; ++fi) {
350-
const { ranges } = functions[fi];
351-
for (let ri = 0; ri < ranges.length; ++ri) {
352-
const range = ranges[ri];
353-
if (range.startOffset <= stmt.startOffset &&
354-
range.endOffset >= stmt.endOffset) {
355-
const size = range.endOffset - range.startOffset;
356-
if (bestRange === null ||
357-
size < (bestRange.endOffset - bestRange.startOffset)) {
358-
bestCount = range.count;
359-
bestRange = range;
360-
}
344+
let bestSize = Infinity;
345+
346+
for (let ri = 0; ri < allRanges.length; ++ri) {
347+
const range = allRanges[ri];
348+
if (range.startOffset <= stmt.startOffset &&
349+
range.endOffset >= stmt.endOffset) {
350+
const size = range.endOffset - range.startOffset;
351+
if (size < bestSize) {
352+
bestCount = range.count;
353+
bestSize = size;
361354
}
362355
}
363356
}
364357

365-
stmt.count = bestRange !== null ? bestCount : 0;
358+
stmt.count = bestSize !== Infinity ? bestCount : 0;
366359

367360
const stmtLine = findLineForOffset(stmt.startOffset, lines);
368361
const isIgnored = stmtLine != null && stmtLine.ignore;
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
'use strict';
2+
const test = require('node:test');
3+
4+
class Animal {
5+
constructor(name) {
6+
this.name = name;
7+
}
8+
9+
speak() {
10+
return `${this.name} makes a sound`;
11+
}
12+
13+
static create(name) {
14+
return new Animal(name);
15+
}
16+
}
17+
18+
class Dog extends Animal {
19+
static {
20+
Dog.species = 'Canis familiaris';
21+
}
22+
23+
speak() {
24+
return `${this.name} barks`;
25+
}
26+
}
27+
28+
// This class is never instantiated — its body should be uncovered.
29+
class UnusedClass {
30+
doSomething() {
31+
return 42;
32+
}
33+
}
34+
35+
test('class coverage', () => {
36+
const dog = Dog.create('Rex');
37+
dog.speak();
38+
});

test/parallel/test-runner-coverage-statements.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,3 +202,32 @@ test('100% statement coverage for fully covered file', () => {
202202
assert.ok(invalidTap, 'invalid-tap.js should be in the report');
203203
assert.strictEqual(invalidTap.coveredStatementPercent, 100);
204204
});
205+
206+
test('statement coverage counts class and function declarations', () => {
207+
const classFixture = fixtures.path('test-runner', 'coverage-class.js');
208+
const result = spawnSync(process.execPath, [
209+
'--test',
210+
'--experimental-test-coverage',
211+
'--test-coverage-exclude=!test/**',
212+
'--test-reporter', reporter,
213+
classFixture,
214+
]);
215+
216+
assert.strictEqual(result.stderr.toString(), '');
217+
assert.strictEqual(result.status, 0);
218+
219+
const { summary } = JSON.parse(result.stdout.toString());
220+
const classFile = summary.files.find(
221+
(f) => f.path.endsWith('coverage-class.js'),
222+
);
223+
224+
assert.ok(classFile, 'coverage-class.js should be in the report');
225+
assert.ok(classFile.totalStatementCount > 0,
226+
`totalStatementCount must be > 0, got ${classFile.totalStatementCount}`);
227+
228+
// The file has an unused class, so statement coverage should be < 100%.
229+
assert.ok(classFile.coveredStatementPercent < 100,
230+
'should have uncovered statements from UnusedClass');
231+
assert.ok(classFile.coveredStatementPercent > 0,
232+
'should have covered statements from Dog/Animal');
233+
});

0 commit comments

Comments
 (0)