Keep test children off production telemetry and fail e2e legs that run no tests

.cargo/config.toml

@@ -32,3 +32,10 @@ SOCKET_NO_CONFIG = "1"
 # tests opt back in with SOCKET_NO_UPDATE_CHECK=0 plus a wiremock
 # SOCKET_UPDATE_BASE_URL.
 SOCKET_NO_UPDATE_CHECK = "1"
+
+# No test child (and no dev `cargo run`) may POST telemetry to the real
+# public proxy: an unauthenticated run sends its events to
+# patches-api.socket.dev, and most spawning suites point no proxy URL at a
+# mock. Suites that assert telemetry events opt back in with
+# SOCKET_TELEMETRY_DISABLED=0 (or remove it) plus a wiremock endpoint.
+SOCKET_TELEMETRY_DISABLED = "1"

.github/workflows/ci.yml

@@ -990,14 +990,14 @@ jobs:
           # setup-node step, no version change.
           #
           # `npm_required` turns the npm suites' "npm not installed" soft-skip
-          # into a hard failure. Not on Windows: `Command::new("npm")` cannot
-          # resolve `npm.cmd` there, so that leg still skips (a known gap; see
-          # docs/testing/npm-compatibility.md).
+          # into a hard failure, on every OS: the suite resolves npm through
+          # PATHEXT, so Windows finds the `npm.cmd` shim instead of skipping.
           - os: ubuntu-latest
             suite: e2e_redirect_npm_build
             npm_required: '1'
           - os: windows-latest
             suite: e2e_redirect_npm_build
+            npm_required: '1'
           - os: ubuntu-latest
             suite: e2e_redirect_rush_sim
           - os: windows-latest
@@ -1252,6 +1252,7 @@ jobs:
     env:
       SOCKET_NO_CONFIG: '1'
       SOCKET_NO_UPDATE_CHECK: '1'
+      SOCKET_TELEMETRY_DISABLED: '1'
     # e2e-full runs these same steps over its rows.
     steps: &e2e-steps
       - name: Checkout
@@ -1617,14 +1618,15 @@ jobs:
           SOCKET_PATCH_SBT_E2E_VERSION: ${{ matrix.sbt }}
           E2E_SUITE: ${{ matrix.suite }}
           E2E_TEST_FILTER: ${{ matrix.test_filter || '--ignored' }}
-          E2E_JVM_TOOL: ${{ matrix.jvm_tool }}
           E2E_ALLOW_EMPTY: ${{ matrix.allow_empty }}
         shell: bash
         # Runs from the package root with CARGO_MANIFEST_DIR set, as
         # `cargo test` would. `suite` may name several binaries; an
         # `allow_empty` row skips the ones whose test file has not landed.
-        # Every suite a Gradle leg runs must run at least one test (per
-        # suite, so one suite's tests never hide another's empty filter).
+        # Every suite a leg runs must run at least one test (per suite, so
+        # one suite's tests never hide another's empty filter): a renamed
+        # module, a dropped `#[ignore]` under the default `--ignored`
+        # selector or a stale `test_filter` would otherwise pass as 0/0.
         run: |
           set -uo pipefail
           exe=''
@@ -1644,8 +1646,8 @@ jobs:
               continue
             fi
             passed=$(sed -n 's/^test result: .* \([0-9][0-9]*\) passed;.*/\1/p' "$log" | head -n 1)
-            if [ "$E2E_JVM_TOOL" = gradle ] && [ "${passed:-0}" = 0 ]; then
-              echo "::error::$suite ran no test in this Gradle leg ($E2E_TEST_FILTER)"
+            if [ "${passed:-0}" = 0 ]; then
+              echo "::error::$suite ran no test in this leg (filter: $E2E_TEST_FILTER). Fix the row's suite or test_filter so it selects the tests it is meant to run."
               status=1
             fi
           done
@@ -1750,6 +1752,7 @@ jobs:
     env:
       SOCKET_NO_CONFIG: '1'
       SOCKET_NO_UPDATE_CHECK: '1'
+      SOCKET_TELEMETRY_DISABLED: '1'
     # e2e-full runs these same steps over its rows.
     steps: *e2e-steps
 
@@ -1798,6 +1801,7 @@ jobs:
     env:
       SOCKET_NO_CONFIG: '1'
       SOCKET_NO_UPDATE_CHECK: '1'
+      SOCKET_TELEMETRY_DISABLED: '1'
     steps: *e2e-steps
 
   # ----------------------------------------------------------------------
@@ -2026,6 +2030,7 @@ jobs:
     env:
       SOCKET_NO_CONFIG: '1'
       SOCKET_NO_UPDATE_CHECK: '1'
+      SOCKET_TELEMETRY_DISABLED: '1'
     strategy:
       fail-fast: false
       matrix:
@@ -2098,6 +2103,7 @@ jobs:
     env:
       SOCKET_NO_CONFIG: '1'
       SOCKET_NO_UPDATE_CHECK: '1'
+      SOCKET_TELEMETRY_DISABLED: '1'
     strategy:
       fail-fast: false
       matrix:
@@ -2118,6 +2124,7 @@ jobs:
     env:
       SOCKET_NO_CONFIG: '1'
       SOCKET_NO_UPDATE_CHECK: '1'
+      SOCKET_TELEMETRY_DISABLED: '1'
     strategy:
       fail-fast: false
       matrix:

.github/workflows/composer-compatibility.yml

@@ -78,6 +78,7 @@ concurrency:
 env:
   SOCKET_NO_CONFIG: '1'
   SOCKET_NO_UPDATE_CHECK: '1'
+  SOCKET_TELEMETRY_DISABLED: '1'
 
 jobs:
   native:

.github/workflows/go-compatibility.yml

@@ -36,6 +36,7 @@ concurrency:
 env:
   SOCKET_NO_CONFIG: '1'
   SOCKET_NO_UPDATE_CHECK: '1'
+  SOCKET_TELEMETRY_DISABLED: '1'
 
 jobs:
   go:

.github/workflows/gradle-compatibility.yml

@@ -95,6 +95,7 @@ concurrency:
 env:
   SOCKET_NO_CONFIG: '1'
   SOCKET_NO_UPDATE_CHECK: '1'
+  SOCKET_TELEMETRY_DISABLED: '1'
 
 jobs:
   build:

.github/workflows/npm-compatibility.yml

@@ -119,6 +119,7 @@ jobs:
           SOCKET_PATCH_NPM_E2E_REQUIRED: '1'
           SOCKET_NO_CONFIG: '1'
           SOCKET_NO_UPDATE_CHECK: '1'
+          SOCKET_TELEMETRY_DISABLED: '1'
         run: |
           chmod +x target/debug/socket-patch target/debug/e2e_redirect_npm_build target/debug/e2e_vendor_npm_build
           target/debug/e2e_redirect_npm_build --include-ignored --test-threads 4

.github/workflows/pdm-compatibility.yml

@@ -57,6 +57,7 @@ concurrency:
 env:
   SOCKET_NO_CONFIG: '1'
   SOCKET_NO_UPDATE_CHECK: '1'
+  SOCKET_TELEMETRY_DISABLED: '1'
   PDM_CHECK_UPDATE: 'false'
 
 jobs:

.github/workflows/pipenv-compatibility.yml

@@ -85,6 +85,7 @@ jobs:
         env:
           SOCKET_NO_CONFIG: '1'
           SOCKET_NO_UPDATE_CHECK: '1'
+          SOCKET_TELEMETRY_DISABLED: '1'
           # The runners carry whatever patch release uv ships; the harness
           # only needs one 3.8 and one 3.12.
           BACKTEST_PY38: '3.8'

.github/workflows/pnpm-compatibility.yml

@@ -120,6 +120,7 @@ jobs:
           SOCKET_PATCH_PNPM_E2E_REQUIRED: '1'
           SOCKET_NO_CONFIG: '1'
           SOCKET_NO_UPDATE_CHECK: '1'
+          SOCKET_TELEMETRY_DISABLED: '1'
         run: |
           chmod +x bin/socket-patch bin/pnpm-e2e bin/pnpm-vendor-e2e
           export SOCKET_PATCH_PNPM_E2E_SOCKET_BIN="$PWD/bin/socket-patch"

.github/workflows/poetry-compatibility.yml

@@ -50,6 +50,7 @@ concurrency:
 env:
   SOCKET_NO_CONFIG: '1'
   SOCKET_NO_UPDATE_CHECK: '1'
+  SOCKET_TELEMETRY_DISABLED: '1'
 
 jobs:
   build:

.github/workflows/sbt-compatibility.yml

@@ -66,6 +66,7 @@ concurrency:
 env:
   SOCKET_NO_CONFIG: '1'
   SOCKET_NO_UPDATE_CHECK: '1'
+  SOCKET_TELEMETRY_DISABLED: '1'
 
 jobs:
   image:

crates/socket-patch-cli/tests/cli_parse_apply.rs

@@ -8,17 +8,21 @@
 
 use std::path::PathBuf;
 
-use clap::Parser;
 use socket_patch_cli::commands::apply::ApplyArgs;
-use socket_patch_cli::{Cli, Commands};
+use socket_patch_cli::Commands;
+
+#[path = "common/hermetic.rs"]
+mod hermetic;
+
+use hermetic::try_parse;
 
 /// Parse `socket-patch apply <extra...>` and return the inner `ApplyArgs`.
 /// Panics if parsing fails or yields a non-`Apply` subcommand — tests for
-/// the failure path call `Cli::try_parse_from` directly.
+/// the failure path call [`try_parse`] directly.
 fn parse_apply(extra: &[&str]) -> ApplyArgs {
     let mut argv: Vec<&str> = vec!["socket-patch", "apply"];
     argv.extend_from_slice(extra);
-    let cli = Cli::try_parse_from(&argv).expect("parse");
+    let cli = try_parse(&argv).expect("parse");
     match cli.command {
         Commands::Apply(a) => a,
         _ => panic!("expected Apply"),
@@ -305,7 +309,7 @@ fn no_telemetry_long() {
 /// the trailing token is rejected as an unknown argument.
 #[test]
 fn bare_bool_does_not_consume_next_token() {
-    match Cli::try_parse_from(["socket-patch", "apply", "--force", "stray"]) {
+    match try_parse(&["socket-patch", "apply", "--force", "stray"]) {
         Ok(_) => panic!("`--force stray` must reject the stray positional"),
         Err(err) => assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument),
     }
@@ -563,7 +567,7 @@ fn download_mode_values_are_not_normalized() {
 /// expectation to assert an `InvalidValue` error.
 #[test]
 fn download_mode_invalid_value_is_only_caught_at_runtime() {
-    match Cli::try_parse_from(["socket-patch", "apply", "--download-mode", "totally-bogus"]) {
+    match try_parse(&["socket-patch", "apply", "--download-mode", "totally-bogus"]) {
         Ok(cli) => match cli.command {
             Commands::Apply(a) => assert_eq!(
                 a.common.download_mode, "totally-bogus",
@@ -588,7 +592,7 @@ fn download_mode_invalid_value_is_only_caught_at_runtime() {
 fn unknown_flag_fails_with_unknown_argument() {
     // `Cli` doesn't implement `Debug`, so we can't use `.expect_err()` —
     // match the Result by hand.
-    match Cli::try_parse_from(["socket-patch", "apply", "--unknown-flag"]) {
+    match try_parse(&["socket-patch", "apply", "--unknown-flag"]) {
         Ok(_) => panic!("--unknown-flag must be rejected"),
         Err(err) => {
             assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument);

crates/socket-patch-cli/tests/cli_parse_rollback.rs

@@ -7,15 +7,19 @@
 //! these tests is a breaking change and requires a MAJOR bump per
 //! `crates/socket-patch-cli/CLI_CONTRACT.md`.
 
-use clap::Parser;
 use socket_patch_cli::commands::rollback::RollbackArgs;
-use socket_patch_cli::{Cli, Commands};
+use socket_patch_cli::Commands;
 use std::path::PathBuf;
 
+#[path = "common/hermetic.rs"]
+mod hermetic;
+
+use hermetic::try_parse;
+
 fn parse_rollback(extra: &[&str]) -> RollbackArgs {
     let mut argv = vec!["socket-patch", "rollback"];
     argv.extend_from_slice(extra);
-    let cli = Cli::try_parse_from(&argv).expect("parse");
+    let cli = try_parse(&argv).expect("parse");
     match cli.command {
         Commands::Rollback(a) => a,
         _ => panic!("expected Rollback"),
@@ -402,7 +406,7 @@ fn preserve_state_does_not_consume_next_token() {
 
 #[test]
 fn unknown_flag_fails() {
-    let err = match Cli::try_parse_from(["socket-patch", "rollback", "--unknown-flag"]) {
+    let err = match try_parse(&["socket-patch", "rollback", "--unknown-flag"]) {
         Ok(_) => panic!("expected parse failure"),
         Err(e) => e,
     };
@@ -411,7 +415,7 @@ fn unknown_flag_fails() {
 
 #[test]
 fn removed_one_off_flag_is_unknown() {
-    let err = match Cli::try_parse_from(["socket-patch", "rollback", "--one-off"]) {
+    let err = match try_parse(&["socket-patch", "rollback", "--one-off"]) {
         Ok(_) => panic!("expected parse error for the v5-removed --one-off"),
         Err(e) => e,
     };

crates/socket-patch-cli/tests/common/hermetic.rs

@@ -48,9 +48,10 @@ const HOSTILE_SEEDS: &[(&str, &str)] = &[
 
 /// A `Command` for `bin` with the hermetic `SOCKET_*` environment: the
 /// hostile seeds scrubbed, every other ambient `SOCKET_*` removed (removing
-/// `SOCKET_API_TOKEN` also forces the public proxy), and the two opt-outs
-/// forced on. Callers add args, cwd and their own env afterwards; caller env
-/// lands last, so explicit injections survive the scrub.
+/// `SOCKET_API_TOKEN` also forces the public proxy), and the three opt-outs
+/// (config layer, update notifier, telemetry) forced on. Callers add args,
+/// cwd and their own env afterwards; caller env lands last, so explicit
+/// injections survive the scrub.
 pub fn command(bin: &Path) -> Command {
     let mut cmd = Command::new(bin);
     for (k, v) in HOSTILE_SEEDS {
@@ -72,6 +73,12 @@ pub fn command(bin: &Path) -> Command {
     // this force-set is the layer that holds there. Notifier tests opt back
     // in via caller env (which lands last).
     cmd.env("SOCKET_NO_UPDATE_CHECK", "1");
+    // And for telemetry: an unauthenticated child POSTs its events to the
+    // real public proxy unless a suite points it at a mock. The workspace
+    // `[env]` default can be overridden by the developer's shell, so force
+    // it here too. Telemetry suites opt back in via caller env with
+    // `SOCKET_TELEMETRY_DISABLED=0` and a wiremock endpoint.
+    cmd.env("SOCKET_TELEMETRY_DISABLED", "1");
     cmd
 }
 
@@ -97,6 +104,35 @@ pub fn scrub_socket_vars(cmd: &mut Command) {
     }
 }
 
+/// The in-process half, for test binaries that only parse argv with clap:
+/// clap reads every env-bound `SOCKET_*` flag at parse time, so an ambient
+/// value (or a `.cargo/config.toml` `[env]` default such as
+/// `SOCKET_TELEMETRY_DISABLED=1`) changes what a parse yields. Removes every
+/// `SOCKET_*` var from this process, once; later calls wait for the first.
+/// Nothing is restored, so call it only from binaries where no test needs a
+/// `SOCKET_*` var, and call it before every parse so no parse reads the
+/// environment while it is being changed.
+pub fn scrub_process_socket_env() {
+    static ONCE: std::sync::Once = std::sync::Once::new();
+    ONCE.call_once(|| {
+        for (key, _) in std::env::vars_os() {
+            if key.to_string_lossy().starts_with("SOCKET_") {
+                std::env::remove_var(&key);
+            }
+        }
+    });
+}
+
+/// `Cli::try_parse_from` with the ambient `SOCKET_*` environment removed
+/// first (see [`scrub_process_socket_env`]): clap reads the env-bound flags
+/// at parse time, so without it the workspace `SOCKET_TELEMETRY_DISABLED=1`
+/// default (or a developer's shell) would set `--no-telemetry` and friends
+/// in every parse.
+pub fn try_parse(argv: &[&str]) -> Result<socket_patch_cli::Cli, clap::Error> {
+    scrub_process_socket_env();
+    <socket_patch_cli::Cli as clap::Parser>::try_parse_from(argv)
+}
+
 /// Opt-in scrubs for the ambient config of the tools a suite drives.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Extra {

crates/socket-patch-cli/tests/npm_e2e_common/mod.rs

@@ -62,7 +62,16 @@ fn env_set(name: &str) -> Option<OsString> {
 
 /// The npm executable under test.
 pub fn npm_program() -> OsString {
-    env_set(BIN_ENV).unwrap_or_else(|| "npm".into())
+    env_set(BIN_ENV).unwrap_or_else(path_npm)
+}
+
+/// `npm` from `PATH`, resolved the way the CLI resolves its own tools
+/// (`PATHEXT` on Windows). A bare `Command::new("npm")` cannot spawn the
+/// `npm.cmd` shim Windows installs, so without this every Windows run
+/// skipped as "npm not installed".
+fn path_npm() -> OsString {
+    socket_patch_core::utils::process::resolve_tool("npm")
+        .map_or_else(|| "npm".into(), PathBuf::into_os_string)
 }
 
 /// `SOCKET_PATCH_NPM_E2E_REQUIRED` is set (CI matrix legs).
@@ -137,7 +146,7 @@ pub fn npm_version() -> Option<String> {
 /// `SOCKET_PATCH_NPM_E2E_LOCK_WRITER_BIN`, else `npm` from `PATH` when it
 /// is not the npm under test and reports major >= 7.
 pub fn modern_npm_writer() -> Option<OsString> {
-    let candidate = env_set("SOCKET_PATCH_NPM_E2E_LOCK_WRITER_BIN").unwrap_or_else(|| "npm".into());
+    let candidate = env_set("SOCKET_PATCH_NPM_E2E_LOCK_WRITER_BIN").unwrap_or_else(path_npm);
     let probe = tempfile::tempdir().unwrap();
     let out = npm_command_for(&candidate, probe.path())
         .arg("--version")