module: disable the compile cache while V8 coverage is collected

doc/api/module.md

@@ -464,10 +464,12 @@ no-op, and the directory is not created if it is missing.
 
 ### Limitations of the compile cache
 
-Currently when using the compile cache with [V8 JavaScript code coverage][], the
-coverage being collected by V8 may be less precise in functions that are
-deserialized from the code cache. It's recommended to turn this off when
-running tests to generate precise coverage.
+V8 cannot collect block-level [V8 JavaScript code coverage][] for functions that
+are deserialized from the code cache. The compile cache is therefore disabled
+when coverage collection starts with the process, through
+[`NODE_V8_COVERAGE=dir`][] or [`--experimental-test-coverage`][]. Coverage that
+is started later, for example through the inspector, may be less precise in
+functions that were already deserialized from the code cache.
 
 Compilation cache generated by one version of Node.js can not be reused by a different
 version of Node.js. Cache generated by different versions of Node.js will be stored
@@ -525,7 +527,8 @@ The following constants are returned as the `status` field in the object returne
     <td><code>DISABLED</code></td>
     <td>
       Node.js cannot enable the compile cache because the environment variable
-      <code>NODE_DISABLE_COMPILE_CACHE=1</code> has been set.
+      <code>NODE_DISABLE_COMPILE_CACHE=1</code> has been set, or because V8
+      coverage is being collected.
     </td>
   </tr>
 </table>
@@ -2111,6 +2114,7 @@ returned object contains the following keys:
 [V8 code cache]: https://v8.dev/blog/code-caching-for-devs
 [`"exports"`]: packages.md#exports
 [`--enable-source-maps`]: cli.md#--enable-source-maps
+[`--experimental-test-coverage`]: cli.md#--experimental-test-coverage
 [`--import`]: cli.md#--importmodule
 [`--require`]: cli.md#-r---require-module
 [`NODE_COMPILE_CACHE=dir`]: cli.md#node_compile_cachedir

src/compile_cache.h

@@ -47,7 +47,7 @@ struct CompileCacheEntry {
   V(FAILED)          /* Failed to enable the cache */                          \
   V(ENABLED)         /* Was not enabled before, and now enabled. */            \
   V(ALREADY_ENABLED) /* Was already enabled. */                                \
-  V(DISABLED)        /* Has been disabled by NODE_DISABLE_COMPILE_CACHE. */
+  V(DISABLED)        /* Disabled by the environment or by V8 coverage. */
 
 enum class CompileCacheEnableStatus : uint8_t {
 #define V(status) status,

src/env.cc

@@ -1396,10 +1396,20 @@ CompileCacheEnableResult Environment::EnableCompileCache(
     const std::string& cache_dir, EnableOption option) {
   CompileCacheEnableResult result;
   std::string disable_env;
+  const char* disabled_reason = nullptr;
   if (credentials::SafeGetenv(
           "NODE_DISABLE_COMPILE_CACHE", &disable_env, this)) {
+    disabled_reason = "Disabled by NODE_DISABLE_COMPILE_CACHE";
+#if HAVE_INSPECTOR
+  } else if (coverage_connection() != nullptr) {
+    // V8 cannot collect block coverage for functions deserialized from the
+    // cache.
+    disabled_reason = "Disabled because V8 coverage is being collected";
+#endif  // HAVE_INSPECTOR
+  }
+  if (disabled_reason != nullptr) {
     result.status = CompileCacheEnableStatus::DISABLED;
-    result.message = "Disabled by NODE_DISABLE_COMPILE_CACHE";
+    result.message = disabled_reason;
     Debug(this,
           DebugCategory::COMPILE_CACHE,
           "[compile cache] %s.\n",

test/common/compile-cache.js

@@ -0,0 +1,24 @@
+'use strict';
+
+const common = require('./');
+const childProcess = require('./child_process');
+
+// The compile cache is disabled while V8 coverage is being collected. In that
+// case, check that the first child says so and skip the rest of the test.
+function withCompileCache(spawn) {
+  return (...args) => {
+    if (!process.env.NODE_V8_COVERAGE) {
+      return spawn(...args);
+    }
+    args.pop();
+    childProcess.spawnSyncAndExit(...args, {
+      stderr: /Disabled because V8 coverage is being collected/,
+    });
+    common.skip('the compile cache is disabled by V8 coverage');
+  };
+}
+
+module.exports = {
+  spawnSyncAndAssert: withCompileCache(childProcess.spawnSyncAndAssert),
+  spawnSyncAndExit: withCompileCache(childProcess.spawnSyncAndExit),
+};

test/parallel/test-compile-cache-api-env.js

@@ -4,7 +4,7 @@
 // with a NODE_COMPILE_CACHE environment variable override.
 
 require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const fixtures = require('../common/fixtures');
 const tmpdir = require('../common/tmpdir');

test/parallel/test-compile-cache-api-flush.js

@@ -3,7 +3,7 @@
 // This tests module.flushCompileCache() works as expected.
 
 require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const tmpdir = require('../common/tmpdir');
 const fixtures = require('../common/fixtures');

test/parallel/test-compile-cache-api-options-portable-env.js

@@ -4,7 +4,7 @@
 // variable NODE_COMPILE_CACHE overrides.
 
 require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const fixtures = require('../common/fixtures');
 const tmpdir = require('../common/tmpdir');

test/parallel/test-compile-cache-api-portable.js

@@ -4,7 +4,7 @@
 // and supports portable paths across directory relocations.
 
 require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const fs = require('fs');
 const tmpdir = require('../common/tmpdir');

test/parallel/test-compile-cache-api-readonly.js

@@ -4,7 +4,7 @@
 // entries are read, nothing is written, and a missing directory is not created.
 
 const common = require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const fs = require('fs');
 const path = require('path');

test/parallel/test-compile-cache-api-success.js

@@ -3,7 +3,7 @@
 // This tests module.enableCompileCache() and module.getCompileCacheDir() work.
 
 require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const fixtures = require('../common/fixtures');
 const tmpdir = require('../common/tmpdir');

test/parallel/test-compile-cache-api-tmpdir.js

@@ -4,7 +4,7 @@
 // the TMPDIR environment variable override.
 
 require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const fixtures = require('../common/fixtures');
 const tmpdir = require('../common/tmpdir');

test/parallel/test-compile-cache-bad-syntax.js

@@ -3,7 +3,7 @@
 // This tests NODE_COMPILE_CACHE works.
 
 require('../common');
-const { spawnSyncAndExit } = require('../common/child_process');
+const { spawnSyncAndExit } = require('../common/compile-cache');
 const fixtures = require('../common/fixtures');
 const tmpdir = require('../common/tmpdir');
 const assert = require('assert');

test/parallel/test-compile-cache-coverage.js

@@ -0,0 +1,45 @@
+'use strict';
+
+const common = require('../common');
+common.skipIfInspectorDisabled();
+
+const { spawnSyncAndAssert } = require('../common/child_process');
+const assert = require('assert');
+const fixtures = require('../common/fixtures');
+const fs = require('fs');
+const tmpdir = require('../common/tmpdir');
+
+tmpdir.refresh();
+const cacheDir = tmpdir.resolve('compile_cache');
+const env = {
+  ...process.env,
+  NODE_DEBUG_NATIVE: 'COMPILE_CACHE',
+  NODE_V8_COVERAGE: tmpdir.resolve('coverage'),
+};
+
+spawnSyncAndAssert(
+  process.execPath,
+  [fixtures.path('empty.js')],
+  { env: { ...env, NODE_COMPILE_CACHE: cacheDir }, cwd: tmpdir.path },
+  { stderr: /Disabled because V8 coverage is being collected/ });
+assert(!fs.existsSync(cacheDir));
+
+spawnSyncAndAssert(
+  process.execPath,
+  ['-r', fixtures.path('compile-cache-wrapper.js'), fixtures.path('empty.js')],
+  {
+    env: {
+      ...env,
+      NODE_COMPILE_CACHE: undefined,
+      NODE_TEST_COMPILE_CACHE_DIR: cacheDir,
+    },
+    cwd: tmpdir.path,
+  },
+  {
+    stdout(output) {
+      assert.match(output, /Compile cache already disabled/);
+      assert.match(output, /dir after enableCompileCache: undefined/);
+    },
+    stderr: /Disabled because V8 coverage is being collected/,
+  });
+assert(!fs.existsSync(cacheDir));

test/parallel/test-compile-cache-dynamic-import.js

@@ -3,7 +3,7 @@
 // This tests NODE_COMPILE_CACHE works in existing directory.
 
 require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const tmpdir = require('../common/tmpdir');
 const fixtures = require('../common/fixtures');

test/parallel/test-compile-cache-esm.js

@@ -3,7 +3,7 @@
 // This tests NODE_COMPILE_CACHE works in existing directory.
 
 require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const tmpdir = require('../common/tmpdir');
 const fixtures = require('../common/fixtures');

test/parallel/test-compile-cache-existing-directory.js

@@ -3,7 +3,7 @@
 // This tests NODE_COMPILE_CACHE works in existing directory.
 
 require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const fs = require('fs');
 const fixtures = require('../common/fixtures');

test/parallel/test-compile-cache-portable-esm.js

@@ -3,7 +3,7 @@
 // This tests NODE_COMPILE_CACHE works after moving directory and unusual characters in path are handled correctly.
 
 require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const tmpdir = require('../common/tmpdir');
 const fs = require('fs');

test/parallel/test-compile-cache-portable.js

@@ -4,7 +4,7 @@
 // environment variable.
 
 require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const fs = require('fs');
 const tmpdir = require('../common/tmpdir');

test/parallel/test-compile-cache-success.js

@@ -3,7 +3,7 @@
 // This tests NODE_COMPILE_CACHE works.
 
 require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const fixtures = require('../common/fixtures');
 const tmpdir = require('../common/tmpdir');

test/parallel/test-compile-cache-typescript-commonjs.js

@@ -6,7 +6,7 @@ const common = require('../common');
 if (!process.config.variables.node_use_amaro) {
   common.skip('Requires Amaro');
 }
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const tmpdir = require('../common/tmpdir');
 const fixtures = require('../common/fixtures');

test/parallel/test-compile-cache-typescript-esm.js

@@ -6,7 +6,7 @@ const common = require('../common');
 if (!process.config.variables.node_use_amaro) {
   common.skip('Requires Amaro');
 }
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const tmpdir = require('../common/tmpdir');
 const fixtures = require('../common/fixtures');

test/parallel/test-compile-cache-typescript-strip-sourcemaps.js

@@ -7,7 +7,7 @@ const common = require('../common');
 if (!process.config.variables.node_use_amaro) {
   common.skip('Requires Amaro');
 }
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const tmpdir = require('../common/tmpdir');
 const fixtures = require('../common/fixtures');

test/parallel/test-compile-cache-updated-file.js

@@ -3,7 +3,7 @@
 // This tests NODE_COMPILE_CACHE works in existing directory.
 
 require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const fs = require('fs');
 const tmpdir = require('../common/tmpdir');

test/parallel/test-compile-cache-wasm.js

@@ -3,7 +3,7 @@
 // This tests NODE_COMPILE_CACHE works for WebAssembly modules imported as ESM.
 
 require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
+const { spawnSyncAndAssert } = require('../common/compile-cache');
 const assert = require('assert');
 const tmpdir = require('../common/tmpdir');
 const fixtures = require('../common/fixtures');