diff --git a/crates/nub-cli/tests/test_coverage_default_exclusion.rs b/crates/nub-cli/tests/test_coverage_default_exclusion.rs new file mode 100644 index 000000000..189e52558 --- /dev/null +++ b/crates/nub-cli/tests/test_coverage_default_exclusion.rs @@ -0,0 +1,190 @@ +//! Under `--experimental-test-coverage`, nub must report exactly the files the +//! host `node` reports. +//! +//! Node applies its default test-file exclusion (`kDefaultPattern` in +//! `lib/internal/test_runner/utils.js`) only when NO `--test-coverage-exclude` is +//! set, and only from 23.5.0 — it was never backported to 22.x. nub injects an +//! exclude of its own to keep the preloaded runtime out of the report, which +//! silently switched that default off. +//! +//! The contract is parity with whatever `node` this host resolved, so these tests +//! diff nub's table against the live node's rather than against a fixed +//! expectation. That is what makes them version-proof: on 26 both exclude the test +//! file, on 22.15 both include it, and a hardcoded table would be wrong on one of +//! the two. + +#![cfg(unix)] + +use std::path::{Path, PathBuf}; +use std::process::Command; + +fn nub_binary() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_nub")) +} + +fn host_node_usable() -> bool { + Command::new("node") + .arg("--version") + .output() + .is_ok_and(|out| out.status.success()) +} + +/// Whether the host node has `--test-coverage-exclude` at all (22.5+). Below it the +/// flag is a bad option and node exits 9, so the user-exclude case has no control to +/// compare against and is skipped rather than asserted. +fn host_node_has_coverage_exclude() -> bool { + Command::new("node") + .args(["--test-coverage-exclude=probe/**", "-e", ""]) + .output() + .is_ok_and(|out| out.status.success()) +} + +struct Fixture { + _temp: tempfile::TempDir, + project: PathBuf, +} + +impl Fixture { + fn new() -> Self { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path().join("project"); + std::fs::create_dir_all(&project).unwrap(); + // ESM, not CJS. A CJS `require('node:test')` trips a separate defect on the + // 22.15 tier — upstream's convertCJSFilenameToURL mishandles scheme-only + // builtin ids once any sync resolve hook is registered; fixed by PR #803 — + // and the fixture itself throwing would red this test for a reason it does + // not govern. + std::fs::write( + project.join("package.json"), + "{ \"name\": \"cov\", \"private\": true, \"type\": \"module\" }\n", + ) + .unwrap(); + std::fs::write( + project.join("logic.js"), + "export const add = (a, b) => a + b;\n", + ) + .unwrap(); + std::fs::write( + project.join("logic.test.js"), + "import t from 'node:test';\n\ + import a from 'node:assert';\n\ + import { add } from './logic.js';\n\ + t('add', () => { a.strictEqual(add(1, 2), 3); });\n", + ) + .unwrap(); + Self { + project, + _temp: temp, + } + } + + fn command(&self, binary: &Path, extra: &[&str]) -> Command { + let mut command = Command::new(binary); + command + .current_dir(&self.project) + .env("XDG_CONFIG_HOME", self._temp.path().join("config")) + .env("XDG_CACHE_HOME", self._temp.path().join("cache")) + .env_remove("NODE_OPTIONS") + .args([ + "--test", + "--experimental-test-coverage", + "--test-reporter=tap", + ]) + .args(extra); + command + } +} + +/// The fixture file names the coverage table lists, in report order. Reducing to +/// names drops the percentages, and keeping only the fixture's own files drops nub's +/// runtime rows — so the comparison is about WHICH of the user's files are reported, +/// which is what the exclusion decides, and is not perturbed by the preload shifting +/// a branch denominator. +fn reported_files(runner: &str, mut command: Command) -> Vec { + let output = command.output().unwrap(); + let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "[{runner}] exited {:?}\nstdout: {stdout}\nstderr: {stderr}", + output.status.code() + ); + let start = stdout + .find("# start of coverage report") + .unwrap_or_else(|| panic!("[{runner}] no coverage report in output:\n{stdout}")); + let end = stdout[start..] + .find("# end of coverage report") + .map(|i| start + i) + .unwrap_or(stdout.len()); + stdout[start..end] + .lines() + .filter_map(|line| line.strip_prefix("# ")) + .filter_map(|row| row.split('|').next()) + .map(str::trim) + .filter(|name| *name == "logic.js" || *name == "logic.test.js") + .map(str::to_string) + .collect() +} + +/// Both runners see the same fixture and the same flags, so any difference is nub's +/// injected exclude changing which files Node decides to report. +#[test] +fn coverage_reports_the_same_files_as_the_host_node() { + if !host_node_usable() { + eprintln!("skipping coverage default-exclusion: no usable node on PATH"); + return; + } + let fixture = Fixture::new(); + + let node = reported_files("node", fixture.command(Path::new("node"), &[])); + let nub = reported_files("nub", fixture.command(&nub_binary(), &[])); + + // Positive control: an empty report on both sides would satisfy the equality + // below while proving nothing. The file under test is covered on every version. + assert!( + node.contains(&"logic.js".to_string()), + "control failed — node reported no coverage for the file under test: {node:?}" + ); + assert_eq!( + nub, node, + "nub and node disagree on which files the coverage report lists. nub's \ + injected --test-coverage-exclude either turned off Node's default \ + test-file exclusion, or re-stated it on a Node that has none." + ); +} + +/// A user-supplied `--test-coverage-exclude` disables Node's default exclusion for +/// node too, so nub must not re-add it — otherwise nub silently excludes test files +/// a user asked to see. +#[test] +fn a_user_supplied_exclude_matches_the_host_node() { + if !host_node_usable() { + eprintln!("skipping coverage user-exclude parity: no usable node on PATH"); + return; + } + if !host_node_has_coverage_exclude() { + eprintln!( + "skipping coverage user-exclude parity: this node predates \ + --test-coverage-exclude (22.5), so there is no control to diff against" + ); + return; + } + let fixture = Fixture::new(); + let user_exclude = ["--test-coverage-exclude=**/no-such-dir/**"]; + + let node = reported_files("node", fixture.command(Path::new("node"), &user_exclude)); + let nub = reported_files("nub", fixture.command(&nub_binary(), &user_exclude)); + + // Positive control: this exclude matches nothing, so node must still report the + // test file — which is exactly what a re-stated default pattern would take away. + assert!( + node.contains(&"logic.test.js".to_string()), + "control failed — a no-op user exclude should leave node's default \ + exclusion off, so the test file stays in the report: {node:?}" + ); + assert_eq!( + nub, node, + "nub applied an exclusion node did not, on top of a user-supplied \ + --test-coverage-exclude" + ); +} diff --git a/crates/nub-core/src/node/flags.rs b/crates/nub-core/src/node/flags.rs index ae158396e..e60ab1fd4 100644 --- a/crates/nub-core/src/node/flags.rs +++ b/crates/nub-core/src/node/flags.rs @@ -305,6 +305,31 @@ pub(crate) fn test_coverage_exclude_supported(node_version: &NodeVersion) -> boo *node_version >= MIN_TEST_COVERAGE_EXCLUDE } +/// Node started excluding the user's own test files from an +/// `--experimental-test-coverage` report by default in **23.5.0** — commit +/// `ea9a675f56`, "test_runner: exclude test files from coverage by default" +/// (nodejs/node#56060), which added the `coverageExcludeGlobs.length === 0` +/// fallback to `kDefaultPattern` in `lib/internal/test_runner/utils.js`. +/// +/// It was **never backported to 22.x**, so this floor does NOT coincide with +/// `MIN_TEST_COVERAGE_EXCLUDE` (22.5.0): on 22.5–22.x and 23.0–23.4 the flag +/// exists while the default exclusion does not. That gap is the whole reason this +/// is a separate gate. nub pairs Node's default pattern with its own runtime +/// exclude to stop the exclude from switching that default off — so on a Node that +/// has no such default, injecting the pattern would EXCLUDE test files stock Node +/// includes, breaking parity in the opposite direction. +/// +/// Verified empirically on the logic.js/logic.test.js fixture: 18.19.0, 22.15.0, +/// 22.16.0, 22.23.1, 22.23.2, 23.0.0, 23.3.0 and 23.4.0 all report the test file; +/// 23.5.0, 23.6.0, 23.11.0, 24.0.0, 24.17.0, 25.9.0, 26.3.0 and 26.7.0 all exclude +/// it. 22.23.2 is the newest 22.x published, which is what rules out a backport. +const MIN_TEST_COVERAGE_DEFAULT_EXCLUSION: NodeVersion = NodeVersion::new(23, 5, 0); + +/// Whether the target Node applies its own default test-file coverage exclusion. +pub(crate) fn test_coverage_default_exclusion_applied(node_version: &NodeVersion) -> bool { + *node_version >= MIN_TEST_COVERAGE_DEFAULT_EXCLUSION +} + /// Compute the flags Nub should inject for the given Node version, /// after subtracting any user opt-outs from argv and NODE_OPTIONS. /// @@ -1066,6 +1091,31 @@ mod tests { assert!(test_coverage_exclude_supported(&v(24, 0, 0))); } + #[test] + fn test_coverage_default_exclusion_gated_to_23_5_and_never_backported_to_22() { + // The two coverage gates do NOT share a floor, and treating them as one is + // what breaks parity on 22.x: there the flag exists but Node has no default + // test-file exclusion, so pairing nub's runtime exclude with Node's default + // pattern would hide test files stock Node reports. 22.23.2 is the newest + // 22.x published and still has no default — the line never got the backport. + assert!(!test_coverage_default_exclusion_applied(&v(18, 19, 0))); + assert!(!test_coverage_default_exclusion_applied(&v(22, 15, 0))); + assert!(!test_coverage_default_exclusion_applied(&v(22, 23, 2))); + assert!(!test_coverage_default_exclusion_applied(&v(23, 4, 0))); + assert!(test_coverage_default_exclusion_applied(&v(23, 5, 0))); + assert!(test_coverage_default_exclusion_applied(&v(24, 0, 0))); + assert!(test_coverage_default_exclusion_applied(&v(26, 7, 0))); + + // The whole band between the two floors has the flag and not the default. + for version in [v(22, 5, 0), v(22, 23, 2), v(23, 4, 0)] { + assert!( + test_coverage_exclude_supported(&version) + && !test_coverage_default_exclusion_applied(&version), + "{version:?} must be inside the flag-without-default band" + ); + } + } + #[test] fn strips_gated_node_options_flag_below_floor_both_token_forms() { // Node 20.0 is below --experimental-webstorage (22.4), --disable-warning diff --git a/crates/nub-core/src/node/spawn.rs b/crates/nub-core/src/node/spawn.rs index f1e6aec3c..fb4a45081 100644 --- a/crates/nub-core/src/node/spawn.rs +++ b/crates/nub-core/src/node/spawn.rs @@ -22,6 +22,7 @@ use same_file::Handle as FileHandle; use super::discovery::ResolvedNode; use super::flags; +use super::version::NodeVersion; #[cfg(windows)] #[derive(Debug)] @@ -1093,7 +1094,8 @@ pub fn spawn_node(config: &SpawnConfig<'_>) -> Result { // duplicate is two independent exclude tokens (a harmless re-exclude), not a // space-joined single value like the preload/PnP `--require` above. if flags::test_coverage_exclude_supported(&config.node.version) { - if let Some(glob) = coverage_exclude_glob( + for glob in coverage_exclude_globs( + &config.node.version, config.user_args, node_options.as_deref(), preload.as_deref(), @@ -1301,6 +1303,20 @@ pub fn spawn_node(config: &SpawnConfig<'_>) -> Result { "--test-coverage-exclude={}", node_options_token(&format!("{}/**", runtime_dir.display())) )); + // …and Node's default test-file pattern beside it, because the exclude + // above is what turns that default off. See + // NODE_DEFAULT_COVERAGE_EXCLUDE, and user_supplied_coverage_exclude for + // why a user's own exclude suppresses this and what nub cannot see. + if should_restate_default_coverage_exclude( + &config.node.version, + config.user_args, + node_options.as_deref(), + ) { + node_opts_parts.push(format!( + "--test-coverage-exclude={}", + node_options_token(NODE_DEFAULT_COVERAGE_EXCLUDE) + )); + } } // Web Storage (mirrors the argv site above): always inject // `--experimental-webstorage` into NODE_OPTIONS on the flag-needed band @@ -2822,9 +2838,51 @@ fn coverage_active_for_cache( coverage_active(user_args, node_options) || node_v8_coverage.is_some_and(|v| !v.is_empty()) } -/// The `--test-coverage-exclude=` flag nub injects to keep its own preloaded -/// runtime modules out of the user's coverage report (R9), or `None` when coverage -/// isn't active or the runtime dir can't be resolved. The glob is keyed to the +/// Node's own default coverage exclusion, which it applies ONLY when no +/// `--test-coverage-exclude` is set at all: `kDefaultPattern` in +/// `lib/internal/test_runner/utils.js`, reached by the +/// `coverageExcludeGlobs.length === 0` fallback in `parseCommandLine`. nub's +/// runtime exclude below is itself a `--test-coverage-exclude`, so injecting it +/// silently switches that default OFF and folds the user's own `*.test.js` back +/// into their report. Where the target Node HAS that default — 23.5.0 and up, see +/// `should_restate_default_coverage_exclude` — every site that injects the runtime +/// exclude re-states the default beside it, so the union is Node's default behavior +/// plus nub's runtime exclusion. Below 23.5 Node applies no default at all, and the +/// runtime exclude goes out alone. +/// +/// The TypeScript extensions are stated unconditionally where Node appends them +/// only under `--strip-types` (default-on since 22.18 / 23.6). nub transpiles TS +/// on every supported Node regardless of that flag, so under nub a `*.test.ts` is +/// a test file on the whole supported band, not just where stock Node could have +/// loaded one. +const NODE_DEFAULT_COVERAGE_EXCLUDE: &str = + "**/{test,test/**/*,test-*,*[._-]test}.{js,mjs,cjs,ts,mts,cts}"; + +/// Whether the USER asked for a specific coverage exclusion, on argv or through an +/// inherited NODE_OPTIONS. Node drops its default pattern the moment any exclude is +/// present, so nub must drop it too — re-adding it would hide files the user asked +/// to see. An ancestor nub's own tokens need no filtering out here: that +/// NODE_OPTIONS already carries the default pattern the ancestor paired with them, +/// and it is forwarded to this child verbatim. +/// +/// KNOWN LIMIT: a grandchild spawned by absolute `process.execPath` never passes +/// through nub, so its own `--test-coverage-exclude` is invisible here while nub's +/// NODE_OPTIONS still reaches it. Such a run gets the default pattern it meant to +/// override. Restoring the default for the far commoner grandchild that overrides +/// nothing is the deliberate trade; nub has no channel that carries one without +/// the other. +fn user_supplied_coverage_exclude(user_args: &[String], node_options: Option<&str>) -> bool { + let is_exclude = |token: &str| { + token == "--test-coverage-exclude" || token.starts_with("--test-coverage-exclude=") + }; + user_args.iter().any(|arg| is_exclude(arg)) + || node_options.is_some_and(|opts| opts.split_whitespace().any(is_exclude)) +} + +/// The `--test-coverage-exclude=` flags nub injects on argv when coverage is +/// active — nub's own preloaded runtime modules (R9), plus Node's default test-file +/// pattern that injecting them would otherwise disable. Empty when coverage isn't +/// active or the runtime dir can't be resolved. The runtime glob is keyed to the /// ABSOLUTE directory holding the injected preload — the same dir `find_preload` /// returns the preload from — so it can never accidentally match a user's own /// `runtime/` directory the way a relative `**/runtime/**` would. @@ -2835,19 +2893,43 @@ fn coverage_active_for_cache( /// denominator a hair. This is a stock-Node quirk of `--test-coverage-exclude`, /// NOT something nub introduces; a future reader comparing nub's aggregate to a /// hand-computed one should not be surprised by a fractional branch-% difference. -fn coverage_exclude_glob( +fn coverage_exclude_globs( + node_version: &NodeVersion, user_args: &[String], node_options: Option<&str>, preload: Option<&str>, -) -> Option { +) -> Vec { if !coverage_active(user_args, node_options) { - return None; + return Vec::new(); } - let runtime_dir = Path::new(preload?).parent()?; - Some(format!( + let Some(runtime_dir) = preload.map(Path::new).and_then(Path::parent) else { + return Vec::new(); + }; + let mut globs = vec![format!( "--test-coverage-exclude={}/**", runtime_dir.display() - )) + )]; + if should_restate_default_coverage_exclude(node_version, user_args, node_options) { + globs.push(format!( + "--test-coverage-exclude={NODE_DEFAULT_COVERAGE_EXCLUDE}" + )); + } + globs +} + +/// Whether nub must re-state Node's default coverage exclusion beside the runtime +/// exclude it is about to inject. Both conditions are load-bearing and neither +/// implies the other: the host Node must HAVE that default (23.5+, a strictly +/// higher floor than the flag's own 22.5 — see +/// `flags::test_coverage_default_exclusion_applied`), and the user must not have +/// supplied an exclude of their own, which turns the default off for stock node too. +fn should_restate_default_coverage_exclude( + node_version: &NodeVersion, + user_args: &[String], + node_options: Option<&str>, +) -> bool { + flags::test_coverage_default_exclusion_applied(node_version) + && !user_supplied_coverage_exclude(user_args, node_options) } /// True when `node_options` already carries OUR specific preload path — i.e. a @@ -5482,30 +5564,67 @@ mod tests { #[test] fn coverage_exclude_targets_absolute_runtime_dir_only_when_coverage_active() { let preload = "/opt/nub/runtime/preload.mjs"; + let runtime = "--test-coverage-exclude=/opt/nub/runtime/**"; + let default = format!("--test-coverage-exclude={NODE_DEFAULT_COVERAGE_EXCLUDE}"); + // 26.7 has Node's own default exclusion; 22.15 has the FLAG but no default. + let modern = NodeVersion::new(26, 7, 0); + let no_default = NodeVersion::new(22, 15, 0); // No coverage flag anywhere → no exclude injected. - assert!(coverage_exclude_glob(&[], None, Some(preload)).is_none()); + assert!(coverage_exclude_globs(&modern, &[], None, Some(preload)).is_empty()); // Coverage via argv → exclude keyed to the ABSOLUTE runtime dir (the - // preload's parent), with a trailing `/**` — not a broad `**/runtime/**`. + // preload's parent), with a trailing `/**` — not a broad `**/runtime/**` — + // PLUS Node's default test-file pattern, which the runtime exclude would + // otherwise disable. let argv = vec![ "--test".to_string(), "--experimental-test-coverage".to_string(), ]; assert_eq!( - coverage_exclude_glob(&argv, None, Some(preload)).as_deref(), - Some("--test-coverage-exclude=/opt/nub/runtime/**"), + coverage_exclude_globs(&modern, &argv, None, Some(preload)), + vec![runtime.to_string(), default.clone()], ); // Coverage via NODE_OPTIONS is detected the same way. assert_eq!( - coverage_exclude_glob(&[], Some("--experimental-test-coverage"), Some(preload)) - .as_deref(), - Some("--test-coverage-exclude=/opt/nub/runtime/**"), + coverage_exclude_globs( + &modern, + &[], + Some("--experimental-test-coverage"), + Some(preload) + ), + vec![runtime.to_string(), default.clone()], + ); + + // On a Node with no default exclusion of its own, re-stating the pattern + // would EXCLUDE test files stock node reports — the parity break in the + // opposite direction. Only the runtime exclude goes out there. + assert_eq!( + coverage_exclude_globs(&no_default, &argv, None, Some(preload)), + vec![runtime.to_string()], + ); + + // A user exclude turns Node's default off for stock node too, so nub must + // not re-add it — on either channel. + let mut user_argv = argv.clone(); + user_argv.push("--test-coverage-exclude=dist/**".to_string()); + assert_eq!( + coverage_exclude_globs(&modern, &user_argv, None, Some(preload)), + vec![runtime.to_string()], + ); + assert_eq!( + coverage_exclude_globs( + &modern, + &argv, + Some("--test-coverage-exclude=dist/**"), + Some(preload) + ), + vec![runtime.to_string()], ); // Coverage active but no resolvable preload → nothing to exclude. - assert!(coverage_exclude_glob(&argv, None, None).is_none()); + assert!(coverage_exclude_globs(&modern, &argv, None, None).is_empty()); } #[test]