mirror of
https://github.com/openai/codex.git
synced 2026-02-01 22:47:52 +00:00
fix: added test helpers for platform-specific paths (#7954)
This addresses post-merge feedback from https://github.com/openai/codex/pull/7856.
This commit is contained in:
2
codex-rs/Cargo.lock
generated
2
codex-rs/Cargo.lock
generated
@@ -837,7 +837,6 @@ dependencies = [
|
||||
"codex-login",
|
||||
"codex-protocol",
|
||||
"codex-rmcp-client",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-json-to-toml",
|
||||
"core_test_support",
|
||||
"mcp-types",
|
||||
@@ -1887,6 +1886,7 @@ dependencies = [
|
||||
"base64",
|
||||
"codex-core",
|
||||
"codex-protocol",
|
||||
"codex-utils-absolute-path",
|
||||
"notify",
|
||||
"regex-lite",
|
||||
"serde_json",
|
||||
|
||||
@@ -27,7 +27,6 @@ codex-protocol = { workspace = true }
|
||||
codex-app-server-protocol = { workspace = true }
|
||||
codex-feedback = { workspace = true }
|
||||
codex-rmcp-client = { workspace = true }
|
||||
codex-utils-absolute-path = { workspace = true }
|
||||
codex-utils-json-to-toml = { workspace = true }
|
||||
chrono = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
|
||||
@@ -14,6 +14,9 @@ pub use core_test_support::format_with_current_shell;
|
||||
pub use core_test_support::format_with_current_shell_display;
|
||||
pub use core_test_support::format_with_current_shell_display_non_login;
|
||||
pub use core_test_support::format_with_current_shell_non_login;
|
||||
pub use core_test_support::test_path_buf_with_windows;
|
||||
pub use core_test_support::test_tmp_path;
|
||||
pub use core_test_support::test_tmp_path_buf;
|
||||
pub use mcp_process::McpProcess;
|
||||
pub use mock_model_server::create_mock_chat_completions_server;
|
||||
pub use mock_model_server::create_mock_chat_completions_server_unchecked;
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use anyhow::Result;
|
||||
use app_test_support::McpProcess;
|
||||
use app_test_support::test_tmp_path;
|
||||
use app_test_support::to_response;
|
||||
use codex_app_server_protocol::GetUserSavedConfigResponse;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
@@ -14,7 +15,6 @@ use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::config_types::SandboxMode;
|
||||
use codex_protocol::config_types::Verbosity;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
@@ -24,11 +24,7 @@ use tokio::time::timeout;
|
||||
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
||||
|
||||
fn create_config_toml(codex_home: &Path) -> std::io::Result<()> {
|
||||
let writable_root = if cfg!(windows) {
|
||||
r"C:\Users\codex\AppData\Local\Temp"
|
||||
} else {
|
||||
"/tmp"
|
||||
};
|
||||
let writable_root = test_tmp_path();
|
||||
let config_toml = codex_home.join("config.toml");
|
||||
std::fs::write(
|
||||
config_toml,
|
||||
@@ -84,12 +80,7 @@ async fn get_config_toml_parses_all_fields() -> Result<()> {
|
||||
.await??;
|
||||
|
||||
let config: GetUserSavedConfigResponse = to_response(resp)?;
|
||||
let writable_root = if cfg!(windows) {
|
||||
r"C:\Users\codex\AppData\Local\Temp"
|
||||
} else {
|
||||
"/tmp"
|
||||
};
|
||||
let writable_root = AbsolutePathBuf::from_absolute_path(writable_root)?;
|
||||
let writable_root = test_tmp_path();
|
||||
let expected = GetUserSavedConfigResponse {
|
||||
config: UserSavedConfig {
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
use anyhow::Result;
|
||||
use app_test_support::McpProcess;
|
||||
use app_test_support::test_path_buf_with_windows;
|
||||
use app_test_support::test_tmp_path_buf;
|
||||
use app_test_support::to_response;
|
||||
use codex_app_server_protocol::AskForApproval;
|
||||
use codex_app_server_protocol::ConfigBatchWriteParams;
|
||||
@@ -18,7 +20,6 @@ use codex_app_server_protocol::ToolsV2;
|
||||
use codex_app_server_protocol::WriteStatus;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::path::PathBuf;
|
||||
use tempfile::TempDir;
|
||||
use tokio::time::timeout;
|
||||
|
||||
@@ -135,11 +136,8 @@ view_image = false
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn config_read_includes_system_layer_and_overrides() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let (user_dir, system_dir) = if cfg!(windows) {
|
||||
(r"C:\Users\user", r"C:\System")
|
||||
} else {
|
||||
("/user", "/system")
|
||||
};
|
||||
let user_dir = test_path_buf_with_windows("/user", Some(r"C:\Users\user"));
|
||||
let system_dir = test_path_buf_with_windows("/system", Some(r"C:\System"));
|
||||
write_config(
|
||||
&codex_home,
|
||||
&format!(
|
||||
@@ -167,7 +165,7 @@ approval_policy = "never"
|
||||
[sandbox_workspace_write]
|
||||
writable_roots = [{}]
|
||||
"#,
|
||||
serde_json::json!(system_dir)
|
||||
serde_json::json!(system_dir.clone())
|
||||
),
|
||||
)?;
|
||||
|
||||
@@ -218,7 +216,7 @@ writable_roots = [{}]
|
||||
.sandbox_workspace_write
|
||||
.as_ref()
|
||||
.expect("sandbox workspace write");
|
||||
assert_eq!(sandbox.writable_roots, vec![PathBuf::from(system_dir)]);
|
||||
assert_eq!(sandbox.writable_roots, vec![system_dir]);
|
||||
assert_eq!(
|
||||
origins
|
||||
.get("sandbox_workspace_write.writable_roots.0")
|
||||
@@ -361,11 +359,7 @@ async fn config_batch_write_applies_multiple_edits() -> Result<()> {
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let writable_root = if cfg!(windows) {
|
||||
r"C:\Users\codex\AppData\Local\Temp"
|
||||
} else {
|
||||
"/tmp"
|
||||
};
|
||||
let writable_root = test_tmp_path_buf();
|
||||
let batch_id = mcp
|
||||
.send_config_batch_write_request(ConfigBatchWriteParams {
|
||||
file_path: Some(codex_home.path().join("config.toml").display().to_string()),
|
||||
@@ -378,7 +372,7 @@ async fn config_batch_write_applies_multiple_edits() -> Result<()> {
|
||||
ConfigEdit {
|
||||
key_path: "sandbox_workspace_write".to_string(),
|
||||
value: json!({
|
||||
"writable_roots": [writable_root],
|
||||
"writable_roots": [writable_root.clone()],
|
||||
"network_access": false
|
||||
}),
|
||||
merge_strategy: MergeStrategy::Replace,
|
||||
@@ -420,7 +414,7 @@ async fn config_batch_write_applies_multiple_edits() -> Result<()> {
|
||||
.sandbox_workspace_write
|
||||
.as_ref()
|
||||
.expect("sandbox workspace write");
|
||||
assert_eq!(sandbox.writable_roots, vec![PathBuf::from(writable_root)]);
|
||||
assert_eq!(sandbox.writable_roots, vec![writable_root]);
|
||||
assert!(!sandbox.network_access);
|
||||
|
||||
Ok(())
|
||||
|
||||
@@ -1315,6 +1315,7 @@ mod tests {
|
||||
use crate::features::Feature;
|
||||
|
||||
use super::*;
|
||||
use core_test_support::test_absolute_path;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use std::time::Duration;
|
||||
@@ -1413,11 +1414,7 @@ network_access = true # This should be ignored.
|
||||
}
|
||||
);
|
||||
|
||||
let writable_root = if cfg!(windows) {
|
||||
"C:\\my\\workspace"
|
||||
} else {
|
||||
"/my/workspace"
|
||||
};
|
||||
let writable_root = test_absolute_path("/my/workspace");
|
||||
let sandbox_workspace_write = format!(
|
||||
r#"
|
||||
sandbox_mode = "workspace-write"
|
||||
@@ -1429,7 +1426,7 @@ writable_roots = [
|
||||
exclude_tmpdir_env_var = true
|
||||
exclude_slash_tmp = true
|
||||
"#,
|
||||
serde_json::json!(writable_root.to_string_lossy())
|
||||
serde_json::json!(writable_root)
|
||||
);
|
||||
|
||||
let sandbox_workspace_write_cfg = toml::from_str::<ConfigToml>(&sandbox_workspace_write)
|
||||
@@ -1453,7 +1450,7 @@ exclude_slash_tmp = true
|
||||
resolution,
|
||||
SandboxPolicyResolution {
|
||||
policy: SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec!["/my/workspace".try_into().unwrap()],
|
||||
writable_roots: vec![writable_root.clone()],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
@@ -1477,7 +1474,7 @@ exclude_slash_tmp = true
|
||||
[projects."/tmp/test"]
|
||||
trust_level = "trusted"
|
||||
"#,
|
||||
serde_json::json!(writable_root.to_string_lossy())
|
||||
serde_json::json!(writable_root)
|
||||
);
|
||||
|
||||
let sandbox_workspace_write_cfg = toml::from_str::<ConfigToml>(&sandbox_workspace_write)
|
||||
@@ -1501,10 +1498,7 @@ trust_level = "trusted"
|
||||
resolution,
|
||||
SandboxPolicyResolution {
|
||||
policy: SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![
|
||||
AbsolutePathBuf::from_absolute_path("/my/workspace")
|
||||
.expect("absolute path")
|
||||
],
|
||||
writable_roots: vec![writable_root],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
|
||||
@@ -192,6 +192,8 @@ mod tests {
|
||||
use crate::shell::ShellType;
|
||||
|
||||
use super::*;
|
||||
use core_test_support::test_path_buf;
|
||||
use core_test_support::test_tmp_path_buf;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn fake_shell() -> Shell {
|
||||
@@ -216,12 +218,19 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn serialize_workspace_write_environment_context() {
|
||||
let cwd = if cfg!(windows) { "C:\\repo" } else { "/repo" };
|
||||
let writable_root = if cfg!(windows) { "C:\\tmp" } else { "/tmp" };
|
||||
let cwd = test_path_buf("/repo");
|
||||
let writable_root = test_tmp_path_buf();
|
||||
let cwd_str = cwd.to_str().expect("cwd is valid utf-8");
|
||||
let writable_root_str = writable_root
|
||||
.to_str()
|
||||
.expect("writable root is valid utf-8");
|
||||
let context = EnvironmentContext::new(
|
||||
Some(PathBuf::from(cwd)),
|
||||
Some(cwd.clone()),
|
||||
Some(AskForApproval::OnRequest),
|
||||
Some(workspace_write_policy(vec![cwd, writable_root], false)),
|
||||
Some(workspace_write_policy(
|
||||
vec![cwd_str, writable_root_str],
|
||||
false,
|
||||
)),
|
||||
fake_shell(),
|
||||
);
|
||||
|
||||
@@ -236,7 +245,9 @@ mod tests {
|
||||
<root>{writable_root}</root>
|
||||
</writable_roots>
|
||||
<shell>bash</shell>
|
||||
</environment_context>"#
|
||||
</environment_context>"#,
|
||||
cwd = cwd.display(),
|
||||
writable_root = writable_root.display(),
|
||||
);
|
||||
|
||||
assert_eq!(context.serialize_to_xml(), expected);
|
||||
|
||||
@@ -13,6 +13,7 @@ assert_cmd = { workspace = true }
|
||||
base64 = { workspace = true }
|
||||
codex-core = { workspace = true, features = ["test-support"] }
|
||||
codex-protocol = { workspace = true }
|
||||
codex-utils-absolute-path = { workspace = true }
|
||||
notify = { workspace = true }
|
||||
regex-lite = { workspace = true }
|
||||
serde_json = { workspace = true }
|
||||
|
||||
@@ -6,7 +6,9 @@ use codex_core::CodexConversation;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::ConfigOverrides;
|
||||
use codex_core::config::ConfigToml;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use regex_lite::Regex;
|
||||
use std::path::PathBuf;
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
use assert_cmd::cargo::cargo_bin;
|
||||
@@ -25,6 +27,49 @@ pub fn assert_regex_match<'s>(pattern: &str, actual: &'s str) -> regex_lite::Cap
|
||||
.unwrap_or_else(|| panic!("regex {pattern:?} did not match {actual:?}"))
|
||||
}
|
||||
|
||||
pub fn test_path_buf_with_windows(unix_path: &str, windows_path: Option<&str>) -> PathBuf {
|
||||
if cfg!(windows) {
|
||||
if let Some(windows) = windows_path {
|
||||
PathBuf::from(windows)
|
||||
} else {
|
||||
let mut path = PathBuf::from(r"C:\");
|
||||
path.extend(
|
||||
unix_path
|
||||
.trim_start_matches('/')
|
||||
.split('/')
|
||||
.filter(|segment| !segment.is_empty()),
|
||||
);
|
||||
path
|
||||
}
|
||||
} else {
|
||||
PathBuf::from(unix_path)
|
||||
}
|
||||
}
|
||||
|
||||
pub fn test_path_buf(unix_path: &str) -> PathBuf {
|
||||
test_path_buf_with_windows(unix_path, None)
|
||||
}
|
||||
|
||||
pub fn test_absolute_path_with_windows(
|
||||
unix_path: &str,
|
||||
windows_path: Option<&str>,
|
||||
) -> AbsolutePathBuf {
|
||||
AbsolutePathBuf::from_absolute_path(test_path_buf_with_windows(unix_path, windows_path))
|
||||
.expect("test path should be absolute")
|
||||
}
|
||||
|
||||
pub fn test_absolute_path(unix_path: &str) -> AbsolutePathBuf {
|
||||
test_absolute_path_with_windows(unix_path, None)
|
||||
}
|
||||
|
||||
pub fn test_tmp_path() -> AbsolutePathBuf {
|
||||
test_absolute_path_with_windows("/tmp", Some(r"C:\Users\codex\AppData\Local\Temp"))
|
||||
}
|
||||
|
||||
pub fn test_tmp_path_buf() -> PathBuf {
|
||||
test_tmp_path().into_path_buf()
|
||||
}
|
||||
|
||||
/// Returns a default `Config` whose on-disk state is confined to the provided
|
||||
/// temporary directory. Using a per-test directory keeps tests hermetic and
|
||||
/// avoids clobbering a developer’s real `~/.codex`.
|
||||
|
||||
Reference in New Issue
Block a user