Skip to content

Fix pytest 9.1 plugin import: marks on fixtures are now an error#537

Merged
tony merged 7 commits into
masterfrom
pytest-9.1-deprecations
Jun 20, 2026
Merged

Fix pytest 9.1 plugin import: marks on fixtures are now an error#537
tony merged 7 commits into
masterfrom
pytest-9.1-deprecations

Conversation

@tony

@tony tony commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • Fix: libvcs.pytest_plugin now imports cleanly under pytest 9.1, which promoted "marks applied to fixtures" from a deprecation warning to a hard error. Because the plugin ships as a pytest11 entry point, the error fired at plugin import — aborting the entire test session before collection for any project that has libvcs installed and runs pytest 9.1+ (e.g. vcspull).
  • Replace: the no-op @skip_if_*_missing mark decorators stacked on 16 Git/Mercurial/Subversion fixtures with in-body pytest.skip() guards (_skip_if_git_missing / _skip_if_svn_missing / _skip_if_hg_missing). Applying a skipif mark to a fixture never had any effect; moving the check into the fixture body makes a missing binary skip dependent tests instead of erroring.
  • Keep: the public skip_if_git_missing / skip_if_svn_missing / skip_if_hg_missing marks unchanged — they remain valid for decorating test functions and importable by downstream suites.

Design decisions

  • In-body guard, not just mark removal: unlike the equivalent libtmux fix (where the conditional already lived in conftest.py), libvcs's fixtures actually invoke the VCS binaries, so deleting the marks alone would turn "skip when missing" into a fixture error. The repo already used this exact in-body pattern in empty_svn_repo; this generalizes it and folds that fixture's inline guard into the shared helper.
  • svn helper checks both svn and svnadmin: the svn fixtures call svnadmin, matching the pre-existing inline guard's semantics.
  • No pytest version ceiling: the plugin ships to downstream users who control their own pytest version; pinning would mask the defect rather than fix it.

Verification

No mark decorators remain on fixtures:

$ rg '^\s*@skip_if_(git|svn|hg)_missing' src/libvcs/pytest_plugin.py

The plugin imports without raising:

$ uv run python -c "import libvcs.pytest_plugin"

Fixtures skip (not error) when a binary is absent:

$ PATH="$PWD/.venv/bin:/tmp/nosvn_bin" .venv/bin/python -m pytest tests/sync/test_svn.py -rs

Test plan

  • uv run python -c "import libvcs.pytest_plugin" — plugin imports cleanly under pytest 9.1
  • uv run pytest — full suite green
  • uv run ruff check . — lint clean
  • uv run ruff format . --check — formatting clean
  • uv run mypy — type check clean
  • svn fixtures report SKIPPED ... svn is not available (not an error) when svn/svnadmin are hidden from PATH

@codecov

codecov Bot commented Jun 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 48.27586% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.98%. Comparing base (54f7447) to head (190dcbb).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/libvcs/pytest_plugin.py 48.27% 12 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #537      +/-   ##
==========================================
+ Coverage   58.51%   60.98%   +2.47%     
==========================================
  Files          40       40              
  Lines        6523     6531       +8     
  Branches     1099     1101       +2     
==========================================
+ Hits         3817     3983     +166     
+ Misses       2176     1951     -225     
- Partials      530      597      +67     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tony

tony commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

1 similar comment
@tony

tony commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

tony added 2 commits June 20, 2026 10:26
why: pytest 9.1 rejects marks applied to fixture functions and raises
the error at plugin import, before collection. Because libvcs ships
this plugin as an installed pytest11 entry point, the error aborted the
entire test session for any downstream project (e.g. vcspull) running
pytest 9.1+. The skipif marks stacked on the fixtures were no-ops in
every prior pytest version; the real gating must run inside each fixture
so a missing binary skips instead of erroring.

what:
- Add private _skip_if_git_missing / _skip_if_svn_missing /
  _skip_if_hg_missing helpers that call pytest.skip() when the binary is
  absent; the svn helper requires both svn and svnadmin
- Remove the @skip_if_*_missing decorators from the affected fixtures and
  call the matching helper as the first body statement
- Fold empty_svn_repo's existing inline svn/svnadmin guard into the
  shared helper
- Keep the public skip_if_*_missing marks: they remain valid for
  decorating test functions and importable by downstream suites
why: Tell downstream users that the plugin now loads on pytest 9.1+, so
they can upgrade pytest without the session aborting before collection.
what:
- Add a Fixes entry under the 0.43.x unreleased block
@tony tony force-pushed the pytest-9.1-deprecations branch 3 times, most recently from dcecb41 to 87b487f Compare June 20, 2026 15:43
@tony

tony commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

@tony tony force-pushed the pytest-9.1-deprecations branch from 87b487f to cb14f80 Compare June 20, 2026 16:45
@tony

tony commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

tony added 5 commits June 20, 2026 12:12
why: The entry narrated implementation mechanism (how the checks moved,
that pytest 9.1 rejects marks on fixtures) -- diff narration in a
shipped artifact per AGENTS.md AI Slop Prevention -- and scoped the
impact to fixture users. The plugin loads at pytest startup for any
project with libvcs installed, so the abort reaches more than fixture
users.

what:
- Drop the mechanism clauses; keep the user-visible outcome
- Broaden the audience to any project with libvcs installed
…inary

why: vcs_gitconfig, set_vcs_gitconfig, vcs_hgconfig and set_vcs_hgconfig
only write a .gitconfig / .hgrc or set env vars -- they never invoke
git or hg. Guarding them on the binary made them skip when it was
absent. Because conftest.py's autouse `setup` fixture depends on
vcs_gitconfig, a host without git would skip the ENTIRE suite, including
svn- and hg-only tests that don't need git at all. The binary-invoking
fixtures (empty_git_repo, git_remote_repo, ...) keep their guards, so
tests that actually run a binary still skip correctly.

what:
- Remove _skip_if_git_missing() from vcs_gitconfig and set_vcs_gitconfig
- Remove _skip_if_hg_missing() from vcs_hgconfig and set_vcs_hgconfig
why: add_doctest_fixtures declared the per-VCS repo factories
(create_git_remote_repo, create_svn_remote_repo, create_hg_remote_repo,
git_repo) as parameters, so pytest instantiated all of them before the
body's `if shutil.which(...)` guards ran. Each factory pulls in a
binary-invoking fixture, so a single missing binary (e.g. hg) skipped
the whole fixture and therefore every doctest -- including doctests for
the binaries that were present.

what:
- Drop the per-VCS fixtures from the signature; request them via
  request.getfixturevalue() inside each `if shutil.which(...)` block
- A missing binary now drops only that VCS's doctest helpers; doctests
  for the present binaries still run
why: The svn fixtures build repositories with `svnadmin`, and
_skip_if_svn_missing() treats svn as available only when both the svn
client and svnadmin are present. The public skip_if_svn_missing mark and
the pytest_ignore_collect hook checked the svn client alone, so on a
host with svn but not svnadmin they disagreed with the fixtures: a
mark-decorated test would run, and svn doctest modules would be
collected while their create_svn_remote_repo namespace helper -- which
also requires svnadmin -- was withheld, erroring with NameError instead
of skipping. Make every svn-availability check require svnadmin.

what:
- Add the svnadmin check to the skip_if_svn_missing mark condition
- Require svnadmin in pytest_ignore_collect so svn modules are ignored
  (not collected) when only the svn client is present
why: git 2.54 changed `git remote set-head` output: when origin/HEAD
already points at the branch it prints "'origin/HEAD' is unchanged and
points to 'master'" instead of "origin/HEAD set to master". The doctest
asserted the older literal string, so it failed under the git on CI.
The failure is independent of the pytest 9.1 fixture changes -- it was
latent until the plugin could import and the suite actually ran.

what:
- Assert version-stable invariants instead of the literal message:
  `'master' in set_head(auto=True)` and that `set_head('master')`
  returns a str
@tony tony force-pushed the pytest-9.1-deprecations branch from cb14f80 to 190dcbb Compare June 20, 2026 17:13
@tony tony merged commit 77d912f into master Jun 20, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant