From ab21ea1469d37d13850de442141d66aa7e7f3b45 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 29 Oct 2026 17:19:43 +0000 Subject: [PATCH] module: disable the compile cache while V8 coverage is collected V8 cannot collect block coverage for functions that are deserialized from the code cache, so it only reports how many times each of them ran. With a warm compile cache, branches that never ran inside those functions are reported as covered, and tools that merge reports from several processes can report functions that ran as never run. The documentation recommended turning the cache off by hand. Do it automatically when coverage collection starts with the process, through NODE_V8_COVERAGE or --experimental-test-coverage. The compile cache tests stop propagating NODE_V8_COVERAGE to the processes they spawn, so that they keep working when the test suite itself runs with coverage. Signed-off-by: Shelley Vohr --- doc/api/module.md | 14 +++--- src/compile_cache.h | 2 +- src/env.cc | 10 ++++- test/parallel/test-compile-cache-api-env.js | 3 ++ test/parallel/test-compile-cache-api-flush.js | 3 ++ ...-compile-cache-api-options-portable-env.js | 3 ++ .../test-compile-cache-api-portable.js | 3 ++ .../test-compile-cache-api-readonly.js | 3 ++ .../test-compile-cache-api-success.js | 3 ++ .../parallel/test-compile-cache-api-tmpdir.js | 3 ++ .../parallel/test-compile-cache-bad-syntax.js | 3 ++ test/parallel/test-compile-cache-coverage.js | 45 +++++++++++++++++++ .../test-compile-cache-dynamic-import.js | 3 ++ test/parallel/test-compile-cache-esm.js | 3 ++ .../test-compile-cache-existing-directory.js | 3 ++ .../test-compile-cache-portable-esm.js | 3 ++ test/parallel/test-compile-cache-portable.js | 3 ++ test/parallel/test-compile-cache-success.js | 3 ++ .../test-compile-cache-typescript-commonjs.js | 3 ++ .../test-compile-cache-typescript-esm.js | 3 ++ ...mpile-cache-typescript-strip-sourcemaps.js | 3 ++ .../test-compile-cache-updated-file.js | 3 ++ test/parallel/test-compile-cache-wasm.js | 3 ++ 23 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-compile-cache-coverage.js diff --git a/doc/api/module.md b/doc/api/module.md index f05726286f09..f3707765a381 100644 --- a/doc/api/module.md +++ b/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 DISABLED Node.js cannot enable the compile cache because the environment variable - NODE_DISABLE_COMPILE_CACHE=1 has been set. + NODE_DISABLE_COMPILE_CACHE=1 has been set, or because V8 + coverage is being collected. @@ -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 diff --git a/src/compile_cache.h b/src/compile_cache.h index 527f72f33d42..25071de4ccea 100644 --- a/src/compile_cache.h +++ b/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, diff --git a/src/env.cc b/src/env.cc index c30b6e2f5049..426b7375679e 100644 --- a/src/env.cc +++ b/src/env.cc @@ -1398,8 +1398,16 @@ CompileCacheEnableResult Environment::EnableCompileCache( std::string disable_env; if (credentials::SafeGetenv( "NODE_DISABLE_COMPILE_CACHE", &disable_env, this)) { - result.status = CompileCacheEnableStatus::DISABLED; result.message = "Disabled by NODE_DISABLE_COMPILE_CACHE"; + } +#if HAVE_INSPECTOR + // V8 cannot collect block coverage for functions deserialized from the cache. + if (result.message.empty() && coverage_connection() != nullptr) { + result.message = "Disabled because V8 coverage is being collected"; + } +#endif // HAVE_INSPECTOR + if (!result.message.empty()) { + result.status = CompileCacheEnableStatus::DISABLED; Debug(this, DebugCategory::COMPILE_CACHE, "[compile cache] %s.\n", diff --git a/test/parallel/test-compile-cache-api-env.js b/test/parallel/test-compile-cache-api-env.js index 197f1495a886..aa7937df92a1 100644 --- a/test/parallel/test-compile-cache-api-env.js +++ b/test/parallel/test-compile-cache-api-env.js @@ -10,6 +10,9 @@ const fixtures = require('../common/fixtures'); const tmpdir = require('../common/tmpdir'); const fs = require('fs'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + { // Test that it works with non-existent directory. tmpdir.refresh(); diff --git a/test/parallel/test-compile-cache-api-flush.js b/test/parallel/test-compile-cache-api-flush.js index 7e66b1fee103..00aee0cd0a09 100644 --- a/test/parallel/test-compile-cache-api-flush.js +++ b/test/parallel/test-compile-cache-api-flush.js @@ -8,6 +8,9 @@ const assert = require('assert'); const tmpdir = require('../common/tmpdir'); const fixtures = require('../common/fixtures'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + { // Test that it works with non-existent directory. tmpdir.refresh(); diff --git a/test/parallel/test-compile-cache-api-options-portable-env.js b/test/parallel/test-compile-cache-api-options-portable-env.js index 5586e3e84046..9922f5f80ed3 100644 --- a/test/parallel/test-compile-cache-api-options-portable-env.js +++ b/test/parallel/test-compile-cache-api-options-portable-env.js @@ -11,6 +11,9 @@ const tmpdir = require('../common/tmpdir'); const fs = require('fs'); const path = require('path'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + const wrapper = fixtures.path('compile-cache-wrapper-options.js'); tmpdir.refresh(); diff --git a/test/parallel/test-compile-cache-api-portable.js b/test/parallel/test-compile-cache-api-portable.js index 620b3bdce875..f29826d469a9 100644 --- a/test/parallel/test-compile-cache-api-portable.js +++ b/test/parallel/test-compile-cache-api-portable.js @@ -11,6 +11,9 @@ const tmpdir = require('../common/tmpdir'); const path = require('path'); const fixtures = require('../common/fixtures'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + tmpdir.refresh(); const workDir = path.join(tmpdir.path, 'work'); const cacheRel = '.compile_cache_dir'; diff --git a/test/parallel/test-compile-cache-api-readonly.js b/test/parallel/test-compile-cache-api-readonly.js index 8ac0248a4d53..7512c847a08f 100644 --- a/test/parallel/test-compile-cache-api-readonly.js +++ b/test/parallel/test-compile-cache-api-readonly.js @@ -11,6 +11,9 @@ const path = require('path'); const tmpdir = require('../common/tmpdir'); const fixtures = require('../common/fixtures'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + tmpdir.refresh(); const wrapper = fixtures.path('compile-cache-wrapper-options.js'); const target = path.join(tmpdir.path, 'target.js'); diff --git a/test/parallel/test-compile-cache-api-success.js b/test/parallel/test-compile-cache-api-success.js index 2d4d94fd6e9d..10b5dc813c2b 100644 --- a/test/parallel/test-compile-cache-api-success.js +++ b/test/parallel/test-compile-cache-api-success.js @@ -10,6 +10,9 @@ const tmpdir = require('../common/tmpdir'); const fs = require('fs'); const path = require('path'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + { // Test that it works with non-existent directory. tmpdir.refresh(); diff --git a/test/parallel/test-compile-cache-api-tmpdir.js b/test/parallel/test-compile-cache-api-tmpdir.js index e676893b01f5..0353aa716af2 100644 --- a/test/parallel/test-compile-cache-api-tmpdir.js +++ b/test/parallel/test-compile-cache-api-tmpdir.js @@ -11,6 +11,9 @@ const tmpdir = require('../common/tmpdir'); const fs = require('fs'); const path = require('path'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + { // Test that it works with non-existent directory. tmpdir.refresh(); diff --git a/test/parallel/test-compile-cache-bad-syntax.js b/test/parallel/test-compile-cache-bad-syntax.js index 63f6374b2c38..213d295b4a02 100644 --- a/test/parallel/test-compile-cache-bad-syntax.js +++ b/test/parallel/test-compile-cache-bad-syntax.js @@ -10,6 +10,9 @@ const assert = require('assert'); const fs = require('fs'); const path = require('path'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + { // Test that it throws if the script fails to parse, and no cache is created. tmpdir.refresh(); diff --git a/test/parallel/test-compile-cache-coverage.js b/test/parallel/test-compile-cache-coverage.js new file mode 100644 index 000000000000..f2f5e9e2f77f --- /dev/null +++ b/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)); diff --git a/test/parallel/test-compile-cache-dynamic-import.js b/test/parallel/test-compile-cache-dynamic-import.js index 01731dc68225..d7b1daf57dc9 100644 --- a/test/parallel/test-compile-cache-dynamic-import.js +++ b/test/parallel/test-compile-cache-dynamic-import.js @@ -8,6 +8,9 @@ const assert = require('assert'); const tmpdir = require('../common/tmpdir'); const fixtures = require('../common/fixtures'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + { tmpdir.refresh(); const dir = tmpdir.resolve('.compile_cache_dir'); diff --git a/test/parallel/test-compile-cache-esm.js b/test/parallel/test-compile-cache-esm.js index 963de8bcaba2..0df4ba6a7373 100644 --- a/test/parallel/test-compile-cache-esm.js +++ b/test/parallel/test-compile-cache-esm.js @@ -8,6 +8,9 @@ const assert = require('assert'); const tmpdir = require('../common/tmpdir'); const fixtures = require('../common/fixtures'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + { tmpdir.refresh(); const dir = tmpdir.resolve('.compile_cache_dir'); diff --git a/test/parallel/test-compile-cache-existing-directory.js b/test/parallel/test-compile-cache-existing-directory.js index 0fc60f9544d5..f4f51e7786a8 100644 --- a/test/parallel/test-compile-cache-existing-directory.js +++ b/test/parallel/test-compile-cache-existing-directory.js @@ -9,6 +9,9 @@ const fs = require('fs'); const fixtures = require('../common/fixtures'); const tmpdir = require('../common/tmpdir'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + { tmpdir.refresh(); const dir = tmpdir.resolve('.compile_cache_dir'); diff --git a/test/parallel/test-compile-cache-portable-esm.js b/test/parallel/test-compile-cache-portable-esm.js index 18766b6810ea..a536602f3cf2 100644 --- a/test/parallel/test-compile-cache-portable-esm.js +++ b/test/parallel/test-compile-cache-portable-esm.js @@ -9,6 +9,9 @@ const tmpdir = require('../common/tmpdir'); const fs = require('fs'); const path = require('path'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + tmpdir.refresh(); const workDir = path.join(tmpdir.path, 'work'); diff --git a/test/parallel/test-compile-cache-portable.js b/test/parallel/test-compile-cache-portable.js index b5978b5b0c62..8ea7cf400d4d 100644 --- a/test/parallel/test-compile-cache-portable.js +++ b/test/parallel/test-compile-cache-portable.js @@ -10,6 +10,9 @@ const fs = require('fs'); const tmpdir = require('../common/tmpdir'); const path = require('path'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + tmpdir.refresh(); const workDir = path.join(tmpdir.path, 'work'); const cacheRel = '.compile_cache_dir'; diff --git a/test/parallel/test-compile-cache-success.js b/test/parallel/test-compile-cache-success.js index c02a62432869..ff834501e542 100644 --- a/test/parallel/test-compile-cache-success.js +++ b/test/parallel/test-compile-cache-success.js @@ -10,6 +10,9 @@ const tmpdir = require('../common/tmpdir'); const fs = require('fs'); const path = require('path'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + { // Test that it works with non-existent directory. tmpdir.refresh(); diff --git a/test/parallel/test-compile-cache-typescript-commonjs.js b/test/parallel/test-compile-cache-typescript-commonjs.js index 60e86c3c0f9d..925d93669925 100644 --- a/test/parallel/test-compile-cache-typescript-commonjs.js +++ b/test/parallel/test-compile-cache-typescript-commonjs.js @@ -11,6 +11,9 @@ const assert = require('assert'); const tmpdir = require('../common/tmpdir'); const fixtures = require('../common/fixtures'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + // Check cache for .ts files that would be run as CommonJS. { tmpdir.refresh(); diff --git a/test/parallel/test-compile-cache-typescript-esm.js b/test/parallel/test-compile-cache-typescript-esm.js index 98473dbb2a5b..05142a2a8e96 100644 --- a/test/parallel/test-compile-cache-typescript-esm.js +++ b/test/parallel/test-compile-cache-typescript-esm.js @@ -11,6 +11,9 @@ const assert = require('assert'); const tmpdir = require('../common/tmpdir'); const fixtures = require('../common/fixtures'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + // Check cache for .ts files that would be run as ESM. { tmpdir.refresh(); diff --git a/test/parallel/test-compile-cache-typescript-strip-sourcemaps.js b/test/parallel/test-compile-cache-typescript-strip-sourcemaps.js index a6c95e432b78..8e7f9f314771 100644 --- a/test/parallel/test-compile-cache-typescript-strip-sourcemaps.js +++ b/test/parallel/test-compile-cache-typescript-strip-sourcemaps.js @@ -12,6 +12,9 @@ const assert = require('assert'); const tmpdir = require('../common/tmpdir'); const fixtures = require('../common/fixtures'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + tmpdir.refresh(); const dir = tmpdir.resolve('.compile_cache_dir'); const script = fixtures.path('typescript', 'ts', 'test-typescript.ts'); diff --git a/test/parallel/test-compile-cache-updated-file.js b/test/parallel/test-compile-cache-updated-file.js index 68f20a6990cc..60c126843f0f 100644 --- a/test/parallel/test-compile-cache-updated-file.js +++ b/test/parallel/test-compile-cache-updated-file.js @@ -8,6 +8,9 @@ const assert = require('assert'); const fs = require('fs'); const tmpdir = require('../common/tmpdir'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + { tmpdir.refresh(); const dir = tmpdir.resolve('.compile_cache_dir'); diff --git a/test/parallel/test-compile-cache-wasm.js b/test/parallel/test-compile-cache-wasm.js index 278198378dd2..ac84f096b000 100644 --- a/test/parallel/test-compile-cache-wasm.js +++ b/test/parallel/test-compile-cache-wasm.js @@ -8,6 +8,9 @@ const assert = require('assert'); const tmpdir = require('../common/tmpdir'); const fixtures = require('../common/fixtures'); +// The compile cache is disabled while V8 coverage is being collected. +process.env.NODE_V8_COVERAGE = ''; + const script = fixtures.path('es-modules', 'test-wasm-js-string-builtins.mjs'); // V8 only serializes optimized code, so a complete cache requires eager