Background

#1679 included improvements to a number of tests that are known to fail on some platforms, by marking themxfail instead ofskip so they are still run and their status is reported, but without a failing status causing the whole test run to fail. However, it appliedxfail to too many tests, due to limitations on granularity when applyingpytest marks tounittest test cases generated by@ddt parameterization.

Upcoming impact

Although this was known and discussed in#1679, and FIXME comments about it were included in the code, the problem turns out to be somewhat more serious than I had anticipated: if not addressed, it will eventually lead to test failures in a future version ofpytest. This is because the default behavior of anunexpectedly passing test--one that is markedxfail but passes--will most likely change in pytest 8. Because GitPython does not specify upper bounds on most of its development dependencies, and pytest is one of the development dependencies for which no upper bound is specified, pytest 8 will be automatically installed once it is (stably) released.

Specifically, and in the absence of configuration or command-line options topytest that override the behavior:

• A test markedxfail that fails, and fails in the expected way, produces an XFAIL status, which is treated similarly to PASS. We always want this.
• A test markedxfail that fails in a detectably unexpected way--where a different exception results than the one that wasexpected--produces a FAIL status. We always want this.
• A test markedxfail thatpasses produces an XPASS status. How this status is treated is more complicated. Thexfail mark supportsan optionalstrict parameter. Where present, it determines whether the XPASS fails the test run like a FAIL status would, or does not fail the test run (thus behaving like PASS or XFAIL). If absent, thexfail_strict configuration option provides the default. Currently, as of pytest 7,xfail_strict defaults toFalse when not specified.

As noted inpytest-dev/pytest#11467, which was opened by a pytest maintainer and is listed for pytest's8.0 milestone, the default is planned to be changed fromFalse toTrue starting in pytest 8.0. (See alsopytest-dev/pytest#11499.)

Possible fixes

Breakage could be avoided (at least for a while, sincestrict=Falsemay eventually be removed as a feature) by passingstrict=False or settingxfail_strict=false forpytest inpyproject.toml. It is also possible to set an upper bound like<8 forpytest intest-requirements.txt.

However, I recommend this instead be fixed by reorganizing the tests intest_util.py so that the tests ofcygpath anddecygpath--which are the ones that have the insufficiently precisexfail markings that mark some generated test casesxfail even though they are known to pass--can be purepytest tests. Because they are currentlyunittest tests, they cannot be generated by@pytest.mark.parametrize (hence@ddt is used). But if they could be generated with theparametrize mark then they could have per-case markings, becauseparametrize supports an optionalmarks argument. They could then have thexfail mark applied to exactly the cases where failure is really expected.

That approach – which I mentioned in#1679 itself and in#1700 (comment), and more recently alluded to in#1725 and#1726 (comment) – has the following advantages over other approaches that effectively just suppress the problem:

• Any XPASS will be a sign that something has changed and should be looked into, thereby building on the improvements in#1679.
• Although we have FIXME comments, the current situation is still misleading in the test results themselves, which indicate that some tests are unexpectedly passing.
• When the default treatment of XPASS inpytest changes--but also even before that, once it is documented to change--the presence of expected XPASSes will be more misleading than it is already, even if GitPython is not using a version ofpytest affected by the change. This is because that change will further solidify people's expectations about what XPASS indicates, including for people who are trying to become familiar with GitPython.
• Reorganizing the tests intest_util.py can also help clarify the tests ofrmtree behavior, and help make them easier to modify. This is useful because it will allow building on#1700 toward an eventual complete fix for#790. (In addition, I want to make sure theplanned native Windows CI jobs don't have the effect of calcifying cleanup logic inrmtree that otherwise could or should change, or at least that this does not happen in ways that impinge on non-Windows platforms. I think such a reorganization will help with that, too.)

I have opened#1729, which fixes this issue by reorganizing tests intest_util.py in this way.