Adds an integration test for the new behavior introduced in https://github.com/openai/codex/pull/9011. The work to create the test setup was substantial enough that I thought it merited a separate PR. This integration test spawns `codex` in TUI mode, which requires spawning a PTY to run successfully, so I had to introduce quite a bit of scaffolding in `run_codex_cli()`. I was surprised to discover that we have not done this in our codebase before, so perhaps this should get moved to a common location so it can be reused. The test itself verifies that a malformed `rules` in `$CODEX_HOME` prints a human-readable error message and exits nonzero.
This commit is contained in:
parent
2cd1a0a45e
commit
4c673086bc
12 changed files with 265 additions and 0 deletions
6
codex-rs/Cargo.lock
generated
6
codex-rs/Cargo.lock
generated
|
|
@ -1746,6 +1746,7 @@ dependencies = [
|
|||
"codex-app-server-protocol",
|
||||
"codex-arg0",
|
||||
"codex-backend-client",
|
||||
"codex-cli",
|
||||
"codex-common",
|
||||
"codex-core",
|
||||
"codex-feedback",
|
||||
|
|
@ -1753,6 +1754,8 @@ dependencies = [
|
|||
"codex-login",
|
||||
"codex-protocol",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-cargo-bin",
|
||||
"codex-utils-pty",
|
||||
"codex-windows-sandbox",
|
||||
"color-eyre",
|
||||
"crossterm",
|
||||
|
|
@ -1818,6 +1821,7 @@ dependencies = [
|
|||
"codex-app-server-protocol",
|
||||
"codex-arg0",
|
||||
"codex-backend-client",
|
||||
"codex-cli",
|
||||
"codex-common",
|
||||
"codex-core",
|
||||
"codex-feedback",
|
||||
|
|
@ -1826,6 +1830,8 @@ dependencies = [
|
|||
"codex-protocol",
|
||||
"codex-tui",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-cargo-bin",
|
||||
"codex-utils-pty",
|
||||
"codex-windows-sandbox",
|
||||
"color-eyre",
|
||||
"crossterm",
|
||||
|
|
|
|||
|
|
@ -71,6 +71,7 @@ codex-arg0 = { path = "arg0" }
|
|||
codex-async-utils = { path = "async-utils" }
|
||||
codex-backend-client = { path = "backend-client" }
|
||||
codex-chatgpt = { path = "chatgpt" }
|
||||
codex-cli = { path = "cli"}
|
||||
codex-client = { path = "codex-client" }
|
||||
codex-common = { path = "common" }
|
||||
codex-core = { path = "core" }
|
||||
|
|
|
|||
|
|
@ -14,4 +14,7 @@ codex_rust_crate(
|
|||
),
|
||||
test_data_extra = glob(["src/**/snapshots/**"]),
|
||||
integration_compile_data_extra = ["src/test_backend.rs"],
|
||||
extra_binaries = [
|
||||
"//codex-rs/cli:codex",
|
||||
],
|
||||
)
|
||||
|
|
|
|||
|
|
@ -113,7 +113,10 @@ arboard = { workspace = true }
|
|||
|
||||
|
||||
[dev-dependencies]
|
||||
codex-cli = { workspace = true }
|
||||
codex-core = { workspace = true, features = ["test-support"] }
|
||||
codex-utils-cargo-bin = { workspace = true }
|
||||
codex-utils-pty = { workspace = true }
|
||||
assert_matches = { workspace = true }
|
||||
chrono = { workspace = true, features = ["serde"] }
|
||||
insta = { workspace = true }
|
||||
|
|
|
|||
|
|
@ -3,4 +3,7 @@
|
|||
#[cfg(feature = "vt100-tests")]
|
||||
mod test_backend;
|
||||
|
||||
#[allow(unused_imports)]
|
||||
use codex_cli as _; // Keep dev-dep for cargo-shear; tests spawn the codex binary.
|
||||
|
||||
mod suite;
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
// Aggregates all former standalone integration tests as modules.
|
||||
mod no_panic_on_startup;
|
||||
mod status_indicator;
|
||||
mod vt100_history;
|
||||
mod vt100_live_commit;
|
||||
|
|
|
|||
119
codex-rs/tui/tests/suite/no_panic_on_startup.rs
Normal file
119
codex-rs/tui/tests/suite/no_panic_on_startup.rs
Normal file
|
|
@ -0,0 +1,119 @@
|
|||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
use std::time::Duration;
|
||||
use tokio::select;
|
||||
use tokio::time::timeout;
|
||||
|
||||
/// Regression test for https://github.com/openai/codex/issues/8803.
|
||||
#[tokio::test]
|
||||
async fn malformed_rules_should_not_panic() -> anyhow::Result<()> {
|
||||
// run_codex_cli() does not work on Windows due to PTY limitations.
|
||||
if cfg!(windows) {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let tmp = tempfile::tempdir()?;
|
||||
let codex_home = tmp.path();
|
||||
std::fs::write(
|
||||
codex_home.join("rules"),
|
||||
"rules should be a directory not a file",
|
||||
)?;
|
||||
|
||||
// TODO(mbolin): Figure out why using a temp dir as the cwd causes this test
|
||||
// to hang.
|
||||
let cwd = std::env::current_dir()?;
|
||||
let config_contents = format!(
|
||||
r#"
|
||||
# Pick a local provider so the CLI doesn't prompt for OpenAI auth in this test.
|
||||
model_provider = "ollama"
|
||||
|
||||
[projects]
|
||||
"{cwd}" = {{ trust_level = "trusted" }}
|
||||
"#,
|
||||
cwd = cwd.display()
|
||||
);
|
||||
std::fs::write(codex_home.join("config.toml"), config_contents)?;
|
||||
|
||||
let CodexCliOutput { exit_code, output } = run_codex_cli(codex_home, cwd).await?;
|
||||
assert_eq!(1, exit_code, "Codex CLI should exit nonzero.");
|
||||
assert!(
|
||||
output.contains("ERROR: Failed to initialize codex:"),
|
||||
"expected startup error in output, got: {output}"
|
||||
);
|
||||
assert!(
|
||||
output.contains("failed to read execpolicy files"),
|
||||
"expected execpolicy read error in output, got: {output}"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
struct CodexCliOutput {
|
||||
exit_code: i32,
|
||||
output: String,
|
||||
}
|
||||
|
||||
async fn run_codex_cli(
|
||||
codex_home: impl AsRef<Path>,
|
||||
cwd: impl AsRef<Path>,
|
||||
) -> anyhow::Result<CodexCliOutput> {
|
||||
let codex_cli = codex_utils_cargo_bin::cargo_bin("codex")?;
|
||||
let mut env = HashMap::new();
|
||||
env.insert(
|
||||
"CODEX_HOME".to_string(),
|
||||
codex_home.as_ref().display().to_string(),
|
||||
);
|
||||
|
||||
let args = vec!["-c".to_string(), "analytics_enabled=false".to_string()];
|
||||
let spawned = codex_utils_pty::spawn_pty_process(
|
||||
codex_cli.to_string_lossy().as_ref(),
|
||||
&args,
|
||||
cwd.as_ref(),
|
||||
&env,
|
||||
&None,
|
||||
)
|
||||
.await?;
|
||||
let mut output = Vec::new();
|
||||
let mut output_rx = spawned.output_rx;
|
||||
let mut exit_rx = spawned.exit_rx;
|
||||
let writer_tx = spawned.session.writer_sender();
|
||||
let exit_code_result = timeout(Duration::from_secs(10), async {
|
||||
// Read PTY output until the process exits while replying to cursor
|
||||
// position queries so the TUI can initialize without a real terminal.
|
||||
loop {
|
||||
select! {
|
||||
result = output_rx.recv() => match result {
|
||||
Ok(chunk) => {
|
||||
// The TUI asks for the cursor position via ESC[6n.
|
||||
// Respond with a valid position to unblock startup.
|
||||
if chunk.windows(4).any(|window| window == b"\x1b[6n") {
|
||||
let _ = writer_tx.send(b"\x1b[1;1R".to_vec()).await;
|
||||
}
|
||||
output.extend_from_slice(&chunk);
|
||||
}
|
||||
Err(tokio::sync::broadcast::error::RecvError::Closed) => break exit_rx.await,
|
||||
Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => {}
|
||||
},
|
||||
result = &mut exit_rx => break result,
|
||||
}
|
||||
}
|
||||
})
|
||||
.await;
|
||||
let exit_code = match exit_code_result {
|
||||
Ok(Ok(code)) => code,
|
||||
Ok(Err(err)) => return Err(err.into()),
|
||||
Err(_) => {
|
||||
spawned.session.terminate();
|
||||
anyhow::bail!("timed out waiting for codex CLI to exit");
|
||||
}
|
||||
};
|
||||
// Drain any output that raced with the exit notification.
|
||||
while let Ok(chunk) = output_rx.try_recv() {
|
||||
output.extend_from_slice(&chunk);
|
||||
}
|
||||
|
||||
let output = String::from_utf8_lossy(&output);
|
||||
Ok(CodexCliOutput {
|
||||
exit_code,
|
||||
output: output.to_string(),
|
||||
})
|
||||
}
|
||||
|
|
@ -14,4 +14,7 @@ codex_rust_crate(
|
|||
),
|
||||
test_data_extra = glob(["src/**/snapshots/**"]),
|
||||
integration_compile_data_extra = ["src/test_backend.rs"],
|
||||
extra_binaries = [
|
||||
"//codex-rs/cli:codex",
|
||||
],
|
||||
)
|
||||
|
|
|
|||
|
|
@ -109,7 +109,10 @@ arboard = { workspace = true }
|
|||
|
||||
[dev-dependencies]
|
||||
assert_matches = { workspace = true }
|
||||
codex-cli = { workspace = true }
|
||||
codex-core = { workspace = true, features = ["test-support"] }
|
||||
codex-utils-cargo-bin = { workspace = true }
|
||||
codex-utils-pty = { workspace = true }
|
||||
chrono = { workspace = true, features = ["serde"] }
|
||||
insta = { workspace = true }
|
||||
pretty_assertions = { workspace = true }
|
||||
|
|
|
|||
|
|
@ -3,4 +3,7 @@
|
|||
#[cfg(feature = "vt100-tests")]
|
||||
mod test_backend;
|
||||
|
||||
#[allow(unused_imports)]
|
||||
use codex_cli as _; // Keep dev-dep for cargo-shear; tests spawn the codex binary.
|
||||
|
||||
mod suite;
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
// Aggregates all former standalone integration tests as modules.
|
||||
mod no_panic_on_startup;
|
||||
mod status_indicator;
|
||||
mod vt100_history;
|
||||
mod vt100_live_commit;
|
||||
|
|
|
|||
119
codex-rs/tui2/tests/suite/no_panic_on_startup.rs
Normal file
119
codex-rs/tui2/tests/suite/no_panic_on_startup.rs
Normal file
|
|
@ -0,0 +1,119 @@
|
|||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
use std::time::Duration;
|
||||
use tokio::select;
|
||||
use tokio::time::timeout;
|
||||
|
||||
/// Regression test for https://github.com/openai/codex/issues/8803.
|
||||
#[tokio::test]
|
||||
async fn malformed_rules_should_not_panic() -> anyhow::Result<()> {
|
||||
// run_codex_cli() does not work on Windows due to PTY limitations.
|
||||
if cfg!(windows) {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let tmp = tempfile::tempdir()?;
|
||||
let codex_home = tmp.path();
|
||||
std::fs::write(
|
||||
codex_home.join("rules"),
|
||||
"rules should be a directory not a file",
|
||||
)?;
|
||||
|
||||
// TODO(mbolin): Figure out why using a temp dir as the cwd causes this test
|
||||
// to hang.
|
||||
let cwd = std::env::current_dir()?;
|
||||
let config_contents = format!(
|
||||
r#"
|
||||
# Pick a local provider so the CLI doesn't prompt for OpenAI auth in this test.
|
||||
model_provider = "ollama"
|
||||
|
||||
[projects]
|
||||
"{cwd}" = {{ trust_level = "trusted" }}
|
||||
"#,
|
||||
cwd = cwd.display()
|
||||
);
|
||||
std::fs::write(codex_home.join("config.toml"), config_contents)?;
|
||||
|
||||
let CodexCliOutput { exit_code, output } = run_codex_cli(codex_home, cwd).await?;
|
||||
assert_eq!(1, exit_code, "Codex CLI should exit nonzero.");
|
||||
assert!(
|
||||
output.contains("ERROR: Failed to initialize codex:"),
|
||||
"expected startup error in output, got: {output}"
|
||||
);
|
||||
assert!(
|
||||
output.contains("failed to read execpolicy files"),
|
||||
"expected execpolicy read error in output, got: {output}"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
struct CodexCliOutput {
|
||||
exit_code: i32,
|
||||
output: String,
|
||||
}
|
||||
|
||||
async fn run_codex_cli(
|
||||
codex_home: impl AsRef<Path>,
|
||||
cwd: impl AsRef<Path>,
|
||||
) -> anyhow::Result<CodexCliOutput> {
|
||||
let codex_cli = codex_utils_cargo_bin::cargo_bin("codex")?;
|
||||
let mut env = HashMap::new();
|
||||
env.insert(
|
||||
"CODEX_HOME".to_string(),
|
||||
codex_home.as_ref().display().to_string(),
|
||||
);
|
||||
|
||||
let args = vec!["-c".to_string(), "analytics_enabled=false".to_string()];
|
||||
let spawned = codex_utils_pty::spawn_pty_process(
|
||||
codex_cli.to_string_lossy().as_ref(),
|
||||
&args,
|
||||
cwd.as_ref(),
|
||||
&env,
|
||||
&None,
|
||||
)
|
||||
.await?;
|
||||
let mut output = Vec::new();
|
||||
let mut output_rx = spawned.output_rx;
|
||||
let mut exit_rx = spawned.exit_rx;
|
||||
let writer_tx = spawned.session.writer_sender();
|
||||
let exit_code_result = timeout(Duration::from_secs(10), async {
|
||||
// Read PTY output until the process exits while replying to cursor
|
||||
// position queries so the TUI can initialize without a real terminal.
|
||||
loop {
|
||||
select! {
|
||||
result = output_rx.recv() => match result {
|
||||
Ok(chunk) => {
|
||||
// The TUI asks for the cursor position via ESC[6n.
|
||||
// Respond with a valid position to unblock startup.
|
||||
if chunk.windows(4).any(|window| window == b"\x1b[6n") {
|
||||
let _ = writer_tx.send(b"\x1b[1;1R".to_vec()).await;
|
||||
}
|
||||
output.extend_from_slice(&chunk);
|
||||
}
|
||||
Err(tokio::sync::broadcast::error::RecvError::Closed) => break exit_rx.await,
|
||||
Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => {}
|
||||
},
|
||||
result = &mut exit_rx => break result,
|
||||
}
|
||||
}
|
||||
})
|
||||
.await;
|
||||
let exit_code = match exit_code_result {
|
||||
Ok(Ok(code)) => code,
|
||||
Ok(Err(err)) => return Err(err.into()),
|
||||
Err(_) => {
|
||||
spawned.session.terminate();
|
||||
anyhow::bail!("timed out waiting for codex CLI to exit");
|
||||
}
|
||||
};
|
||||
// Drain any output that raced with the exit notification.
|
||||
while let Ok(chunk) = output_rx.try_recv() {
|
||||
output.extend_from_slice(&chunk);
|
||||
}
|
||||
|
||||
let output = String::from_utf8_lossy(&output);
|
||||
Ok(CodexCliOutput {
|
||||
exit_code,
|
||||
output: output.to_string(),
|
||||
})
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue