Skip to content

Make xpass failure again #11498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

TanyaAgarwal28
Copy link
Contributor

Problem:

Some years ago, a change was made that caused xpass not to be considered a failure. However, there have been multiple cases where it was missed that a test was fixed, and this is not an acceptable default behavior.

Solution:

This PR aims to address the issue by modifying the behavior of xpass in the following way: In this strict parameter of xfail is set to true by default and it will ensure xpass is either a warning or a failure in any case.

Checklist:

  • Include documentation for the proposed change.
  • Include new tests or update existing tests, where applicable.
  • Allow maintainers to push and squash when merging my commits.

This change closes issue #11467.

Changelog: Created a new changelog file named 11467.feature.rst and added the entry for this change.

Author: Tanya Agarwal

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.

thank you for getting started on this

unfortunately the need for backward compatibility makes it necessary to start with a warning and progress the change of the default on one of the next major releases

item: Item, call: CallInfo[None]
) -> Generator[None, TestReport, TestReport]:
rep = yield
@hookimpl(hookwrapper=True)
Copy link
Member

Choose a reason for hiding this comment

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

please restore the prior wrapper definition as thats the new style hook wrapper declaration from pluggy

@@ -208,7 +207,7 @@ def evaluate_xfail_marks(item: Item) -> Optional[Xfail]:
"""Evaluate xfail marks on item, returning Xfail if triggered."""
for mark in item.iter_markers(name="xfail"):
run = mark.kwargs.get("run", True)
strict = mark.kwargs.get("strict", item.config.getini("xfail_strict"))
strict = mark.kwargs.get(item.config.getini("xfail_strict"), True)
Copy link
Member

Choose a reason for hiding this comment

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

in order to ensure backward compatibility we cannot go straight from non-strict to strict
instead we have to start by warning if strict was not set to true or false

the warning should indicate that a future major release of pytest will change the default from False to True
and recommend to use struct=True as default and a plugin for actually flaky tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

@TanyaAgarwal28
Copy link
Contributor Author

Sorry I'm closing this PR due to misunderstaning of a solution. I have created an another PR.
Make xpass failure again warning #11467

@TanyaAgarwal28
Copy link
Contributor Author

Closes

@TanyaAgarwal28
Copy link
Contributor Author

Please find below the latest open PR for this same issue with the changes suggested by you as well.
#11499
@RonnyPfannschmidt

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