From f73a07224653c2cc775b3f84f129b872b1e08f85 Mon Sep 17 00:00:00 2001 From: "Adam Perry @ OpenAI" Date: Wed, 8 Jul 2026 10:50:18 -0700 Subject: [PATCH] test: remove TestAppServer constructors (#31452) ## Why Finish the TestAppServer builder migration after every caller has moved off the compatibility constructors. ## What - remove the obsolete public TestAppServer constructors - leave TestAppServer::builder() as the only fixture construction API ## Validation - cargo check -p codex-app-server --tests ## Cleanup stack 1. [#31425 test: add TestAppServer builder](https://github.com/openai/codex/pull/31425) 2. [#31451 test: migrate TestAppServer callers to builder](https://github.com/openai/codex/pull/31451) 3. this PR --- AGENTS.md | 2 +- .../tests/common/test_app_server.rs | 134 +----------------- .../app-server/tests/suite/v2/app_list.rs | 6 +- 3 files changed, 13 insertions(+), 129 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index bdfa798f5c..faa57cc0db 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -253,7 +253,7 @@ Use `just bench-smoke` to dry-run the benchmark for a single iteration to ensure - Tests should exercise app-server's public JSON-RPC API. - Use similar server mocking as for core integration tests. -- Use `TestAppServer::new_with_auto_env()` and `TestAppServer::send_thread_start_request_with_auto_env()` +- Use `TestAppServer::builder().build()` and `TestAppServer::send_thread_start_request_with_auto_env()` by default to ensure that new tests work with foreign app/exec OSes. See `$remote-tests` for details. diff --git a/codex-rs/app-server/tests/common/test_app_server.rs b/codex-rs/app-server/tests/common/test_app_server.rs index e080ab0261..c41b2e7c19 100644 --- a/codex-rs/app-server/tests/common/test_app_server.rs +++ b/codex-rs/app-server/tests/common/test_app_server.rs @@ -165,55 +165,19 @@ impl TestAppServer { self.process.wait().await } - pub async fn new(codex_home: &Path) -> anyhow::Result { - Self::builder() - .with_codex_home(codex_home) - .without_auto_env() - .build() - .await - } - - /// Starts an app server with the standard test environment and retains it - /// for the server's lifetime. - /// - /// Local test runs explicitly remove `CODEX_EXEC_SERVER_URL`; Docker- and - /// Wine-backed runs set it to the remote fixture URL. Use - /// [`Self::auto_env_params`] or - /// [`Self::send_thread_start_request_with_auto_env`] to select the matching - /// target-native cwd in a thread. Because `environments.toml` overrides the - /// URL-based configuration, this helper rejects a `codex_home` containing - /// that file. - pub async fn new_with_auto_env(codex_home: &Path) -> anyhow::Result { - Self::builder().with_codex_home(codex_home).build().await - } - - /// Starts an auto-environment app server that emits JSON logs. - /// - /// `rust_log` is the value to use for the `RUST_LOG` environment variable. - pub async fn new_with_auto_env_and_json_logging( - codex_home: &Path, - rust_log: impl Into, - ) -> anyhow::Result { - Self::builder() - .with_codex_home(codex_home) - .with_json_logging(rust_log) - .build() - .await - } - /// Returns the automatically selected test environment retained by this server. /// /// Tests can use the environment to arrange target-native filesystem fixtures before starting - /// a thread. Returns an error unless this server was created with [`Self::new_with_auto_env`]. + /// a thread. Returns an error unless the builder's automatic environment is enabled. pub fn auto_env(&self) -> anyhow::Result<&TestEnv> { self.auto_env .as_ref() - .context("auto environment is unavailable; use TestAppServer::new_with_auto_env") + .context("auto environment is unavailable; enable it on TestAppServer::builder") } /// Returns app-server protocol parameters for the automatically selected - /// test environment. Returns an error unless this server was created with - /// [`Self::new_with_auto_env`]. + /// test environment. Returns an error unless the builder's automatic + /// environment is enabled. pub fn auto_env_params(&self) -> anyhow::Result { let selection = self.auto_env()?.selection(); Ok(TurnEnvironmentParams { @@ -249,90 +213,6 @@ impl TestAppServer { self.json_logs.events() } - pub async fn new_without_managed_config(codex_home: &Path) -> anyhow::Result { - Self::builder() - .with_codex_home(codex_home) - .without_auto_env() - .without_managed_config() - .build() - .await - } - - pub async fn new_without_managed_config_with_env( - codex_home: &Path, - env_overrides: &[(&str, Option<&str>)], - ) -> anyhow::Result { - Self::builder() - .with_codex_home(codex_home) - .without_auto_env() - .without_managed_config() - .with_env_overrides(env_overrides) - .build() - .await - } - - pub async fn new_with_plugin_startup_tasks(codex_home: &Path) -> anyhow::Result { - Self::builder() - .with_codex_home(codex_home) - .without_auto_env() - .with_plugin_startup_tasks() - .build() - .await - } - - pub async fn new_with_env_and_plugin_startup_tasks( - codex_home: &Path, - env_overrides: &[(&str, Option<&str>)], - ) -> anyhow::Result { - Self::builder() - .with_codex_home(codex_home) - .without_auto_env() - .with_plugin_startup_tasks() - .with_env_overrides(env_overrides) - .build() - .await - } - - pub async fn new_with_args(codex_home: &Path, args: &[&str]) -> anyhow::Result { - Self::builder() - .with_codex_home(codex_home) - .without_auto_env() - .with_args(args) - .build() - .await - } - - /// Creates a new MCP process, allowing tests to override or remove - /// specific environment variables for the child process only. - /// - /// Pass a tuple of (key, Some(value)) to set/override, or (key, None) to - /// remove a variable from the child's environment. - pub async fn new_with_env( - codex_home: &Path, - env_overrides: &[(&str, Option<&str>)], - ) -> anyhow::Result { - Self::builder() - .with_codex_home(codex_home) - .without_auto_env() - .with_env_overrides(env_overrides) - .build() - .await - } - - pub async fn new_with_program_and_env( - codex_home: &Path, - program: &Path, - env_overrides: &[(&str, Option<&str>)], - ) -> anyhow::Result { - Self::builder() - .with_codex_home(codex_home) - .without_auto_env() - .with_program(program) - .with_env_overrides(env_overrides) - .build() - .await - } - async fn new_with_program_env_and_args( codex_home: &Path, program: &Path, @@ -579,9 +459,9 @@ impl TestAppServer { self.send_request("thread/start", params).await } - /// Sends a `thread/start` request selecting the environment provisioned by - /// [`Self::new_with_auto_env`]. Returns an error if `params` already select - /// environments so the caller cannot accidentally override the fixture. + /// Sends a `thread/start` request selecting the builder's automatic + /// environment. Returns an error if `params` already select environments + /// so the caller cannot accidentally override the fixture. pub async fn send_thread_start_request_with_auto_env( &mut self, mut params: ThreadStartParams, diff --git a/codex-rs/app-server/tests/suite/v2/app_list.rs b/codex-rs/app-server/tests/suite/v2/app_list.rs index 35c21e4c68..a5e589be42 100644 --- a/codex-rs/app-server/tests/suite/v2/app_list.rs +++ b/codex-rs/app-server/tests/suite/v2/app_list.rs @@ -206,7 +206,11 @@ async fn list_apps_uses_external_chatgpt_auth() -> Result<()> { let codex_home = TempDir::new()?; write_connectors_config(codex_home.path(), &server_url)?; - let mut mcp = TestAppServer::new(codex_home.path()).await?; + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build() + .await?; timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; let login_id = mcp .send_chatgpt_auth_tokens_login_request(