From 6a9851822c6dc3fed1741d794c8cbda60d8aba3a Mon Sep 17 00:00:00 2001 From: konstin Date: Mon, 16 Jun 2025 18:27:38 +0200 Subject: [PATCH 1/4] Clear the env vars for tests Currently, it's possible to break our test suite by having an env var set that influences uv, either a `UV_*` var, or something more generic such as the XDG env vars. We previously fixed them env-var-by-env-var as we discovered. By clearing the env var for subcommands entirely, we can invert this to only include the env vars we care about. Notable limitations are that this only affects tests that use `TestContext::add_shared_env` (default, can be opted-out) and we still pass a (modified) `PATH`. --- crates/uv/tests/it/common/mod.rs | 19 ++++++++++++------- crates/uv/tests/it/pip_install.rs | 2 ++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/crates/uv/tests/it/common/mod.rs b/crates/uv/tests/it/common/mod.rs index f997561a9..74d16ec38 100644 --- a/crates/uv/tests/it/common/mod.rs +++ b/crates/uv/tests/it/common/mod.rs @@ -614,7 +614,7 @@ impl TestContext { "Activate with: source $1/[BIN]/activate".to_string(), )); filters.push(( - r"Activate with: source (.*)/bin/activate(?:\.\w+)?".to_string(), + r"Activate with: source (.*)/bin/activate".to_string(), "Activate with: source $1/[BIN]/activate".to_string(), )); @@ -725,8 +725,6 @@ impl TestContext { .unwrap(); command - // When running the tests in a venv, ignore that venv, otherwise we'll capture warnings. - .env_remove(EnvVars::VIRTUAL_ENV) // Disable wrapping of uv output for readability / determinism in snapshots. .env(EnvVars::UV_NO_WRAP, "1") // While we disable wrapping in uv above, invoked tools may still wrap their output so @@ -744,12 +742,14 @@ impl TestContext { // Since downloads, fetches and builds run in parallel, their message output order is // non-deterministic, so can't capture them in test output. .env(EnvVars::UV_TEST_NO_CLI_PROGRESS, "1") - .env_remove(EnvVars::UV_CACHE_DIR) - .env_remove(EnvVars::UV_TOOL_BIN_DIR) - .env_remove(EnvVars::XDG_CONFIG_HOME) - .env_remove(EnvVars::XDG_DATA_HOME) .current_dir(self.temp_dir.path()); + // For Unix, we pretend the tests run in a bash for the activate hint, for Windows, shell + // detection assumes PowerShell if `PROMPT` is not set. + if cfg!(unix) { + command.env("SHELL", "/bin/bash"); + } + for (key, value) in &self.extra_env { command.env(key, value); } @@ -1084,6 +1084,9 @@ impl TestContext { command } + /// The path to the Python interpreter in the venv. + /// + /// Don't use this for `Command::new`, use `Self::python_command` instead. pub fn interpreter(&self) -> PathBuf { let venv = &self.venv; if cfg!(unix) { @@ -1307,6 +1310,7 @@ impl TestContext { /// all tests, but with the given binary. fn new_command_with(&self, bin: &Path) -> Command { let mut command = Command::new(bin); + command.env_clear(); // I believe the intent of all tests is that they are run outside the // context of an existing git repository. And when they aren't, state // from the parent git repository can bleed into the behavior of `uv @@ -1385,6 +1389,7 @@ pub fn get_python(version: &PythonVersion) -> PathBuf { /// Create a virtual environment at the given path. pub fn create_venv_from_executable>(path: P, cache_dir: &ChildPath, python: &Path) { assert_cmd::Command::new(get_bin()) + .env_clear() .arg("venv") .arg(path.as_ref().as_os_str()) .arg("--cache-dir") diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index e0876b23c..858996981 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -5753,6 +5753,8 @@ fn deptry_gitignore() { uv_snapshot!(context.filters(), context.pip_install() .arg(format!("deptry_reproducer @ {}", source_dist_dir.join("deptry_reproducer-0.1.0.tar.gz").simplified_display())) .arg("--strict") + // Set by the test harness, needed by the test + .env("RUSTUP_TOOLCHAIN", std::env::var("RUSTUP_TOOLCHAIN").unwrap()) .current_dir(source_dist_dir), @r###" success: true exit_code: 0 From 10ca672ddc7594f6e02793e1a47752be76afc0b9 Mon Sep 17 00:00:00 2001 From: konstin Date: Wed, 18 Jun 2025 10:12:50 +0200 Subject: [PATCH 2/4] Set SYSTEMROOT for windows --- crates/uv/tests/it/common/mod.rs | 8 ++++++++ crates/uv/tests/it/show_settings.rs | 1 + 2 files changed, 9 insertions(+) diff --git a/crates/uv/tests/it/common/mod.rs b/crates/uv/tests/it/common/mod.rs index 74d16ec38..68c2ee355 100644 --- a/crates/uv/tests/it/common/mod.rs +++ b/crates/uv/tests/it/common/mod.rs @@ -750,6 +750,14 @@ impl TestContext { command.env("SHELL", "/bin/bash"); } + // Without `SYSTEMROOT`, Windows can't resolve DNS, plus proxy settings in case they are needed. + let env_vars = ["SYSTEMROOT", "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"]; + for env_var in env_vars { + if let Some(system_root) = env::var_os(env_var) { + command.env(env_var, system_root); + } + } + for (key, value) in &self.extra_env { command.env(key, value); } diff --git a/crates/uv/tests/it/show_settings.rs b/crates/uv/tests/it/show_settings.rs index 7635bd523..aea48c156 100644 --- a/crates/uv/tests/it/show_settings.rs +++ b/crates/uv/tests/it/show_settings.rs @@ -1,3 +1,4 @@ +use std::env; use std::path::Path; use std::process::Command; From c702afaa954b4f19c054f235a74a337fa5dc9288 Mon Sep 17 00:00:00 2001 From: konstin Date: Wed, 18 Jun 2025 16:19:52 +0200 Subject: [PATCH 3/4] Just use All The Env Vars --- crates/uv/tests/it/common/mod.rs | 121 ++++++++++++++++++++++--------- 1 file changed, 85 insertions(+), 36 deletions(-) diff --git a/crates/uv/tests/it/common/mod.rs b/crates/uv/tests/it/common/mod.rs index 68c2ee355..12a241d80 100644 --- a/crates/uv/tests/it/common/mod.rs +++ b/crates/uv/tests/it/common/mod.rs @@ -744,20 +744,6 @@ impl TestContext { .env(EnvVars::UV_TEST_NO_CLI_PROGRESS, "1") .current_dir(self.temp_dir.path()); - // For Unix, we pretend the tests run in a bash for the activate hint, for Windows, shell - // detection assumes PowerShell if `PROMPT` is not set. - if cfg!(unix) { - command.env("SHELL", "/bin/bash"); - } - - // Without `SYSTEMROOT`, Windows can't resolve DNS, plus proxy settings in case they are needed. - let env_vars = ["SYSTEMROOT", "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"]; - for env_var in env_vars { - if let Some(system_root) = env::var_os(env_var) { - command.env(env_var, system_root); - } - } - for (key, value) in &self.extra_env { command.env(key, value); } @@ -765,11 +751,6 @@ impl TestContext { if activate_venv { command.env(EnvVars::VIRTUAL_ENV, self.venv.as_os_str()); } - - if cfg!(unix) { - // Avoid locale issues in tests - command.env(EnvVars::LC_ALL, "C"); - } } /// Create a `pip compile` command for testing. @@ -1251,7 +1232,17 @@ impl TestContext { .as_ref() .expect("A Python version must be provided to create a test virtual environment"), ); - create_venv_from_executable(&self.venv, &self.cache_dir, &executable); + assert_cmd::Command::from(self.new_command()) + .arg("venv") + .arg(self.venv.as_ref().as_os_str()) + .arg("--cache-dir") + .arg(self.cache_dir.path()) + .arg("--python") + .arg(executable) + .current_dir(self.venv.as_ref().parent().unwrap()) + .assert() + .success(); + ChildPath::new(self.venv.as_ref()).assert(predicate::path::is_dir()); } /// Copies the files from the ecosystem project given into this text @@ -1316,9 +1307,13 @@ impl TestContext { /// Creates a new `Command` that is intended to be suitable for use in /// all tests, but with the given binary. + /// + /// The command contains only essential env vars to avoid the tests getting + /// influenced by host `UV_*`, XDG, or other environment variables. fn new_command_with(&self, bin: &Path) -> Command { let mut command = Command::new(bin); command.env_clear(); + // I believe the intent of all tests is that they are run outside the // context of an existing git repository. And when they aren't, state // from the parent git repository can bleed into the behavior of `uv @@ -1333,6 +1328,76 @@ impl TestContext { // impact it, since it only prevents git from discovering repositories // at or above the root. command.env(EnvVars::GIT_CEILING_DIRECTORIES, self.root.path()); + + if cfg!(unix) { + // For Unix, we pretend the tests run in a bash for the activate hint, for Windows, shell + // detection assumes PowerShell if `PROMPT` is not set. + command.env("SHELL", "/bin/bash"); + + // Avoid locale issues in tests + command.env(EnvVars::LC_ALL, "C"); + } + + /// Standard Windows environment variables that CLI applications can rely on + /// + /// Sources: + /// - https://ss64.com/nt/syntax-variables.html + /// - https://learn.microsoft.com/en-us/windows/win32/procthread/environment-variables + /// - https://en.wikipedia.org/wiki/Environment_variable + /// - https://www.elevenforum.com/t/complete-list-of-environment-variables-in-windows-11.11212/ + let env_vars = &[ + "ALLUSERSPROFILE", // C:\ProgramData + "APPDATA", // C:\Users\{username}\AppData\Roaming + "CD", // Current directory (dynamic, cmd.exe only) + "CMDCMDLINE", // Command line that started cmd.exe (dynamic) + "CMDEXTVERSION", // Command extensions version (dynamic) + "COMPUTERNAME", // Computer name + "COMSPEC", // C:\Windows\system32\cmd.exe + "CommonProgramFiles", // C:\Program Files\Common Files + "CommonProgramFiles(x86)", // C:\Program Files (x86)\Common Files (64-bit only) + "CommonProgramW6432", // C:\Program Files\Common Files (64-bit only) + "DATE", // Current date (dynamic, cmd.exe only) + "DIRCMD", // Directory listing format + "DriverData", // C:\Windows\System32\Drivers\DriverData + "ERRORLEVEL", // Exit code of last command (dynamic) + "HOMEDRIVE", // C: + "HOMEPATH", // \Users\{username} + "LOCALAPPDATA", // C:\Users\{username}\AppData\Local + "LOGONSERVER", // \\{servername} (volatile) + "NUMBER_OF_PROCESSORS", // Number of processors + "OS", // Windows_NT + "PATH", // Executable search paths + "PATHEXT", // Executable file extensions + "PROCESSOR_ARCHITECTURE", // AMD64, x86, etc. + "PROCESSOR_IDENTIFIER", // Processor description + "PROCESSOR_LEVEL", // Processor level + "PROCESSOR_REVISION", // Processor revision + "PROMPT", // Command prompt format + "PSModulePath", // PowerShell module paths + "PUBLIC", // C:\Users\Public + "ProgramData", // C:\ProgramData + "ProgramFiles", // C:\Program Files + "ProgramFiles(x86)", // C:\Program Files (x86) (64-bit only) + "ProgramW6432", // C:\Program Files (64-bit only) + "RANDOM", // Random 0-32767 (dynamic, cmd.exe only) + "SystemDrive", // C: + "SystemRoot", // C:\Windows + "SYSTEMROOT", // C:\Windows + "TEMP", // C:\Users\{username}\AppData\Local\Temp + "TIME", // Current time (dynamic, cmd.exe only) + "TMP", // Same as TEMP + "USERDOMAIN", // User domain (volatile) + "USERDOMAIN_ROAMINGPROFILE", // User domain for roaming profile (volatile) + "USERNAME", // Current username + "USERPROFILE", // C:\Users\{username} + "windir", // C:\Windows (same as SystemRoot) + ]; + for env_var in env_vars { + if let Some(system_root) = env::var_os(env_var) { + command.env(env_var, system_root); + } + } + command } } @@ -1394,22 +1459,6 @@ pub fn get_python(version: &PythonVersion) -> PathBuf { .unwrap_or(PathBuf::from(version.to_string())) } -/// Create a virtual environment at the given path. -pub fn create_venv_from_executable>(path: P, cache_dir: &ChildPath, python: &Path) { - assert_cmd::Command::new(get_bin()) - .env_clear() - .arg("venv") - .arg(path.as_ref().as_os_str()) - .arg("--cache-dir") - .arg(cache_dir.path()) - .arg("--python") - .arg(python) - .current_dir(path.as_ref().parent().unwrap()) - .assert() - .success(); - ChildPath::new(path.as_ref()).assert(predicate::path::is_dir()); -} - /// Returns the uv binary that cargo built before launching the tests. /// /// From 59584b58b1317b50ed045f3120d9345c9602d66e Mon Sep 17 00:00:00 2001 From: konstin Date: Wed, 18 Jun 2025 16:41:42 +0200 Subject: [PATCH 4/4] . --- crates/uv/tests/it/common/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/uv/tests/it/common/mod.rs b/crates/uv/tests/it/common/mod.rs index 12a241d80..47712e73c 100644 --- a/crates/uv/tests/it/common/mod.rs +++ b/crates/uv/tests/it/common/mod.rs @@ -1338,13 +1338,13 @@ impl TestContext { command.env(EnvVars::LC_ALL, "C"); } - /// Standard Windows environment variables that CLI applications can rely on - /// - /// Sources: - /// - https://ss64.com/nt/syntax-variables.html - /// - https://learn.microsoft.com/en-us/windows/win32/procthread/environment-variables - /// - https://en.wikipedia.org/wiki/Environment_variable - /// - https://www.elevenforum.com/t/complete-list-of-environment-variables-in-windows-11.11212/ + // Standard Windows environment variables + // + // Sources: + // - https://ss64.com/nt/syntax-variables.html + // - https://learn.microsoft.com/en-us/windows/win32/procthread/environment-variables + // - https://en.wikipedia.org/wiki/Environment_variable + // - https://www.elevenforum.com/t/complete-list-of-environment-variables-in-windows-11.11212/ let env_vars = &[ "ALLUSERSPROFILE", // C:\ProgramData "APPDATA", // C:\Users\{username}\AppData\Roaming