Skip to content
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

fix: Allow subtests to xfail #40

Merged
merged 2 commits into from
May 29, 2021

Conversation

maybe-sybr
Copy link
Contributor

@maybe-sybr maybe-sybr commented May 20, 2021

This change ensures that all makereport hooks get called prior to
subtests transforming the constructed TestReport to a SubTestReport.
A test is added for pytest style usage and an xfailing test is added for
unittest style since the same issue affecting skip counting is present
for xfail counting.

Fixes #24

@maybe-sybr
Copy link
Contributor Author

I don't really know why this doesn't work. FWICT there is some stuff in https://github.com/pytest-dev/pytest/blob/c198a7a67ead8f4b1933d74c2c7a7f7ede93d284/src/_pytest/skipping.py#L269-L272 which is supposed to handle translation of the XFailed exception type in excinfo to an actual xfail in the report. Perhaps because subtests makes its own report using TestReport.from_item_and_call(), the other hooks aren't getting a chance to run?

@maybe-sybr maybe-sybr force-pushed the fix/subtest-xfail branch from 70fbec1 to b66b5c7 Compare May 20, 2021 02:29
@maybe-sybr maybe-sybr changed the title test: Add simple failing subtest xfail test fix: Allow subtests to xfail May 20, 2021
@maybe-sybr maybe-sybr force-pushed the fix/subtest-xfail branch from b66b5c7 to f06298d Compare May 20, 2021 02:30
@maybe-sybr
Copy link
Contributor Author

Okay, I think I worked out a nice approach for fixing this issue :) Let me know what you think and if this is mergable or needs more work.

@maybe-sybr maybe-sybr marked this pull request as ready for review May 20, 2021 02:31
@nicoddemus
Copy link
Member

Thanks @maybe-sybr! I will review it ASAP, thanks a lot for the contribution, appreciate it. 👍

@maybe-sybr
Copy link
Contributor Author

I'll fix the lint issue now :)

This change ensures that all `makereport` hooks get called prior to
subtests transforming the constructed `TestReport` to a `SubTestReport`.
A test is added for pytest style usage and an xfailing test is added for
unittest style since the same issue affecting skip counting is present
for xfail counting.

Fixes pytest-dev#24
@maybe-sybr maybe-sybr force-pushed the fix/subtest-xfail branch from f06298d to b03438d Compare May 26, 2021 23:15
@@ -71,14 +71,19 @@ def _from_json(cls, reportdict):
)
return report

@classmethod
def _from_test_report(cls, test_report):
Copy link
Member

Choose a reason for hiding this comment

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

FYI made this private users from using it (at least for now).


def _addSubTest(self, test_case, test, exc_info):
if exc_info is not None:
msg = test._message if isinstance(test._message, str) else None
call_info = make_call_info(
ExceptionInfo(exc_info), start=0, stop=0, duration=0, when="call"
)
sub_report = SubTestReport.from_item_and_call(item=self, call=call_info)
report = self.ihook.pytest_runtest_makereport(item=self, call=call_info)
Copy link
Member

Choose a reason for hiding this comment

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

Clever solution! 😁

@nicoddemus nicoddemus merged commit ec6f8a5 into pytest-dev:master May 29, 2021
@maybe-sybr
Copy link
Contributor Author

Thanks for the merge @nicoddemus :)

@maybe-sybr maybe-sybr deleted the fix/subtest-xfail branch June 1, 2021 00:14
@nicoddemus
Copy link
Member

Sure, and thanks for the contribution! 👍

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.

pytest.xfail reported as hard failure
2 participants