isolate codex transport tests from the developer's config - #150
Merged
Merged
Conversation
The codex_transport tests call clear_env(), which removes CCP_CONFIG_DIR,
and then call codex_transport() without pointing it at a temp dir. The
accessor falls back to paths::config_dir(), so the tests read the real
~/.config/claude-code-proxy/config.json. On any machine whose config sets
codex.transport to something other than websocket, the default test fails
and the poisoned ENV_LOCK takes the other four down with it.
Point each of the five tests at a tempfile::TempDir through
EnvGuard::set("CCP_CONFIG_DIR", ...), the same shape the other config
accessor tests in this module already use. The assertions are unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
|
Thanks for catching this! Merged. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The five
config::tests::codex_transport_*tests callclear_env(), which removesCCP_CONFIG_DIR, and then callcodex_transport()without pointing it at a temp dir. The accessor falls back topaths::config_dir(), so the tests read the developer's real~/.config/claude-code-proxy/config.json.On any machine whose config sets
codex.transportto something other than the default,cargo testfails:Only one of those is a real assertion failure — the shared
ENV_LOCKis then poisoned and takes the other four down with it, so a single config value presents as five failures.Worth noting:
CCP_CONFIG_DIR=$(mktemp -d) cargo testdoes not work around this, becauseclear_env()strips exactly that variable. The suite has to be isolated viaHOME, which is not obvious.Reproduce
Fix
Point each of the five tests at a
tempfile::TempDirviaEnvGuard::set("CCP_CONFIG_DIR", ...)— the same shapecodex_server_compaction_defaults_and_overridesand the other config accessor tests in the module already use. Assertions are unchanged, andclear_env()is deliberately left alone since other tests rely on the unset state.I checked every other
clear_env()caller in the module; they all either setCCP_CONFIG_DIRalready or useload_config_for_envwith an explicit dir, so this is the only instance.Verification
6 passed; 0 failedaftercargo clippy --all-targets -- -D warningsclean,cargo fmt --checkcleanHOME: green🤖 Generated with Claude Code