Skip to content

Commit 31b91ca

Browse files
committed
Run tests/common selftests in one binary
The oracle, cache_env and jvm_env selftests (20 tests) sat ungated in tests/common, so every one of the 118 integration binaries that declares `mod common` compiled and re-ran them, and the 25 that include cache_env.rs directly ran its 10 again: about 2,600 redundant test runs per `cargo test`, each failure reported once per binary. Move the three modules verbatim into tests/common_selftests.rs, which includes common/mod.rs once. The four private cache_env items they use become pub(crate). The envelope re-exports in common/mod.rs were only used everywhere because of the oracle selftest; allow unused_imports there so includers that never read envelopes don't warn. Drop the comments claiming integration crates lack cfg(test); they do set it, so the gate would not have helped either. Fixes #1957. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011f9aePuuKNcXTPE2Z4nq1H
1 parent 7bde3ba commit 31b91ca

6 files changed

Lines changed: 656 additions & 648 deletions

File tree

‎crates/socket-patch-cli/tests/common/bundler_e2e.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,7 @@ pub fn gate(
164164
Some(bundler)
165165
}
166166

167-
/// Runs in every suite that includes this module (no `#[cfg(test)]`: an
168-
/// integration crate does not set it, so the gate would compile the test
169-
/// out).
167+
/// Runs in every suite that includes this module.
170168
mod bundler_e2e_selftest {
171169
use super::*;
172170

‎crates/socket-patch-cli/tests/common/cache_env.rs‎

Lines changed: 8 additions & 288 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ use std::process::Command;
7575
/// Each entry is seeded only when the variable is not already set and the
7676
/// default directory actually exists, which makes it a no-op on machines
7777
/// (and CI runners) that do not use the version manager in question.
78-
const TOOLCHAIN_ROOTS: &[(&str, &str)] = &[
78+
pub(crate) const TOOLCHAIN_ROOTS: &[(&str, &str)] = &[
7979
("RUSTUP_HOME", ".rustup"),
8080
("RBENV_ROOT", ".rbenv"),
8181
("PYENV_ROOT", ".pyenv"),
@@ -299,7 +299,7 @@ pub fn scrub_ambient_bun_tool_env(cmd: &mut Command) {
299299
}
300300

301301
/// The names of the PARENT environment, read once.
302-
fn parent_var_names() -> Vec<OsString> {
302+
pub(crate) fn parent_var_names() -> Vec<OsString> {
303303
std::env::vars_os().map(|(k, _)| k).collect()
304304
}
305305

@@ -308,7 +308,11 @@ fn parent_var_names() -> Vec<OsString> {
308308
/// result against the same snapshot of the parent env the scrub saw:
309309
/// in-process tests in the same binary mirror flags into the env
310310
/// (`--offline` sets `SOCKET_OFFLINE`), so two separate reads race.
311-
fn scrub_parent_vars(cmd: &mut Command, parent: Vec<OsString>, ambient: fn(&str) -> bool) {
311+
pub(crate) fn scrub_parent_vars(
312+
cmd: &mut Command,
313+
parent: Vec<OsString>,
314+
ambient: fn(&str) -> bool,
315+
) {
312316
for k in parent {
313317
if ambient(&k.to_string_lossy()) {
314318
cmd.env_remove(&k);
@@ -345,7 +349,7 @@ pub fn scrub_ambient_vlt_env(cmd: &mut Command) {
345349
}
346350

347351
/// [`is_ambient_vlt_var`] minus `SOCKET_*`.
348-
fn is_ambient_vlt_tool_var(name: &str) -> bool {
352+
pub(crate) fn is_ambient_vlt_tool_var(name: &str) -> bool {
349353
!name.starts_with("SOCKET_") && is_ambient_vlt_var(name)
350354
}
351355

@@ -355,287 +359,3 @@ fn is_ambient_vlt_tool_var(name: &str) -> bool {
355359
pub fn scrub_ambient_vlt_tool_env(cmd: &mut Command) {
356360
scrub_parent_vars(cmd, parent_var_names(), is_ambient_vlt_tool_var);
357361
}
358-
359-
// ── Self-tests ────────────────────────────────────────────────────────
360-
//
361-
// Integration-test crates do not get `cfg(test)`, so — exactly as in
362-
// `common/mod.rs` — these must stay ungated to run at all. They are pure
363-
// env/path arithmetic, so they cost nothing in the binaries that pick this
364-
// module up.
365-
mod cache_env_selftests {
366-
use super::*;
367-
368-
/// The names the bun suites' hermeticity depends on — the registry
369-
/// overrides that reproduced the false negative, bun's auth and cache
370-
/// knobs, the CLI's own surface — are covered; the hermetic switch, the
371-
/// toolchain vars and look-alike prefixes (`BUNDLE_*`) survive.
372-
#[test]
373-
fn ambient_bun_scrub_covers_the_registry_and_config_overrides() {
374-
for name in [
375-
"BUN_CONFIG_REGISTRY",
376-
"BUN_CONFIG_TOKEN",
377-
"BUN_INSTALL",
378-
"BUN_INSTALL_CACHE_DIR",
379-
"BUN_RUNTIME_TRANSPILER_CACHE_PATH",
380-
"npm_config_registry",
381-
"NPM_CONFIG_REGISTRY",
382-
"Npm_Config_Registry",
383-
"npm_config__auth",
384-
"npm_config_cache",
385-
"SOCKET_API_TOKEN",
386-
"SOCKET_DRY_RUN",
387-
"VIRTUAL_ENV",
388-
] {
389-
assert!(is_ambient_bun_var(name), "{name} must be scrubbed");
390-
}
391-
for name in [
392-
"SOCKET_NO_CONFIG",
393-
"PATH",
394-
"HOME",
395-
"BUNDLE_PATH",
396-
"BUNDLE_USER_HOME",
397-
"npm_lifecycle_event",
398-
"XDG_CONFIG_HOME",
399-
"RUSTUP_HOME",
400-
] {
401-
assert!(!is_ambient_bun_var(name), "{name} must survive the scrub");
402-
}
403-
}
404-
405-
/// The scrub removes exactly the parent variables the predicate names:
406-
/// nothing else is touched, and no matching parent variable is missed.
407-
#[test]
408-
fn ambient_bun_scrub_removes_only_matching_parent_vars() {
409-
let parent = parent_var_names();
410-
let mut cmd = Command::new("true");
411-
scrub_parent_vars(&mut cmd, parent.clone(), is_ambient_bun_var);
412-
let removed: Vec<String> = cmd
413-
.get_envs()
414-
.filter(|(_, value)| value.is_none())
415-
.map(|(name, _)| name.to_string_lossy().into_owned())
416-
.collect();
417-
for name in &removed {
418-
assert!(
419-
is_ambient_bun_var(name),
420-
"{name} was removed but is not an ambient bun var"
421-
);
422-
}
423-
for name in &parent {
424-
let name = name.to_string_lossy();
425-
if is_ambient_bun_var(&name) {
426-
assert!(
427-
removed.iter().any(|r| *r == name),
428-
"{name} is set in the parent env but was not scrubbed"
429-
);
430-
}
431-
}
432-
assert!(
433-
cmd.get_envs().all(|(_, value)| value.is_none()),
434-
"the scrub only removes; it seeds nothing"
435-
);
436-
}
437-
438-
#[test]
439-
fn ambient_vlt_scrub_covers_vlt_config_and_keeps_the_harness_gates() {
440-
for name in [
441-
"VLT_REGISTRY",
442-
"VLT_REGISTRIES",
443-
"VLT_CACHE",
444-
"VLT_STORE_LINKER",
445-
"VLT_TELEMETRY",
446-
"npm_config_registry",
447-
"NPM_CONFIG_REGISTRY",
448-
"SOCKET_API_TOKEN",
449-
"SOCKET_NO_VLT_INSTALL_CLEANUP",
450-
"SOCKET_PATCH_BUN_E2E_JS",
451-
"VIRTUAL_ENV",
452-
] {
453-
assert!(is_ambient_vlt_var(name), "{name} must be scrubbed");
454-
}
455-
for name in [
456-
"SOCKET_NO_CONFIG",
457-
"SOCKET_PATCH_VLT_E2E_JS",
458-
"SOCKET_PATCH_VLT_E2E_VERSION",
459-
"SOCKET_PATCH_VLT_E2E_REQUIRED",
460-
"SOCKET_PATCH_VLT_E2E_STORE_LINKER",
461-
"PATH",
462-
"HOME",
463-
"XDG_CONFIG_HOME",
464-
"NODE_OPTIONS_X",
465-
] {
466-
assert!(!is_ambient_vlt_var(name), "{name} must survive the scrub");
467-
}
468-
}
469-
470-
/// The CLI-child variant keeps every `SOCKET_*`, so it never undoes
471-
/// the opt-outs `hermetic::command` forces on, and still strips the
472-
/// vlt, npm and Python config.
473-
#[test]
474-
fn ambient_vlt_tool_scrub_leaves_socket_vars_to_the_hermetic_builder() {
475-
for name in [
476-
"SOCKET_TELEMETRY_DISABLED",
477-
"SOCKET_NO_UPDATE_CHECK",
478-
"SOCKET_NO_CONFIG",
479-
"SOCKET_API_TOKEN",
480-
] {
481-
assert!(!is_ambient_vlt_tool_var(name), "{name} must survive");
482-
}
483-
for name in ["VLT_REGISTRY", "npm_config_registry", "VIRTUAL_ENV"] {
484-
assert!(is_ambient_vlt_tool_var(name), "{name} must be scrubbed");
485-
}
486-
}
487-
488-
#[test]
489-
fn ambient_vlt_scrub_removes_only_matching_parent_vars() {
490-
let parent = parent_var_names();
491-
let mut cmd = Command::new("true");
492-
scrub_parent_vars(&mut cmd, parent.clone(), is_ambient_vlt_var);
493-
let removed: Vec<String> = cmd
494-
.get_envs()
495-
.filter(|(_, value)| value.is_none())
496-
.map(|(name, _)| name.to_string_lossy().into_owned())
497-
.collect();
498-
for name in &removed {
499-
assert!(is_ambient_vlt_var(name), "{name} was removed");
500-
}
501-
for name in &parent {
502-
let name = name.to_string_lossy();
503-
if is_ambient_vlt_var(&name) {
504-
assert!(removed.iter().any(|r| *r == name), "{name} kept");
505-
}
506-
}
507-
}
508-
509-
/// The variables whose whole point is that they outrank `HOME`. A future
510-
/// edit that drops one would silently restore the leak this module
511-
/// exists to close, and nothing else in the suite would notice.
512-
const MUST_PIN: &[&str] = &[
513-
"HOME",
514-
"GOCACHE",
515-
"GOMODCACHE",
516-
"GOPATH",
517-
"COREPACK_HOME",
518-
"PNPM_HOME",
519-
"CARGO_HOME",
520-
"npm_config_cache",
521-
"YARN_CACHE_FOLDER",
522-
"BUN_INSTALL_CACHE_DIR",
523-
"PIP_CACHE_DIR",
524-
"UV_CACHE_DIR",
525-
"GEM_SPEC_CACHE",
526-
"NUGET_PACKAGES",
527-
];
528-
529-
#[test]
530-
fn every_leak_prone_var_is_pinned() {
531-
let pinned = overrides();
532-
for want in MUST_PIN {
533-
assert!(
534-
pinned.iter().any(|(var, _)| var == want),
535-
"{want} is no longer pinned by cache_env::overrides(); package-manager \
536-
caches will leak into the home directory of whoever runs the suite"
537-
);
538-
}
539-
}
540-
541-
#[test]
542-
fn every_override_lands_inside_the_sandbox() {
543-
let root = cache_root();
544-
for (var, path) in overrides() {
545-
assert!(
546-
path.starts_with(&root),
547-
"{var} points outside the cache sandbox: {} is not under {}",
548-
path.display(),
549-
root.display()
550-
);
551-
}
552-
}
553-
554-
#[test]
555-
fn no_override_points_into_the_real_home() {
556-
let Some(real) = real_home() else {
557-
return;
558-
};
559-
// A machine whose TMPDIR is itself inside the home directory has no
560-
// way to satisfy this; the sandbox is still a dedicated directory, so
561-
// skip rather than fail.
562-
if cache_root().starts_with(&real) {
563-
return;
564-
}
565-
for (var, path) in overrides() {
566-
assert!(
567-
!path.starts_with(&real),
568-
"{var} still resolves inside the real home: {}",
569-
path.display()
570-
);
571-
}
572-
}
573-
574-
#[test]
575-
fn isolate_applies_the_overrides_to_a_command() {
576-
let mut cmd = Command::new("true");
577-
isolate(&mut cmd);
578-
let applied: Vec<(String, Option<String>)> = cmd
579-
.get_envs()
580-
.map(|(k, v)| {
581-
(
582-
k.to_string_lossy().into_owned(),
583-
v.map(|v| v.to_string_lossy().into_owned()),
584-
)
585-
})
586-
.collect();
587-
for (var, path) in overrides() {
588-
let seen = applied
589-
.iter()
590-
.find(|(name, _)| name == var)
591-
.unwrap_or_else(|| panic!("isolate() did not set {var}"));
592-
assert_eq!(
593-
seen.1.as_deref(),
594-
Some(path.to_string_lossy().as_ref()),
595-
"isolate() set {var} to the wrong path"
596-
);
597-
}
598-
assert!(
599-
sandbox_home().is_dir(),
600-
"isolate() must create the sandbox home so tools that require \
601-
an existing $HOME can start"
602-
);
603-
}
604-
605-
#[test]
606-
fn toolchain_roots_are_only_seeded_when_they_exist() {
607-
// The preservation pass must never invent a path. Whatever it seeds
608-
// has to be a directory that is really there under the real home,
609-
// and it must leave a variable the caller already exported alone.
610-
let Some(real) = real_home() else {
611-
return;
612-
};
613-
let mut cmd = Command::new("true");
614-
isolate(&mut cmd);
615-
let applied: Vec<(String, Option<PathBuf>)> = cmd
616-
.get_envs()
617-
.map(|(k, v)| (k.to_string_lossy().into_owned(), v.map(PathBuf::from)))
618-
.collect();
619-
for (var, relative) in TOOLCHAIN_ROOTS {
620-
let Some((_, value)) = applied.iter().find(|(name, _)| name == var) else {
621-
continue;
622-
};
623-
if std::env::var_os(var).is_some() {
624-
// Already exported by the caller: inherited untouched, so it
625-
// must not appear in the command's explicit env at all.
626-
panic!("{var} was already set in the parent env; isolate() must not override it");
627-
}
628-
let value = value.as_ref().expect("seeded roots always have a value");
629-
assert_eq!(
630-
value,
631-
&real.join(relative),
632-
"{var} was seeded to something other than its default under the real home"
633-
);
634-
assert!(
635-
value.is_dir(),
636-
"{var} was seeded to a path that does not exist: {}",
637-
value.display()
638-
);
639-
}
640-
}
641-
}

0 commit comments

Comments
 (0)