Repository navigation
chore(tests): clear ANTHROPIC_BASE_URL in the credential tests' env isolation - #2021
alphasafal wants to merge 1 commit into
Conversation
chrikrah
left a comment
chrikrah
left a comment
There was a problem hiding this comment.
@alphasafal this is right as it stands at 941c0a1. Your before counts reproduce in a clean environment (after, I get 292 passed against your 291), and fill(config, "base_url", ENV_BASE_URL) at _providers.py:101 is the read that leaks the shell value into the profile.
$ env -i HOME=<empty> ANTHROPIC_BASE_URL=https://gateway.example python -m pytest tests/lib/test_credentials.py -q -n 4
2a53eaa (base): 49 failed, 243 passed, 2 skipped, 48 errors
941c0a1: 292 passed, 2 skipped
$ env -i HOME=<empty> ANTHROPIC_BASE_URL=https://api.anthropic.com python -m pytest tests/lib/test_credentials.py -q -n 4 # 2a53eaa
4 failed, 288 passed, 2 skipped, 3 errors
$ env -i HOME=<empty> ANTHROPIC_BASE_URL=https://gateway.example python -m pytest tests/lib -q -n 4 # 941c0a1
2161 passed, 10 skipped, 1 xfailed
non-blocking: the list missed ANTHROPIC_BASE_URL from the start. Building it from the module keeps it in step with _constants.py:
from anthropic.lib.credentials import _constants as _credential_constants
_ALL_ENV = [v for k, v in vars(_credential_constants).items() if k.startswith("ENV_")]That yields the same 12 names, and the file gives 292 passed, 2 skipped with the gateway variable set. The list came from 3be1e89 without BASE_URL, and 119e123 later added WORKSPACE_ID to both files by hand.
@dtmeadows-ant I'd merge the one-line version now and leave the derived list as a follow-up, since it touches nothing outside the test file. Is that fine on your side?
|
Thanks for reproducing it, @chrikrah. Your clean-env counts are the ones to go by; I'll use Checking the rest of |
_ALL_ENV in tests/lib/test_credentials.py was a hand-kept copy of the ENV_* names in lib/credentials/_constants.py. It never had ANTHROPIC_BASE_URL, so a value exported in the shell leaked into the profiles under test. Build the list from the module instead, so a new variable added there is cleared here automatically. Follow-up to anthropics#2021, as suggested in review.
|
@dtmeadows-ant following up on @chrikrah's question above: is the one-line version OK to merge as is? The derived |
|
The CI run on this branch expired before it was approved, so no checks have run on it yet. @chrikrah @dtmeadows-ant could one of you approve a fresh run when you get a chance? Thanks! |
_ALL_ENVintests/lib/test_credentials.pyis meant to mirror theENV_*constants inlib/credentials/_constants.py, but it's missingANTHROPIC_BASE_URL.CredentialsFilefills a profile's missingbase_urlfrom that variable, so if it's set in your shell (e.g. behind a gateway), token exchanges go to your host instead of the one the tests mock.Running
tests/lib/test_credentials.pywithANTHROPIC_BASE_URL=https://gateway.example:It also breaks 4 tests when set to
https://api.anthropic.com. This only changes the tests, so runtime behaviour is untouched. The fulltests/libsuite passes with the variable set, and ruff and pyright are clean.