Skip to content

Commit 2a17b32

Browse files
authored
Stabilize marketplace/remove installedRoot test (#17721)
## Why This addresses the review comment from #17751 about `marketplace/remove` app-server test portability: #17751 (comment) The API returns the removed installed root using the app-server's effective `CODEX_HOME`. On macOS, temporary directory paths can appear as either `/var/...` or `/private/var/...`, so comparing one raw path against another can fail even when `marketplace/remove` behaves correctly. ## What changed - Removed the direct whole-response equality assertion for the installed root path. - Asserted the stable response field, `marketplace_name`, directly. - Compared the expected and returned installed-root paths after canonicalizing their existing parent directories, which avoids requiring the removed leaf directory to still exist. ## Verification - `cargo test -p codex-app-server marketplace_remove_deletes_config_and_installed_root` - `cargo test -p codex-app-server marketplace_remove`
1 parent 7171b25 commit 2a17b32

1 file changed

Lines changed: 19 additions & 9 deletions

File tree

codex-rs/app-server/tests/suite/v2/marketplace_remove.rs

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::time::Duration;
22

3+
use anyhow::Context;
34
use anyhow::Result;
45
use app_test_support::McpProcess;
56
use app_test_support::to_response;
@@ -10,7 +11,6 @@ use codex_app_server_protocol::RequestId;
1011
use codex_config::MarketplaceConfigUpdate;
1112
use codex_config::record_user_marketplace;
1213
use codex_core::plugins::marketplace_install_root;
13-
use codex_utils_absolute_path::AbsolutePathBuf;
1414
use pretty_assertions::assert_eq;
1515
use tempfile::TempDir;
1616
use tokio::time::timeout;
@@ -35,14 +35,23 @@ fn write_installed_marketplace(codex_home: &std::path::Path, marketplace_name: &
3535
Ok(())
3636
}
3737

38+
fn canonicalize_path_with_existing_parent(path: &std::path::Path) -> Result<std::path::PathBuf> {
39+
let parent = path
40+
.parent()
41+
.with_context(|| format!("path {} should have a parent", path.display()))?;
42+
let file_name = path
43+
.file_name()
44+
.with_context(|| format!("path {} should have a file name", path.display()))?;
45+
46+
Ok(parent.canonicalize()?.join(file_name))
47+
}
48+
3849
#[tokio::test]
3950
async fn marketplace_remove_deletes_config_and_installed_root() -> Result<()> {
4051
let codex_home = TempDir::new()?;
4152
record_user_marketplace(codex_home.path(), "debug", &configured_marketplace_update())?;
4253
write_installed_marketplace(codex_home.path(), "debug")?;
43-
let installed_root = marketplace_install_root(codex_home.path())
44-
.join("debug")
45-
.canonicalize()?;
54+
let installed_root = marketplace_install_root(codex_home.path()).join("debug");
4655

4756
let mut mcp = McpProcess::new(codex_home.path()).await?;
4857
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
@@ -59,12 +68,13 @@ async fn marketplace_remove_deletes_config_and_installed_root() -> Result<()> {
5968
)
6069
.await??;
6170
let response: MarketplaceRemoveResponse = to_response(response)?;
71+
assert_eq!(response.marketplace_name, "debug");
72+
let removed_installed_root = response
73+
.installed_root
74+
.context("marketplace/remove should return removed installed root")?;
6275
assert_eq!(
63-
response,
64-
MarketplaceRemoveResponse {
65-
marketplace_name: "debug".to_string(),
66-
installed_root: Some(AbsolutePathBuf::try_from(installed_root)?),
67-
}
76+
canonicalize_path_with_existing_parent(removed_installed_root.as_path())?,
77+
canonicalize_path_with_existing_parent(&installed_root)?,
6878
);
6979

7080
let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?;

0 commit comments

Comments
 (0)