Skip to content

Conversation

ogrisel
Copy link
Contributor

@ogrisel ogrisel commented Aug 27, 2025

Closes #13678.

This would help projects avoid wasting their precious CI resources when running extensive thread-safety tests (which will become more popular in the new free-threading era).

I also changed the logic to mark the test as XFAIL conditionally on the presence of the "CI" environment variable instead of always skipping (related to #7022). This makes it possible to check that the test pass as expected when running locally.

If you think this is too risky, I can remove the condition and always mark the offending params with the xfail mark instead of skipping.

EDIT: I will do the later right away because I have already observed such a random failure on the CI: https://github.com/pytest-dev/pytest/actions/runs/17269674978/job/49010727146

EDIT 2: that does not work because xfail_strict = true is configured in pyproject.toml. Let me skip instead...

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

At first glance this looks nice

There may be some nitpicks but they relate to preexisting patterns

@ogrisel
Copy link
Contributor Author

ogrisel commented Aug 27, 2025

I think I understand why the CI var was not properly detected: we probably need to add it to the passenv directive of tox. Let me try that quickly.

@ogrisel
Copy link
Contributor Author

ogrisel commented Aug 27, 2025

Ok so many other unrelated test started to fail because they are not written in a way that expect to be ever executed in the presence of the CI environment variable... Let me revert 000876e and 5066d98...

Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
@ogrisel
Copy link
Contributor Author

ogrisel commented Sep 3, 2025

I synced this PR with main as a follow-up on #13684. I think this PR is now ready for review. The matter of actually fixing #7022 is deferred to the exploratory #13695.

True,
marks=pytest.mark.skipif(
"CI" in os.environ, reason="sometimes crashes on CI (#7022)"
),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the skip mark to a conditional mark to make it easy to check that this test actually passes locally.

Copy link
Member

Choose a reason for hiding this comment

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

Can it be an xfail, then?

Copy link
Contributor Author

@ogrisel ogrisel Sep 4, 2025

Choose a reason for hiding this comment

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

I think I tried earlier, but this does not work because xfail_strict = true is configured in pyproject.toml and those tests would XPASS on most CI configurations except ubuntu-py314 that tends to fail for a reason I do not understand as investigated in #13695.

@ogrisel
Copy link
Contributor Author

ogrisel commented Sep 9, 2025

I synced this PR with main after the recent merge of #13695 to see if the new test also fails on ubuntu-py314 or not. If the new test fails here, I will add the same conditional skip as done for test_timeout in #13695

@ogrisel
Copy link
Contributor Author

ogrisel commented Sep 9, 2025

The new test did not fail, but let me trigger it a few more times to check that it's not (too) flaky.

EDIT: the new test is also flaky, selectively on ubuntu-py314. Let me push a fix to conditionally skip it in cfa2aa6.

@ogrisel
Copy link
Contributor Author

ogrisel commented Sep 9, 2025

I had a look at the codecov results and there is a single missing line:

https://app.codecov.io/gh/pytest-dev/pytest/pull/13679#f164a8d7864732ad2650984cfc20ed8e-R96

This line is actually covered in the new test, but the coverage data is not collected because the test uses a Python subprocess where coverage tracing is not enabled. I think collecting such coverage data is possible but out of the scope of the current PR.

@ogrisel
Copy link
Contributor Author

ogrisel commented Sep 9, 2025

One of the CI jobs for cfa2aa6 failed for an unrelated problem (a transient codecov upload error). All the non-skipped tests actually passed.

@ogrisel
Copy link
Contributor Author

ogrisel commented Sep 9, 2025

@RonnyPfannschmidt @webknjaz @nicoddemus I think this PR is ready for final review.

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

Thanks for delivering this as well as gracefully fixing prerequisites

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided (automation) changelog entry is part of PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make it possible to terminate pytest on deadlock after via the faulthandler timeout
3 participants