ref(integrations): collapse auto_test_scripts to a nox-level smoke - #738
Open
starfolkai[bot] wants to merge 3 commits into
Open
ref(integrations): collapse auto_test_scripts to a nox-level smoke#738starfolkai[bot] wants to merge 3 commits into
starfolkai[bot] wants to merge 3 commits into
Conversation
…ipts
Add `run_auto_smoke()` to `braintrust.integrations.test_utils` and rewrite 23
of the 29 `auto_test_scripts/` to use it. The helper encodes the standard
contract for `auto_instrument()` smoke tests: optional pre/post `is_patched`
check, first call returns `{name: True, ...}`, second call is idempotent, and
an optional memory-logger + VCR context that delegates to a per-script `run()`
callback for the API call and span-shape asserts.
Net -58 lines across the auto_test_scripts, but the real win is a single
authoritative definition of "auto_instrument works" that every simple
provider script now shares. Complex scripts that are mostly provider-specific
setup rather than boilerplate (agentscope, pipecat, agno) are left alone, as
are the `test_patch_litellm_*` scripts which exercise `patch_litellm()`
directly rather than `auto_instrument()`.
Verified: `nox -s pylint`, `nox -s test_core`, `nox -s test_litellm(latest)`
covering both the refactored `test_auto_litellm` and the untouched
`test_patch_litellm_*` scripts, plus pre-commit hooks on all changed files.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![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: 04be476fca
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".
Comment on lines
+176
to
+177
| second = auto_instrument(**kwargs) | ||
| assert second.get(name) is True, f"auto_instrument (2nd call) returned {second!r}" |
There was a problem hiding this comment.
When a repeated auto_instrument() call reports success but accidentally removes, replaces, or duplicates installed instrumentation, this helper still passes because is_patched() is only evaluated after the first call. Several converted scripts previously verified post-second-call state—including object identity for the CrewAI listener and LangChain handler—so the shared runner now allows idempotence regressions to escape; re-run an appropriate state verifier after the second call or retain the provider-specific identity assertions.
Useful? React with 👍 / 👎.
…ry-driven smoke Replace 26 near-identical per-provider `test_auto_*.py` scripts with one runner (`_run_smoke.py`) driven by an inline SMOKES registry, invoked from each `TestAutoInstrument*` via a new `verify_autoinstrument_smoke(name)` helper. The scripts were doing far more than a subprocess sanity check requires. The only bug class a fresh subprocess uniquely proves is "from a cold Python process, `auto_instrument()` successfully sets up this integration"; span shape, provider metadata, patching topology, and real API calls all live in the in-process `test_*.py` files for each integration. The runner asserts exactly that minimum: `auto_instrument(**kwargs).get(name) is True`, twice (idempotent). Net: −1222 LOC. The `test_patch_litellm_*` scripts stay put — they exercise `patch_litellm()` directly rather than `auto_instrument()`. The `run_auto_smoke` helper added in the previous commit is removed along with the scripts that used it. Verified: `nox -s pylint`, `nox -s "test_litellm(latest)" -- -k "test_auto_instrument_litellm or test_patch_litellm"` (3 passed), and pre-commit hooks on all changed files. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
starfolkai
Bot
changed the title
Abhijeet Prasad (AbhiPrasad)
changed the title
…-provider tests
Follow-up to the previous commit: instead of every provider test file carrying
a tiny `TestAutoInstrument*` class that spawns the same subprocess, hoist the
smoke into `_run_tests` in the noxfile. Each integration session already knows
which provider it's testing (via the test path), so it invokes
python -m braintrust.integrations.auto_test_scripts._run_smoke <name>
once before pytest starts. This works uniformly in src and wheel modes.
Deletes:
- 26 `TestAutoInstrument*` classes/functions and their imports across every
provider test file
- `verify_autoinstrument_smoke` from `test_utils` (no callers left)
- The `args` parameter I added to `verify_autoinstrument_script` in the last
commit (only `verify_autoinstrument_smoke` used it; reverted to the original
signature)
Adds:
- `_maybe_run_autoinstrument_smoke(session, test_path)` in noxfile.py — infers
the integration name from the test path, dedupes per (session, integration),
skips paths that aren't per-provider (e.g. test_core, test_versioning).
- `llamaindex` and `strands` to the SMOKES registry — they had no smoke scripts
before but do have `auto_instrument` entries, so they get free coverage now.
Cost: the smoke no longer runs under raw `pytest test_foo.py` — only under
`nox -s "test_foo(latest)"`. CI runs everything through nox so CI coverage is
unchanged; devs running pytest directly lose the local sanity check, which is
a fair trade for the −135 net LOC and one-line "add a new integration" story.
Verified: `nox -s "test_litellm(latest)" -- -k test_patch_litellm` prints
"python -m ..._run_smoke litellm\nSUCCESS" before pytest runs (2 passed),
`nox -s pylint` clean, `nox -s test_core` clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
starfolkai
Bot
changed the title
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
1 participant
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.
Summary
Replace 26 per-provider
test_auto_*.pyscripts, plus the 26TestAutoInstrument*classes in every provider test file, with:_run_smoke.py— one runner + inlineSMOKESregistry (kwargs per integration; onlylitellmneedsopenai=False)._maybe_run_autoinstrument_smoke()innoxfile.py— hoisted into_run_tests. Each session's_run_tests(session, test_path, ...)extracts the integration name fromtest_path(.../integrations/<name>/test_<name>.py→<name>) and invokespython -m braintrust.integrations.auto_test_scripts._run_smoke <name>once per (session, integration) before pytest starts.The reasoning
The scripts +
TestAutoInstrument*classes were doing far more than a subprocess sanity check requires. The only bug class a fresh subprocess uniquely proves is:Span shape, provider metadata, patching topology, and real API calls all live (and belong) in the in-process
test_*.pyfiles for each integration. The new runner asserts exactly the minimum:auto_instrumentinternally returnsTrueonly ifintegration.setup()returnedTrue(ImportError is swallowed to False, other exceptions logged), so a two-callTrue/Trueis already a strong signal that setup succeeded and is idempotent.Cost
The smoke no longer runs under raw
pytest test_foo.py— only undernox -s "test_foo(latest)". CI runs everything through nox so CI coverage is unchanged; devs running pytest directly lose the local sanity check, which is a fair trade for the ~1300 net LOC deleted and a "new integrations get smoke for free" story (add the name toSMOKESin_run_smoke.py— that's the only place).Bonus:
llamaindexandstrandsare added to the registry — they had no smoke scripts before but do haveauto_instrumententries, so they get free coverage now.Diff shape
Cumulative across the branch: 78 files changed, −1357 net lines (~+117 / −1474). One commit per logical step (initial shared helper → collapse to registry → hoist into nox).
What stays put
test_patch_litellm_responses.py,test_patch_litellm_aresponses.py: exercisepatch_litellm()directly, notauto_instrument().Test plan
cd py && nox -s pylint— cleancd py && nox -s "test_litellm(latest)" -- -k "test_patch_litellm"— 2 passed; nox output showspython -m ..._run_smoke litellm→SUCCESSbefore pytest startscd py && nox -s test_core— cleannoxmatrix will exercise the smoke for every integration session (nox calls the runner once per session before pytest).🤖 Generated with Claude Code
Created by Abhijeet Prasad (@AbhiPrasad)
Slack thread