Skip to content

Conversation

@henryiii
Copy link
Contributor

These xfails are being printed as tracebacks on every test run. Since they are known to not work, let's hide them for now.

@webknjaz
Copy link
Member

I think this is an incorrect use of xfail(run=False). Doing this would make their existence in the codebase pointless. The tracebacks are only shown in pytest 8, not older — perhaps, there's a report setting to simply hide them.

The idea is that the xfails may start reporting XPASS at some point if fixed accidentally.

Here's some of @pganssle's explanations on doing it properly: https://blog.ganssle.io/articles/2021/11/pytest-xfail.html / https://ganssle.io/talks/#xfail-and-skip / https://ganssle.io/talks/#xfail-lightning.

@henryiii
Copy link
Contributor Author

This was broken by pytest-dev/pytest#11233. And it was reported upstream in pytest-dev/pytest#12231, and fixed in pytest-dev/pytest#12280.

So we can either wait till a pytest release contains this fix, or temporarily turn them off.

I think this is an incorrect use of xfail(run=False)

That's why this change was bad; xfails are expected to fail, so printing an exception traceback explaining the known failure pollutes the logs and pretty much forces these to be turned off. It will is listed in the summary at the end (which is what the -rx part of -ra is for), so someone is still gently reminded (with three lines instead of hundreds) that these xfail tests exist and can be worked on. (The new version will have an optional flag to see the traceback, which is far better).

@webknjaz
Copy link
Member

@henryiii yes, so I'd suggest setting -rfEsX instead (excluding x there but including X). You'd correct the output which is what's annoying you but would still get the benefits of being alerted about XPASS (strict). On top of that, you could add some linting check for when the pytest version is higher than 8.2.2 there shouldn't be a non -ra reporting setting in the config. You could stick this into a conftest.py or an arbitrary *_test.py.

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii henryiii force-pushed the henryiii/tests/cleanup branch from c36a32a to bdfb0bd Compare June 11, 2024 20:16
@henryiii
Copy link
Contributor Author

I just added a todo, don't think we need anything too fancy, and we don't have to drop it the second pytest releases. ;) Otherwise followed your suggestion and put that sequence of letters in.

@henryiii henryiii merged commit 8d86d31 into pypa:main Jun 12, 2024
@henryiii henryiii deleted the henryiii/tests/cleanup branch June 12, 2024 14:46
m-richards added a commit to m-richards/cibuildwheel that referenced this pull request Jul 22, 2024
henryiii pushed a commit that referenced this pull request Jul 23, 2024
Revert "tests: don't print xfails (#1865)"

This reverts commit 8d86d31.
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.

2 participants