From b4787bf4c00fa0c3253f5979330b37d41e740da7 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 2 Apr 2026 23:06:36 -0700 Subject: [PATCH] fix: changes to test that should help them pass on Windows under Bazel (#16662) https://github.com/openai/codex/pull/16460 was a large PR created by Codex to try to get the tests to pass under Bazel on Windows. Indeed, it successfully ran all of the tests under `//codex-rs/core:` with its changes to `codex-rs/core/`, though the full set of changes seems to be too broad. This PR tries to port the key changes, which are: - Under Bazel, the `USERNAME` environment variable is not guaranteed to be set on Windows, so for tests that need a non-empty env var as a convenient substitute for an env var containing an API key, just use `PATH`. Note that `PATH` is unlikely to contain characters that are not allowed in an HTTP header value. - Specify `"powershell.exe"` instead of just `"powershell"` in case the `PATHEXT` env var gets lost in the shuffle. --- codex-rs/core/tests/suite/client.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 898e37ee1..e7701abe7 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -196,7 +196,7 @@ $lines | Select-Object -Skip 1 | Set-Content -Path tokens.txt "#, )?; ( - "powershell".to_string(), + "powershell.exe".to_string(), vec![ "-NoProfile".to_string(), "-ExecutionPolicy".to_string(), @@ -2590,10 +2590,18 @@ async fn incomplete_response_emits_content_filter_error_message() -> anyhow::Res Ok(()) } +/// We try to avoid setting env vars in tests because std::env::set_var() is +/// process-wide and unsafe. Though for this test, we want to simulate the +/// presence of an environment variable that the provider will read for auth, so +/// we pick a commonly existing env var that is guaranteed to have a non-empty +/// value on both Windows and Unix. Note that this test must also work when run +/// under Bazel in CI, which uses a restricted environment, so PATH seems like +/// the safest choice. +const EXISTING_ENV_VAR_WITH_NON_EMPTY_VALUE: &str = "PATH"; + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn azure_overrides_assign_properties_used_for_responses_url() { skip_if_no_network!(); - let existing_env_var_with_random_value = if cfg!(windows) { "USERNAME" } else { "USER" }; // Mock server let server = MockServer::start().await; @@ -2611,11 +2619,11 @@ async fn azure_overrides_assign_properties_used_for_responses_url() { .and(path("/openai/responses")) .and(query_param("api-version", "2025-04-01-preview")) .and(header_regex("Custom-Header", "Value")) - .and(header_regex( + .and(header( "Authorization", format!( "Bearer {}", - std::env::var(existing_env_var_with_random_value).unwrap() + std::env::var(EXISTING_ENV_VAR_WITH_NON_EMPTY_VALUE).unwrap() ) .as_str(), )) @@ -2628,7 +2636,7 @@ async fn azure_overrides_assign_properties_used_for_responses_url() { name: "custom".to_string(), base_url: Some(format!("{}/openai", server.uri())), // Reuse the existing environment variable to avoid using unsafe code - env_key: Some(existing_env_var_with_random_value.to_string()), + env_key: Some(EXISTING_ENV_VAR_WITH_NON_EMPTY_VALUE.to_string()), experimental_bearer_token: None, auth: None, query_params: Some(std::collections::HashMap::from([( @@ -2679,7 +2687,6 @@ async fn azure_overrides_assign_properties_used_for_responses_url() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn env_var_overrides_loaded_auth() { skip_if_no_network!(); - let existing_env_var_with_random_value = if cfg!(windows) { "USERNAME" } else { "USER" }; // Mock server let server = MockServer::start().await; @@ -2697,11 +2704,11 @@ async fn env_var_overrides_loaded_auth() { .and(path("/openai/responses")) .and(query_param("api-version", "2025-04-01-preview")) .and(header_regex("Custom-Header", "Value")) - .and(header_regex( + .and(header( "Authorization", format!( "Bearer {}", - std::env::var(existing_env_var_with_random_value).unwrap() + std::env::var(EXISTING_ENV_VAR_WITH_NON_EMPTY_VALUE).unwrap() ) .as_str(), )) @@ -2714,7 +2721,7 @@ async fn env_var_overrides_loaded_auth() { name: "custom".to_string(), base_url: Some(format!("{}/openai", server.uri())), // Reuse the existing environment variable to avoid using unsafe code - env_key: Some(existing_env_var_with_random_value.to_string()), + env_key: Some(EXISTING_ENV_VAR_WITH_NON_EMPTY_VALUE.to_string()), query_params: Some(std::collections::HashMap::from([( "api-version".to_string(), "2025-04-01-preview".to_string(),