FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test_runner: improve code coverage cleanup · nodejs/node@c981e61 · GitHub

/ node Public

Commit c981e61

Browse files
cjihrig
authored and
targos
committed
test_runner: improve code coverage cleanup
The test runner's code coverage leaves old coverage data in the temp directory. This commit updates the cleanup logic to: - Stop code collection. Otherwise V8 would write collection data again when the process exits. - Remove the temp directory containing the coverage data. - Attempt to clean up the coverage data even if parsing the data resulted in an error. With this change, I no longer see any coverage data left behind in the system temp directory. Refs: nodejs/build#3864 Refs: nodejs/build#3887 PR-URL: #54856 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 309cb1c commit c981e61

3 files changed

Lines changed: 48 additions & 20 deletions

File tree

‎lib/internal/test_runner/coverage.js‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const {
2323
mkdtempSync,
2424
opendirSync,
2525
readFileSync,
26+
rmSync,
2627
} = require('fs');
2728
const { setupCoverageHooks } = require('internal/util');
2829
const { tmpdir } = require('os');
@@ -270,28 +271,35 @@ class TestCoverage {
270271
cleanup() {
271272
// Restore the original value of process.env.NODE_V8_COVERAGE. Then, copy
272273
// all of the created coverage files to the original coverage directory.
274+
internalBinding('profiler').endCoverage();
275+
273276
if (this.originalCoverageDirectory === undefined) {
274277
delete process.env.NODE_V8_COVERAGE;
275-
return;
276-
}
277-
278-
process.env.NODE_V8_COVERAGE = this.originalCoverageDirectory;
279-
let dir;
278+
} else {
279+
process.env.NODE_V8_COVERAGE = this.originalCoverageDirectory;
280+
let dir;
280281

281-
try {
282-
mkdirSync(this.originalCoverageDirectory, { __proto__: null, recursive: true });
283-
dir = opendirSync(this.coverageDirectory);
282+
try {
283+
mkdirSync(this.originalCoverageDirectory, { __proto__: null, recursive: true });
284+
dir = opendirSync(this.coverageDirectory);
284285

285-
for (let entry; (entry = dir.readSync()) !== null;) {
286-
const src = join(this.coverageDirectory, entry.name);
287-
const dst = join(this.originalCoverageDirectory, entry.name);
288-
copyFileSync(src, dst);
289-
}
290-
} finally {
291-
if (dir) {
292-
dir.closeSync();
286+
for (let entry; (entry = dir.readSync()) !== null;) {
287+
const src = join(this.coverageDirectory, entry.name);
288+
const dst = join(this.originalCoverageDirectory, entry.name);
289+
copyFileSync(src, dst);
290+
}
291+
} finally {
292+
if (dir) {
293+
dir.closeSync();
294+
}
293295
}
294296
}
297+
298+
try {
299+
rmSync(this.coverageDirectory, { __proto__: null, recursive: true });
300+
} catch {
301+
// Ignore cleanup errors.
302+
}
295303
}
296304

297305
getCoverageFromDirectory() {

‎lib/internal/test_runner/harness.js‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,15 @@ function collectCoverage(rootTest, coverage) {
113113

114114
try {
115115
summary = coverage.summary();
116-
coverage.cleanup();
117116
} catch (err) {
118-
const op = summary ? 'clean up' : 'report';
119-
const msg = `Warning: Could not ${op} code coverage. ${err}`;
117+
rootTest.diagnostic(`Warning: Could not report code coverage. ${err}`);
118+
process.exitCode = kGenericUserError;
119+
}
120120

121-
rootTest.diagnostic(msg);
121+
try {
122+
coverage.cleanup();
123+
} catch (err) {
124+
rootTest.diagnostic(`Warning: Could not clean up code coverage. ${err}`);
122125
process.exitCode = kGenericUserError;
123126
}
124127

‎src/inspector_profiler.cc‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,21 @@ static void StopCoverage(const FunctionCallbackInfo<Value>& args) {
503503
}
504504
}
505505

506+
static void EndCoverage(const FunctionCallbackInfo<Value>& args) {
507+
Environment* env = Environment::GetCurrent(args);
508+
V8CoverageConnection* connection = env->coverage_connection();
509+
510+
Debug(env,
511+
DebugCategory::INSPECTOR_PROFILER,
512+
"EndCoverage, connection %s nullptr\n",
513+
connection == nullptr ? "==" : "!=");
514+
515+
if (connection != nullptr) {
516+
Debug(env, DebugCategory::INSPECTOR_PROFILER, "Ending coverage\n");
517+
connection->End();
518+
}
519+
}
520+
506521
static void Initialize(Local<Object> target,
507522
Local<Value> unused,
508523
Local<Context> context,
@@ -512,13 +527,15 @@ static void Initialize(Local<Object> target,
512527
context, target, "setSourceMapCacheGetter", SetSourceMapCacheGetter);
513528
SetMethod(context, target, "takeCoverage", TakeCoverage);
514529
SetMethod(context, target, "stopCoverage", StopCoverage);
530+
SetMethod(context, target, "endCoverage", EndCoverage);
515531
}
516532

517533
void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
518534
registry->Register(SetCoverageDirectory);
519535
registry->Register(SetSourceMapCacheGetter);
520536
registry->Register(TakeCoverage);
521537
registry->Register(StopCoverage);
538+
registry->Register(EndCoverage);
522539
}
523540

524541
} // namespace profiler

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL