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 <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: James M Snell <[email protected]>

lib/internal/test_runner/coverage.js

@@ -23,6 +23,7 @@ const {
  mkdtempSync,
  opendirSync,
  readFileSync,
+ rmSync,
 } = require('fs');
 const { setupCoverageHooks } = require('internal/util');
 const { tmpdir } = require('os');
@@ -272,28 +273,35 @@ class TestCoverage {
  cleanup() {
  // Restore the original value of process.env.NODE_V8_COVERAGE. Then, copy
  // all of the created coverage files to the original coverage directory.
+ internalBinding('profiler').endCoverage();
+
  if (this.originalCoverageDirectory === undefined) {
  delete process.env.NODE_V8_COVERAGE;
- return;
- }
-
- process.env.NODE_V8_COVERAGE = this.originalCoverageDirectory;
- let dir;
+ } else {
+ process.env.NODE_V8_COVERAGE = this.originalCoverageDirectory;
+ let dir;
 
- try {
- mkdirSync(this.originalCoverageDirectory, { __proto__: null, recursive: true });
- dir = opendirSync(this.coverageDirectory);
+  try {
+  mkdirSync(this.originalCoverageDirectory, { __proto__: null, recursive: true });
+  dir = opendirSync(this.coverageDirectory);
 
- for (let entry; (entry = dir.readSync()) !== null;) {
- const src = join(this.coverageDirectory, entry.name);
- const dst = join(this.originalCoverageDirectory, entry.name);
- copyFileSync(src, dst);
- }
- } finally {
- if (dir) {
- dir.closeSync();
+ for (let entry; (entry = dir.readSync()) !== null;) {
+ const src = join(this.coverageDirectory, entry.name);
+ const dst = join(this.originalCoverageDirectory, entry.name);
+ copyFileSync(src, dst);
+ }
+ } finally {
+ if (dir) {
+ dir.closeSync();
+ }
  }
  }
+
+ try {
+ rmSync(this.coverageDirectory, { __proto__: null, recursive: true });
+ } catch {
+ // Ignore cleanup errors.
+ }
  }
 
  getCoverageFromDirectory() {

lib/internal/test_runner/harness.js

@@ -159,12 +159,15 @@ function collectCoverage(rootTest, coverage) {
 
  try {
  summary = coverage.summary();
- coverage.cleanup();
  } catch (err) {
- const op = summary ? 'clean up' : 'report';
- const msg = `Warning: Could not ${op} code coverage. ${err}`;
+ rootTest.diagnostic(`Warning: Could not report code coverage. ${err}`);
+ process.exitCode = kGenericUserError;
+ }
 
- rootTest.diagnostic(msg);
+ try {
+ coverage.cleanup();
+ } catch (err) {
+ rootTest.diagnostic(`Warning: Could not clean up code coverage. ${err}`);
  process.exitCode = kGenericUserError;
  }
 

src/inspector_profiler.cc

@@ -556,6 +556,21 @@ static void StopCoverage(const FunctionCallbackInfo<Value>& args) {
  }
 }
 
+static void EndCoverage(const FunctionCallbackInfo<Value>& args) {
+ Environment* env = Environment::GetCurrent(args);
+ V8CoverageConnection* connection = env->coverage_connection();
+
+ Debug(env,
+ DebugCategory::INSPECTOR_PROFILER,
+ "EndCoverage, connection %s nullptr\n",
+ connection == nullptr ? "==" : "!=");
+
+ if (connection != nullptr) {
+ Debug(env, DebugCategory::INSPECTOR_PROFILER, "Ending coverage\n");
+ connection->End();
+ }
+}
+
 static void Initialize(Local<Object> target,
  Local<Value> unused,
  Local<Context> context,
@@ -565,13 +580,15 @@ static void Initialize(Local<Object> target,
  context, target, "setSourceMapCacheGetter", SetSourceMapCacheGetter);
  SetMethod(context, target, "takeCoverage", TakeCoverage);
  SetMethod(context, target, "stopCoverage", StopCoverage);
+ SetMethod(context, target, "endCoverage", EndCoverage);
 }
 
 void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
  registry->Register(SetCoverageDirectory);
  registry->Register(SetSourceMapCacheGetter);
  registry->Register(TakeCoverage);
  registry->Register(StopCoverage);
+ registry->Register(EndCoverage);
 }
 
 } // namespace profiler