-
Notifications
You must be signed in to change notification settings - Fork 84
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
Ensure all test reports are processed regardless of prior report status #124
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR. I would prefer to have a testcase in this PR, which actually shows the effect of this change. What is the effect of calling pytest_runtest_logreport
now in every case. From my perspective, in almost every case, except parallel and xdist, this hook was called.
or not report.failed | ||
or xfail | ||
or is_terminal_error | ||
or need_to_rerun |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this way of writing we have 5 possible cases in this branch, which will never be properly addressed by any branch coverage. I would prefer a way, so that we need a testcase to make the coverage happy. And I would prefer a testcase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hook was not called for reports of a test run beyond the first failure, which this PR fixes. So, before this PR if a test fails in phase setup
, its call
and teardown
reports aren't processed ( in this case all we do for processing is call pytest_runtest_logreport
Now that I look at my change again I have a general question. Why do we not call pytest_runtest_logreport
for failed test phases when xdist or parellel?
What is the current status of this PR? |
This PR has conflicts which need to be resolved first. |
Not sure how active @gnikonorov is these days. According to GitHub is last activity was in Jan 2022. I'm happy to pick this up, given the maintainers are willing to have the discussion. |
@BeyondEvil Thank you for volunteering for this PR. I am here as a maintainer to help and discuss the issue as needed. |
Ensure that we always process all reports for a test in
pytest_runtest_protocol
regardless of whether a past report for a test failed. We now defer any pre-rerun setup until after all reports are executed while still keeping the same rerun logic.Fixes #108