fix(tests): move cli integration tests to codex-cli crate #9329
Conversation
Moved find_codex_home from codex-core (and duplicated in rmcp-client) to a new shared utility crate codex-utils-home. This resolves a TODO regarding code duplication and dependency structure.
Moved integration tests that depend on the codex binary from codex-core to codex-cli. This fixes test failures in clean environments where the binary was not yet built.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fda7200bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn cli_responses_fixture() -> std::path::PathBuf { | ||
| #[expect(clippy::expect_used)] | ||
| find_resource!("tests/cli_responses_fixture.sse").expect("failed to resolve fixture path") | ||
| find_resource!("../core/tests/cli_responses_fixture.sse").expect("failed to resolve fixture path") |
There was a problem hiding this comment.
In Cargo this relative path works, but under Bazel find_resource! resolves from RUNFILES_DIR + BAZEL_PACKAGE. The codex_rust_crate macro only adds tests/** as data for integration tests, and codex-rs/cli/BUILD.bazel doesn’t add extra test data, so ../core/tests/cli_responses_fixture.sse won’t be present in runfiles. When RUNFILES_DIR is set, the test will fail trying to read a non-existent file. Consider copying the fixture into codex-rs/cli/tests or adding the core fixture file to the CLI test’s data list.
Useful? React with 👍 / 👎.
|
@tavaresgmg, please see the code review comment from Codex. |
|
Closing because of lack of response from contributor |
3 participants
Description
Relocated
cli_streamintegration tests fromcodex-coretocodex-cli.Motivation
Tests in
codex-corewere failing in clean environments because they relied on thecodexbinary being present (viacargo_bin("codex")), butcargo test -p codex-coredoes not automatically build thecodex-clibinary.Moving these tests to
codex-cliensures the binary is built as part of the test lifecycle, resolving CI/local dev failures and "binary not found" panics.Changes
codex-rs/core/tests/suite/cli_stream.rstocodex-rs/cli/tests/cli_stream.rs.codex-rs/cli/Cargo.tomlwith necessary dev-dependencies (wiremock,uuid, etc.).Verification
cargo test -p codex-cli --test cli_streamsuccessfully.