search

pypa / cibuildwheel

🎡 Build Python wheels for all the platforms with minimal configuration.
https://cibuildwheel.pypa.io
Other
1.84k stars 235 forks source link

tests: don't print xfails #1865

Closed henryiii closed 3 months ago

henryiii commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

This was broken by https://github.com/pytest-dev/pytest/issues/11233. And it was reported upstream in https://github.com/pytest-dev/pytest/issues/12231, and fixed in https://github.com/pytest-dev/pytest/pull/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 commented 3 months ago

@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.

henryiii commented 3 months ago

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.