Skip to content

Conversation

shlompy
Copy link

@shlompy shlompy commented Feb 25, 2024

Fixes #261

@icemac
Copy link
Contributor

icemac commented Feb 26, 2024

@shlompy Thank you for your PR. Currently many tests break. Could you please have a look what's causing this?

@shlompy
Copy link
Author

shlompy commented Feb 26, 2024

Thanks @icemac
I hope I fixed the precommit issues.
As for test_execution_count_exposed test failures, I changed the assert. It looks like the test assertion was wrong due to the bug.

I hope I have not misunderstood.
This test should fail in teardown until teardown will succeed on the third run - Meaning it should have had 2 reruns and 1 pass.
I'm not sure why the test asserted for 3 passes and 2 reruns in first place. Probably it took into consideration the bug, because the teardown failed twice and passed one time, but when it failed twice, it was too late because the 'call' (test) report was already concluded as pass.

Now with my bug fix the outcome is indeed 1 pass and 2 reruns, so I've updated the test assertion to match that.

But I don't see the test git actions being triggered again after my last push

@icemac icemac requested a review from hugovk February 28, 2024 07:43
@hugovk
Copy link
Member

hugovk commented Feb 28, 2024

Thanks for the review request, but I'll stick to general CI/maintenance things for this library and leave the internals to others 👍

@hugovk hugovk removed their request for review February 28, 2024 09:22
@shlompy
Copy link
Author

shlompy commented Mar 1, 2024

resolved conflicts

@shlompy
Copy link
Author

shlompy commented Mar 3, 2024

gentle reminder, can this fix please be reviewed?

Copy link
Contributor

@icemac icemac left a comment

Choose a reason for hiding this comment

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

It is not clear why the test breaks, it looks like a smell.

I'd like to see a test like you had in the issue failing without the changes and succeeding with them.

)
result = testdir.runpytest("--reruns", "2")
assert_outcomes(result, passed=3, rerun=2)
assert_outcomes(result, passed=1, rerun=2)
Copy link
Contributor

Choose a reason for hiding this comment

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

The test name says that execution_count is exposed to the test runner. So it should be 3 passed tests, as this is set during tear down.

I do not know why this behaviour is here and why it breaks now.

@icemac icemac added the do not merge Not (yet) ready to be merged. label Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do not merge Not (yet) ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

only_rerun with exception raised from fixture teardown reruns test but report previous runs as failure

4 participants