diff --git a/AGENTS.md b/AGENTS.md index e924ceeae1b..5c0a6db6374 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -77,11 +77,11 @@ If you don’t have the tool: - Prefer deep equals comparisons whenever possible. Perform `assert_eq!()` on entire objects, rather than individual fields. - Avoid mutating process environment in tests; prefer passing environment-derived flags or dependencies from above. -### Spawning workspace binaries in tests (Cargo vs Buck2) +### Spawning workspace binaries in tests (Cargo vs Bazel) - Prefer `codex_utils_cargo_bin::cargo_bin("...")` over `assert_cmd::Command::cargo_bin(...)` or `escargot` when tests need to spawn first-party binaries. - - Under Buck2, `CARGO_BIN_EXE_*` may be project-relative (e.g. `buck-out/...`), which breaks if a test changes its working directory. `codex_utils_cargo_bin::cargo_bin` resolves to an absolute path first. -- When locating fixture files under Buck2, avoid `env!("CARGO_MANIFEST_DIR")` (Buck codegen sets it to `"."`). Prefer deriving paths from `codex_utils_cargo_bin::buck_project_root()` when needed. + - Under Bazel, binaries and resources may live under runfiles; use `codex_utils_cargo_bin::cargo_bin` to resolve absolute paths that remain stable after `chdir`. +- When locating fixture files or test resources under Bazel, avoid `env!("CARGO_MANIFEST_DIR")`. Prefer `codex_utils_cargo_bin::find_resource!` so paths resolve correctly under both Cargo and Bazel runfiles. ### Integration tests (core) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index c0cbcf1788e..add99854fbd 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1129,6 +1129,7 @@ dependencies = [ "codex-common", "codex-core", "codex-git", + "codex-utils-cargo-bin", "serde", "serde_json", "tempfile", @@ -2844,6 +2845,7 @@ dependencies = [ "anyhow", "codex-core", "codex-utils-cargo-bin", + "path-absolutize", "rmcp", "serde_json", "tokio", diff --git a/codex-rs/apply-patch/tests/suite/scenarios.rs b/codex-rs/apply-patch/tests/suite/scenarios.rs index 0e21a7bc0a1..e53f2f1192a 100644 --- a/codex-rs/apply-patch/tests/suite/scenarios.rs +++ b/codex-rs/apply-patch/tests/suite/scenarios.rs @@ -1,3 +1,4 @@ +use codex_utils_cargo_bin::find_resource; use pretty_assertions::assert_eq; use std::collections::BTreeMap; use std::fs; @@ -8,7 +9,7 @@ use tempfile::tempdir; #[test] fn test_apply_patch_scenarios() -> anyhow::Result<()> { - let scenarios_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/scenarios"); + let scenarios_dir = find_resource!("tests/fixtures/scenarios")?; for scenario in fs::read_dir(scenarios_dir)? { let scenario = scenario?; let path = scenario.path(); diff --git a/codex-rs/chatgpt/Cargo.toml b/codex-rs/chatgpt/Cargo.toml index b58cd623402..70cd0aa5aa3 100644 --- a/codex-rs/chatgpt/Cargo.toml +++ b/codex-rs/chatgpt/Cargo.toml @@ -12,6 +12,7 @@ anyhow = { workspace = true } clap = { workspace = true, features = ["derive"] } codex-common = { workspace = true, features = ["cli"] } codex-core = { workspace = true } +codex-utils-cargo-bin = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } tokio = { workspace = true, features = ["full"] } diff --git a/codex-rs/chatgpt/tests/suite/apply_command_e2e.rs b/codex-rs/chatgpt/tests/suite/apply_command_e2e.rs index 7e096bce080..c2d570528ce 100644 --- a/codex-rs/chatgpt/tests/suite/apply_command_e2e.rs +++ b/codex-rs/chatgpt/tests/suite/apply_command_e2e.rs @@ -1,6 +1,6 @@ use codex_chatgpt::apply_command::apply_diff_from_task; use codex_chatgpt::get_task::GetTaskResponse; -use std::path::Path; +use codex_utils_cargo_bin::find_resource; use tempfile::TempDir; use tokio::process::Command; @@ -68,7 +68,7 @@ async fn create_temp_git_repo() -> anyhow::Result { } async fn mock_get_task_with_fixture() -> anyhow::Result { - let fixture_path = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/task_turn_fixture.json"); + let fixture_path = find_resource!("tests/task_turn_fixture.json")?; let fixture_content = tokio::fs::read_to_string(fixture_path).await?; let response: GetTaskResponse = serde_json::from_str(&fixture_content)?; Ok(response) diff --git a/codex-rs/core/tests/suite/cli_stream.rs b/codex-rs/core/tests/suite/cli_stream.rs index d6cf977a48a..633d55a608a 100644 --- a/codex-rs/core/tests/suite/cli_stream.rs +++ b/codex-rs/core/tests/suite/cli_stream.rs @@ -1,6 +1,7 @@ use assert_cmd::Command as AssertCommand; use codex_core::RolloutRecorder; use codex_core::protocol::GitInfo; +use codex_utils_cargo_bin::find_resource; use core_test_support::fs_wait; use core_test_support::skip_if_no_network; use std::time::Duration; @@ -12,6 +13,16 @@ use wiremock::ResponseTemplate; use wiremock::matchers::method; use wiremock::matchers::path; +fn repo_root() -> std::path::PathBuf { + #[expect(clippy::expect_used)] + find_resource!(".").expect("failed to resolve repo root") +} + +fn cli_responses_fixture() -> std::path::PathBuf { + #[expect(clippy::expect_used)] + find_resource!("tests/cli_responses_fixture.sse").expect("failed to resolve fixture path") +} + /// Tests streaming chat completions through the CLI using a mock server. /// This test: /// 1. Sets up a mock server that simulates OpenAI's chat completions API @@ -23,6 +34,7 @@ async fn chat_mode_stream_cli() { skip_if_no_network!(); let server = MockServer::start().await; + let repo_root = repo_root(); let sse = concat!( "data: {\"choices\":[{\"delta\":{\"content\":\"hi\"}}]}\n\n", "data: {\"choices\":[{\"delta\":{}}]}\n\n", @@ -53,7 +65,7 @@ async fn chat_mode_stream_cli() { .arg("-c") .arg("model_provider=\"mock\"") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg("hello?"); cmd.env("CODEX_HOME", home.path()) .env("OPENAI_API_KEY", "dummy") @@ -127,6 +139,7 @@ async fn exec_cli_applies_experimental_instructions_file() { ); let home = TempDir::new().unwrap(); + let repo_root = repo_root(); let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap(); let mut cmd = AssertCommand::new(bin); cmd.arg("exec") @@ -140,7 +153,7 @@ async fn exec_cli_applies_experimental_instructions_file() { "experimental_instructions_file=\"{custom_path_str}\"" )) .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg("hello?\n"); cmd.env("CODEX_HOME", home.path()) .env("OPENAI_API_KEY", "dummy") @@ -177,8 +190,8 @@ async fn exec_cli_applies_experimental_instructions_file() { async fn responses_api_stream_cli() { skip_if_no_network!(); - let fixture = - std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/cli_responses_fixture.sse"); + let fixture = cli_responses_fixture(); + let repo_root = repo_root(); let home = TempDir::new().unwrap(); let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap(); @@ -186,7 +199,7 @@ async fn responses_api_stream_cli() { cmd.arg("exec") .arg("--skip-git-repo-check") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg("hello?"); cmd.env("CODEX_HOME", home.path()) .env("OPENAI_API_KEY", "dummy") @@ -213,8 +226,8 @@ async fn integration_creates_and_checks_session_file() -> anyhow::Result<()> { let prompt = format!("echo {marker}"); // 3. Use the same offline SSE fixture as responses_api_stream_cli so the test is hermetic. - let fixture = - std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/cli_responses_fixture.sse"); + let fixture = cli_responses_fixture(); + let repo_root = repo_root(); // 4. Run the codex CLI and invoke `exec`, which is what records a session. let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap(); @@ -222,7 +235,7 @@ async fn integration_creates_and_checks_session_file() -> anyhow::Result<()> { cmd.arg("exec") .arg("--skip-git-repo-check") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg(&prompt); cmd.env("CODEX_HOME", home.path()) .env("OPENAI_API_KEY", "dummy") @@ -343,7 +356,7 @@ async fn integration_creates_and_checks_session_file() -> anyhow::Result<()> { cmd2.arg("exec") .arg("--skip-git-repo-check") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg(&prompt2) .arg("resume") .arg("--last"); diff --git a/codex-rs/exec-server/tests/common/Cargo.toml b/codex-rs/exec-server/tests/common/Cargo.toml index 6444b61f97e..4846db52f75 100644 --- a/codex-rs/exec-server/tests/common/Cargo.toml +++ b/codex-rs/exec-server/tests/common/Cargo.toml @@ -11,6 +11,7 @@ path = "lib.rs" anyhow = { workspace = true } codex-core = { workspace = true } codex-utils-cargo-bin = { workspace = true } +path-absolutize = { workspace = true } rmcp = { workspace = true } serde_json = { workspace = true } tokio = { workspace = true } diff --git a/codex-rs/exec-server/tests/common/lib.rs b/codex-rs/exec-server/tests/common/lib.rs index d5e3af208a2..b587868d80f 100644 --- a/codex-rs/exec-server/tests/common/lib.rs +++ b/codex-rs/exec-server/tests/common/lib.rs @@ -1,6 +1,8 @@ use codex_core::MCP_SANDBOX_STATE_METHOD; use codex_core::SandboxState; use codex_core::protocol::SandboxPolicy; +use codex_utils_cargo_bin::find_resource; +use path_absolutize::Absolutize; use rmcp::ClientHandler; use rmcp::ErrorData as McpError; use rmcp::RoleClient; @@ -35,16 +37,15 @@ where let mcp_executable = codex_utils_cargo_bin::cargo_bin("codex-exec-mcp-server")?; let execve_wrapper = codex_utils_cargo_bin::cargo_bin("codex-execve-wrapper")?; - // `bash` requires a special lookup when running under Bazel because it is a - // _resource_ rather than a binary target. - let bash = if let Some(bazel_runfiles_dir) = std::env::var_os("RUNFILES_DIR") { - PathBuf::from(bazel_runfiles_dir).join("_main/codex-rs/exec-server/tests/suite/bash") - } else { - Path::new(env!("CARGO_MANIFEST_DIR")) - .join("..") - .join("suite") - .join("bash") - }; + // `bash` is a test resource rather than a binary target, so we must use + // `find_resource!` to locate it instead of `cargo_bin`. + // + // Note we also have to normalize (but not canonicalize!) the path for + // _Bazel_ because the original value ends with + // `codex-rs/exec-server/tests/common/../suite/bash`, but the `tests/common` + // folder will not exist at runtime under Bazel. As such, we have to + // normalize it before passing it to `dotslash fetch`. + let bash = find_resource!("../suite/bash")?.absolutize()?.to_path_buf(); // Need to ensure the artifact associated with the bash DotSlash file is // available before it is run in a read-only sandbox. diff --git a/codex-rs/exec/tests/suite/auth_env.rs b/codex-rs/exec/tests/suite/auth_env.rs index 91d7bad8f8f..4f8018e808f 100644 --- a/codex-rs/exec/tests/suite/auth_env.rs +++ b/codex-rs/exec/tests/suite/auth_env.rs @@ -1,4 +1,5 @@ #![allow(clippy::unwrap_used, clippy::expect_used)] +use codex_utils_cargo_bin::find_resource; use core_test_support::responses::ev_completed; use core_test_support::responses::mount_sse_once_match; use core_test_support::responses::sse; @@ -10,6 +11,7 @@ use wiremock::matchers::header; async fn exec_uses_codex_api_key_env_var() -> anyhow::Result<()> { let test = test_codex_exec(); let server = start_mock_server().await; + let repo_root = find_resource!(".")?; mount_sse_once_match( &server, @@ -21,7 +23,7 @@ async fn exec_uses_codex_api_key_env_var() -> anyhow::Result<()> { test.cmd_with_server(&server) .arg("--skip-git-repo-check") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg("echo testing codex api key") .assert() .success(); diff --git a/codex-rs/exec/tests/suite/resume.rs b/codex-rs/exec/tests/suite/resume.rs index b6b750dc3e1..32913983e61 100644 --- a/codex-rs/exec/tests/suite/resume.rs +++ b/codex-rs/exec/tests/suite/resume.rs @@ -1,9 +1,9 @@ #![allow(clippy::unwrap_used, clippy::expect_used)] use anyhow::Context; +use codex_utils_cargo_bin::find_resource; use core_test_support::test_codex_exec::test_codex_exec; use pretty_assertions::assert_eq; use serde_json::Value; -use std::path::Path; use std::string::ToString; use uuid::Uuid; use walkdir::WalkDir; @@ -103,11 +103,19 @@ fn last_user_image_count(path: &std::path::Path) -> usize { last_count } +fn exec_fixture() -> anyhow::Result { + Ok(find_resource!("tests/fixtures/cli_responses_fixture.sse")?) +} + +fn exec_repo_root() -> anyhow::Result { + Ok(find_resource!(".")?) +} + #[test] fn exec_resume_last_appends_to_existing_file() -> anyhow::Result<()> { let test = test_codex_exec(); - let fixture = - Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/cli_responses_fixture.sse"); + let fixture = exec_fixture()?; + let repo_root = exec_repo_root()?; // 1) First run: create a session with a unique marker in the content. let marker = format!("resume-last-{}", Uuid::new_v4()); @@ -118,7 +126,7 @@ fn exec_resume_last_appends_to_existing_file() -> anyhow::Result<()> { .env("OPENAI_BASE_URL", "http://unused.local") .arg("--skip-git-repo-check") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg(&prompt) .assert() .success(); @@ -137,7 +145,7 @@ fn exec_resume_last_appends_to_existing_file() -> anyhow::Result<()> { .env("OPENAI_BASE_URL", "http://unused.local") .arg("--skip-git-repo-check") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg(&prompt2) .arg("resume") .arg("--last") @@ -160,8 +168,8 @@ fn exec_resume_last_appends_to_existing_file() -> anyhow::Result<()> { #[test] fn exec_resume_last_accepts_prompt_after_flag_in_json_mode() -> anyhow::Result<()> { let test = test_codex_exec(); - let fixture = - Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/cli_responses_fixture.sse"); + let fixture = exec_fixture()?; + let repo_root = exec_repo_root()?; // 1) First run: create a session with a unique marker in the content. let marker = format!("resume-last-json-{}", Uuid::new_v4()); @@ -172,7 +180,7 @@ fn exec_resume_last_accepts_prompt_after_flag_in_json_mode() -> anyhow::Result<( .env("OPENAI_BASE_URL", "http://unused.local") .arg("--skip-git-repo-check") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg(&prompt) .assert() .success(); @@ -191,7 +199,7 @@ fn exec_resume_last_accepts_prompt_after_flag_in_json_mode() -> anyhow::Result<( .env("OPENAI_BASE_URL", "http://unused.local") .arg("--skip-git-repo-check") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg("--json") .arg("resume") .arg("--last") @@ -214,8 +222,7 @@ fn exec_resume_last_accepts_prompt_after_flag_in_json_mode() -> anyhow::Result<( #[test] fn exec_resume_accepts_global_flags_after_subcommand() -> anyhow::Result<()> { let test = test_codex_exec(); - let fixture = - Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/cli_responses_fixture.sse"); + let fixture = exec_fixture()?; // Seed a session. test.cmd() @@ -249,8 +256,8 @@ fn exec_resume_accepts_global_flags_after_subcommand() -> anyhow::Result<()> { #[test] fn exec_resume_by_id_appends_to_existing_file() -> anyhow::Result<()> { let test = test_codex_exec(); - let fixture = - Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/cli_responses_fixture.sse"); + let fixture = exec_fixture()?; + let repo_root = exec_repo_root()?; // 1) First run: create a session let marker = format!("resume-by-id-{}", Uuid::new_v4()); @@ -261,7 +268,7 @@ fn exec_resume_by_id_appends_to_existing_file() -> anyhow::Result<()> { .env("OPENAI_BASE_URL", "http://unused.local") .arg("--skip-git-repo-check") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg(&prompt) .assert() .success(); @@ -284,7 +291,7 @@ fn exec_resume_by_id_appends_to_existing_file() -> anyhow::Result<()> { .env("OPENAI_BASE_URL", "http://unused.local") .arg("--skip-git-repo-check") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg(&prompt2) .arg("resume") .arg(&session_id) @@ -306,8 +313,8 @@ fn exec_resume_by_id_appends_to_existing_file() -> anyhow::Result<()> { #[test] fn exec_resume_preserves_cli_configuration_overrides() -> anyhow::Result<()> { let test = test_codex_exec(); - let fixture = - Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/cli_responses_fixture.sse"); + let fixture = exec_fixture()?; + let repo_root = exec_repo_root()?; let marker = format!("resume-config-{}", Uuid::new_v4()); let prompt = format!("echo {marker}"); @@ -321,7 +328,7 @@ fn exec_resume_preserves_cli_configuration_overrides() -> anyhow::Result<()> { .arg("--model") .arg("gpt-5.1") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg(&prompt) .assert() .success(); @@ -343,7 +350,7 @@ fn exec_resume_preserves_cli_configuration_overrides() -> anyhow::Result<()> { .arg("--model") .arg("gpt-5.1-high") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg(&prompt2) .arg("resume") .arg("--last") @@ -382,8 +389,8 @@ fn exec_resume_preserves_cli_configuration_overrides() -> anyhow::Result<()> { #[test] fn exec_resume_accepts_images_after_subcommand() -> anyhow::Result<()> { let test = test_codex_exec(); - let fixture = - Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/cli_responses_fixture.sse"); + let fixture = exec_fixture()?; + let repo_root = exec_repo_root()?; let marker = format!("resume-image-{}", Uuid::new_v4()); let prompt = format!("echo {marker}"); @@ -393,7 +400,7 @@ fn exec_resume_accepts_images_after_subcommand() -> anyhow::Result<()> { .env("OPENAI_BASE_URL", "http://unused.local") .arg("--skip-git-repo-check") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg(&prompt) .assert() .success(); @@ -417,7 +424,7 @@ fn exec_resume_accepts_images_after_subcommand() -> anyhow::Result<()> { .env("OPENAI_BASE_URL", "http://unused.local") .arg("--skip-git-repo-check") .arg("-C") - .arg(env!("CARGO_MANIFEST_DIR")) + .arg(&repo_root) .arg("resume") .arg("--last") .arg("--image") diff --git a/codex-rs/utils/cargo-bin/src/lib.rs b/codex-rs/utils/cargo-bin/src/lib.rs index 16f54939150..2858ad6bca7 100644 --- a/codex-rs/utils/cargo-bin/src/lib.rs +++ b/codex-rs/utils/cargo-bin/src/lib.rs @@ -70,6 +70,43 @@ fn cargo_bin_env_keys(name: &str) -> Vec { keys } +/// Macro that derives the path to a test resource at runtime, the value of +/// which depends on whether Cargo or Bazel is being used to build and run a +/// test. Note the return value may be a relative or absolute path. +/// (Incidentally, this is a macro rather than a function because it reads +/// compile-time environment variables that need to be captured at the call +/// site.) +/// +/// This is expected to be used exclusively in test code because Codex CLI is a +/// standalone binary with no packaged resources. +#[macro_export] +macro_rules! find_resource { + ($resource:expr) => {{ + // When this code is built and run with Bazel: + // - we inject `BAZEL_PACKAGE` as a compile-time environment variable + // that points to native.package_name() + // - at runtime, Bazel will set `RUNFILES_DIR` to the runfiles directory + // + // Therefore, the compile-time value of `BAZEL_PACKAGE` will always be + // included in the compiled binary (even if it is built with Cargo), but + // we only check it at runtime if `RUNFILES_DIR` is set. + let resource = std::path::Path::new(&$resource); + let manifest_dir = match std::env::var("RUNFILES_DIR") { + Ok(bazel_runtime_files) => match option_env!("BAZEL_PACKAGE") { + Some(bazel_package) => Ok(std::path::PathBuf::from(bazel_runtime_files) + .join("_main") + .join(bazel_package)), + None => Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + "BAZEL_PACKAGE not set in Bazel build", + )), + }, + Err(_) => Ok(std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"))), + }; + manifest_dir.map(|dir| dir.join(resource)) + }}; +} + fn resolve_bin_from_env(key: &str, value: OsString) -> Result { let abs = absolutize_from_buck_or_cwd(PathBuf::from(value))?;