Skip to content

Fix remote environment test fixtures#22572

Merged
starr-openai merged 2 commits into
mainfrom
baseline-remote-tests-20260513-143528
May 14, 2026
Merged

Fix remote environment test fixtures#22572
starr-openai merged 2 commits into
mainfrom
baseline-remote-tests-20260513-143528

Conversation

@starr-openai

@starr-openai starr-openai commented May 13, 2026

Copy link
Copy Markdown
Contributor

Why

The Docker remote-env coverage was failing before it reached the behavior those tests are meant to exercise. The remote-aware test fixture only registered the remote environment, so tests that intentionally select both local and remote could not start a turn. After that was fixed, two tests exposed stale fixtures: the approval test was auto-approving under workspace-write, and the remote view_image test was writing invalid PNG bytes.

What Changed

  • Added EnvironmentManager::create_for_tests_with_local(...) so tests can keep the provider default while also selecting local explicitly.
  • Updated build_remote_aware() to use that test-only manager when a remote exec-server URL is present.
  • Changed the remote apply-patch approval helper to use SandboxPolicy::new_read_only_policy() so the test actually exercises approval caching per environment.
  • Replaced the hardcoded remote view_image PNG blob with the existing png_bytes(...) helper so the test uses a valid image fixture.

Validation

Ran these isolated Docker remote-env tests on the devbox with $remote-tests setup:

  • suite::remote_env::apply_patch_freeform_routes_to_selected_remote_environment
  • suite::remote_env::apply_patch_approvals_are_remembered_per_environment
  • suite::remote_env::apply_patch_intercepted_exec_command_routes_to_selected_remote_environment
  • suite::remote_env::exec_command_routes_to_selected_remote_environment
  • suite::view_image::view_image_routes_to_selected_remote_environment

All five pass.

@starr-openai starr-openai requested a review from a team as a code owner May 13, 2026 23:53
Comment thread codex-rs/core/tests/common/test_codex.rs Outdated
);
let environment_manager = Arc::new(match exec_server_url {
Some(exec_server_url) => {
codex_exec_server::EnvironmentManager::create_for_tests_with_local(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not always use this method?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It already takes optional

}

pub async fn build_remote_aware(
pub async fn build_with_remote_env(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this doesn't always build remote

it's either local or remode depending on CI job

Comment thread codex-rs/core/tests/suite/view_image.rs
@starr-openai starr-openai merged commit 2557486 into main May 14, 2026
31 checks passed
@starr-openai starr-openai deleted the baseline-remote-tests-20260513-143528 branch May 14, 2026 19:40
@github-actions github-actions Bot locked and limited conversation to collaborators May 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants