fix(#648): the documented conftest import does not work — use a marker instead#657
Merged
Merged
Conversation
…r instead PR #648 added `tests/conftest.py` and told test modules to do: from conftest import aws_creds_available That does not work, and I shipped it merged. There is a tracked **repo-root** `conftest.py`, so the bare name resolves to THAT file, which has no such function. The other obvious spelling, `from tests.conftest import ...`, fails as `tests.tests` under pytest's import mode here. Both were tried on CI by the #645 work and both failed — the exact class of breakage #648 exists to prevent, in #648 itself. Fix: make the guard reachable WITHOUT an import. * register a `requires_s3` marker in `pytest_configure`; * add `pytest_collection_modifyitems` to skip marked items when credentials are absent. A test module now writes: pytestmark = pytest.mark.requires_s3 and imports nothing. The docstring records both broken spellings so the next person does not re-derive them. Verified both directions: creds present : 1 passed (marker does not over-skip) creds absent : 1 skipped (CI's shape, reason reported) The automatic `pytest_runtest_makereport` net from #648 is unchanged and still catches modules that forget the marker entirely; this adds the explicit, declarative path that was advertised but unusable. Co-Authored-By: Claude Opus 4.8 (1M context) <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
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.
Self-inflicted, found by the #645 work on CI.
The bug
#648 added
tests/conftest.pyand documented:That resolves to the repo-root
conftest.py— which is tracked, exists, and has no such function. The other obvious spelling,from tests.conftest import …, fails astests.testsunder pytest’s import mode here.Both were tried on CI during the #645 fix and both failed. So the file whose entire purpose is to stop PRs going red for environmental non-reasons advertised an API that itself goes red. That agent worked around it by loading the module by path and — correctly — declined to patch shared test infra from inside an unrelated PR.
The fix: make it reachable without an import
requires_s3marker inpytest_configurepytest_collection_modifyitemsto skip marked items when credentials are absentA test module now writes, and imports nothing:
Verified both directions, because a guard that skips everywhere is worse than none:
1 passed— does not over-skip1 skipped, reason reportedThe automatic
pytest_runtest_makereportnet from #648 is untouched and still catches modules that forget the marker entirely. This adds the explicit, declarative path that was advertised but unusable.The docstring now records both broken spellings and why, so the next person does not re-derive them.
Refs #648, #645.