Closed
Conversation
nicoddemus
requested changes
Oct 17, 2023
Comment on lines
+1
to
+4
| The documentation for pytest.mark.xfail(): | ||
| reason=None: If no conditions are passed, reason’s default is "" (code). Adding @pytest.mark.xfail(reason=None, strict=True) to a passing test results in “TypeError: can only concatenate str (not "NoneType") to str”. If a condition is passed, passing no reason behaves like passing None to it (code). I’m unsure how this subtlety could be captured in the function signature. pytest.mark.skipif() has the same issue. pytest.mark.skip()’s default value for reason is in fact "unconditional skip" (code). | ||
| raises: The type (according to the type annotations) is Union[Type[BaseException], Tuple[Type[BaseException], ...]] instead of Type[Exception]. Neither captures the fact that None can be passed to it (which is also the documented default value). | ||
| strict=False: The default value of strict is actually determined by the xfail_strict config (code). |
Member
There was a problem hiding this comment.
The changelog should be written in a language targeting end users, not necessarily duplicating what was done in detail, so I suggest we go with:
Suggested change
| The documentation for pytest.mark.xfail(): | |
| reason=None: If no conditions are passed, reason’s default is "" (code). Adding @pytest.mark.xfail(reason=None, strict=True) to a passing test results in “TypeError: can only concatenate str (not "NoneType") to str”. If a condition is passed, passing no reason behaves like passing None to it (code). I’m unsure how this subtlety could be captured in the function signature. pytest.mark.skipif() has the same issue. pytest.mark.skip()’s default value for reason is in fact "unconditional skip" (code). | |
| raises: The type (according to the type annotations) is Union[Type[BaseException], Tuple[Type[BaseException], ...]] instead of Type[Exception]. Neither captures the fact that None can be passed to it (which is also the documented default value). | |
| strict=False: The default value of strict is actually determined by the xfail_strict config (code). | |
| Improved/fixed the documentation and signature for :func:`pytest.mark.xfail <pytest.mark.xfail>`. |
| unexpectedly passes then it will **fail** the test suite. This is particularly useful to mark functions | ||
| that are always failing and there should be a clear indication if they unexpectedly start to pass (for example | ||
| a new release of a library fixes a known bug). | ||
| * The value of `strict` is determined by the `xfail_strict` configuration. |
Member
There was a problem hiding this comment.
This is duplicated from line 262:
Suggested change
| * The value of `strict` is determined by the `xfail_strict` configuration. |
|
|
||
| Defaults to :confval:`xfail_strict`, which is ``False`` by default. | ||
|
|
||
| .. py:function:: pytest.mark.xfail(condition=None, *, reason=None, raises=None, run=True, strict=xfail_strict) |
Member
There was a problem hiding this comment.
This is duplicated from line 240 and should be removed.
Suggested change
| .. py:function:: pytest.mark.xfail(condition=None, *, reason=None, raises=None, run=True, strict=xfail_strict) |
Comment on lines
+249
to
+251
| If no conditions are passed, the default value of `reason` is an empty string (""). | ||
| However, please note that you should avoid passing `None` as the `reason`, as this can result in a TypeError. | ||
| If a condition is passed and no `reason` is specified, it behaves as if you passed `None` as the `reason`. |
Member
There was a problem hiding this comment.
I don't think we should be documenting this at all, we should just change the reason=None in line 240 to reason="" to reflect reality: users are not passing reason=None currently already as that would result in an error.
Suggested change
| If no conditions are passed, the default value of `reason` is an empty string (""). | |
| However, please note that you should avoid passing `None` as the `reason`, as this can result in a TypeError. | |
| If a condition is passed and no `reason` is specified, it behaves as if you passed `None` as the `reason`. |
5 tasks
Member
|
Closing this because @TanyaAgarwal28 hasn't been seen on GitHub for several months. Happy to reopen if it updates! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
This pull request addresses discrepancies in the documentation for
pytest.mark.xfail()and aligns it with the actual behavior in the code. The key points of this PR include:reasonargument, specifically addressing the use ofNone.raisesargument to match the code, allowing forNone.strictargument on thexfail_strictconfiguration.Fixes:
This PR resolves the issue #10094.
Documentation:
pytest.mark.xfail()to accurately represent its behavior based on the code.Tests:
Changelog:
changelog/10094.improvement.rstto document the improvement made in this PR.Author:
Tanya Agarwal