← 返回首页
Defer xfail condition evaluation with xfail_if_raises context manager by elovelan · Pull Request #2153 · gitpython-developers/GitPython · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Resetting focus

Defer xfail condition evaluation with xfail_if_raises context manager#2153

Merged
Byron merged 1 commit into
gitpython-developers:mainfrom
elovelan:fix/xfail_side_effects
May 16, 2026
Merged

Defer xfail condition evaluation with xfail_if_raises context manager#2153
Byron merged 1 commit into
gitpython-developers:mainfrom
elovelan:fix/xfail_side_effects

Conversation

Copy link
Copy Markdown
Contributor

elovelan commented May 15, 2026
edited
Loading

Summary

This PR introduces xfail_if_raises, a context manager that approximates @pytest.mark.xfail(..., raises=...) behavior inside a test body rather than as a decorator.

Problem

@pytest.mark.xfail evaluates its condition argument during test collection (module import time). In test_index_mutation, this condition includes Git().config("core.symlinks") == "true", which:

  1. Spawns an external git config process even when the test is not being run
  2. Can fail or behave unexpectedly during collection if git configuration is unavailable
  3. Relies on in-repo code during test discovery, which may not be appropriate in all contexts

Solution

xfail_if_raises moves the xfail logic into the test body. The side-effectful condition is only evaluated if the test actually raises the specified exception(s). This keeps test collection lightweight and avoids relying on in-repo code as part of module import/test discovery.

This change is applied to test_index_mutation in test/test_index.py.

AI disclosure

This PR description was prepared with assistance from an agent. All code was written by hand.

elovelan marked this pull request as draft May 15, 2026 15:52
The @pytest.mark.xfail decorator with raises=... evaluates its condition during test collection, which can trigger external calls (e.g., Git().config()) that have side effects. Introduce xfail_if_raises as a context manager that delays this evaluation until test execution time, and apply it to test_index_mutation.
elovelan force-pushed the fix/xfail_side_effects branch from 46daafe to 03baced Compare May 15, 2026 20:29
elovelan marked this pull request as ready for review May 15, 2026 20:38
Copy link
Copy Markdown
Contributor Author

elovelan commented May 15, 2026
edited
Loading

I didn't change anything that should have affected this one failed test on Cygwin. I have a Windows VM now so I'll see if I can repro.

Copy link
Copy Markdown
Member

The failed Cygwin test looks like a known flake with submodule ownership on Cygwin CI that I've been working on and hope to have a PR in for soon. To the best of my knowledge, that is independent of the changes here, though I emphasize that I have not reviewed this PR. I've triggered the test to rerun--my guess is that it will pass.

Copy link
Copy Markdown
Member

Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Thanks a lot, looks good to me!

Initially I was confused about strict as it's not actually used, but it's Ok and nothing one couldn't figure out and address next time someone gets confused by it.

Hide details View details Byron merged commit 9c5b57b into gitpython-developers:main May 16, 2026
31 of 32 checks passed
Copy link
Copy Markdown
Contributor Author

Initially I was confused about strict as it's not actually used, but it's Ok and nothing one couldn't figure out and address next time someone gets confused by it.

I debated a YAGNI approach, but I figured as a utility method, I should try to mirror the decorator as closely as I could.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Footer

© 2026 GitHub, Inc.