ci: verify adapter imports on a bare install - #7
Merged
Conversation
The `test` job installs .[dev], which pulls openeval-core[fixtura,langchain,openai], so every optional dependency is always present and the job cannot detect an adapter that has started requiring one. That is how the eager `from .langchain import ...` in openeval/adapters/__init__.py shipped: the suite stayed green the whole time while `pip install openeval-core` could not import any adapter at all. The fix in #5 removed the bug but left nothing to stop it coming back. Add a bare-install job that reproduces what a real user gets and asserts: - adapters with no third-party dependency import (fixtura, openai -- the openai adapter parses dicts and never imports the openai package) - the LangChain adapter fails only on access, and cleanly - an unknown attribute still raises AttributeError, so the lazy __getattr__ cannot silently swallow typos - the optional deps really are absent, so the job cannot pass vacuously if one ever becomes a base dependency Wired into all-tests-passed, which is the required check, so it actually gates. Verified against a clean venv: all assertions pass on current main, and reinstating the eager import makes the import assertion fail as intended. Closes #6 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Closes #6.
Problem
The
testjob installs.[dev], anddevpullsopeneval-core[fixtura,langchain,openai]. Every optional dependency is therefore always present in CI, so the suite cannot detect an adapter that has started requiring one.That is precisely how the bug fixed in #5 shipped.
openeval/adapters/__init__.pyeagerly imported the LangChain adapter, sopip install openeval-corecould not import any adapter — while every test stayed green. #5 removed the eager import but added nothing to stop it returning.Change
A
bare-installjob that reproduces what a real user gets (pip install ., no extras) and asserts four things:fixtura+openaiadapters importAttributeError__getattr__must not swallow typosWired into
all-tests-passed, the required check, so it actually gates rather than sitting decorative.One assumption corrected while writing it:
from_openai_messagesimports fine with no extras. That adapter parses message dicts and never imports theopenaipackage —openaiis in the extras for tests and examples only. The guard asserts that reality rather than a symmetry that doesn't exist.Verified in a clean venv
And — the part that matters — reinstating the old eager import makes it fail:
Restoring the lazy version makes it pass again. A guard that can't catch the regression it exists for is worse than none, so this was checked rather than assumed.
🤖 Generated with Claude Code