Add option to use strict xfail mode in parametrize_with_checks
#31951
Conversation
In strict mode unexpectedly passing tests lead to a test failure. This helps keep the xfail list up to date.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
doc/whats_new/upcoming_changes/sklearn.utils/31951.enhancement.rst
Outdated
Show resolved
Hide resolved
sklearn/utils/estimator_checks.py
Outdated
| @@ -424,6 +424,7 @@ def _maybe_mark( | |||
| expected_failed_checks: dict[str, str] | None = None, | |||
| mark: Literal["xfail", "skip", None] = None, | |||
| pytest=None, | |||
| strict: bool | None = None, | |||
There was a problem hiding this comment.
Maybe it would be a bit clearer to call the parameter xfail_strict (same name as the pytest config)?
| strict: bool | None = None, | |
| xfail_strict: bool | None = None, |
There was a problem hiding this comment.
Good idea.
I also changed it for the "higher level" functions like estimator_checks_generator and parametrize_with_checks. I like having it consistent between them, even though I find xfail_strict a bit clunky.
To make sure I follow, you are saying that doing |
Co-authored-by: Loïc Estève <loic.esteve@ymail.com>
Co-authored-by: Loïc Estève <loic.esteve@ymail.com>
More consistent with the name of the pytest config option
This is what I meant. The thing I didn't love was that the tests won't tell you this, you have to know it (or try it out). Basically this whole section |
Reference Issues/PRs
Fixes #31931
What does this implement/fix? Explain your changes.
This adds an option to
parametrize_with_checks(and the functions it uses) that allow you to specify "strict mode" for xfailed tests. In strict mode unexpectedly passing tests lead to a test failure. This helps keep the xfail list up to date.You can configure this globally via
pytest.inifor the whole test suite or for individual tests via thexfailmarker.Any other comments?
In cuml we have a lot of common checks that fail and hence are marked as xfail. As we go through fixing things it would be good to have strict mode enabled to help keep the xfail list up to date. We don't really want to turn on strict mode for the whole test suite though (this would be a bigger discussion).
I am not super happy with the testing of this testing infrastructure. For example I only noticed that passing
strict=Noneto the mark overrides the config frompytest.initby trying it out for real, I couldn't figure out a test that would catch it (except the explicit one I added). Ideas welcome, also comments that say "this looks super good enough" :D