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

stopTest wrapper calling addSuccess unconditionally #150

Open
romainkomorndatadog opened this issue Dec 15, 2023 · 1 comment
Open

stopTest wrapper calling addSuccess unconditionally #150

romainkomorndatadog opened this issue Dec 15, 2023 · 1 comment
Labels

Comments

@romainkomorndatadog
Copy link

This is more a question than an issue, really, but I'm trying to understand why the patched version of stopTest always calls addSuccess, regardless of the state of the actual test run.

In our (Datadog's CI Visibility integration of unittest) case, this makes us incorrectly report a test as passing even if it fails, since we consider any call of addSuccess in the test's run as an indication the test succeeded, and since flexmock wraps the original test run.

For us, I see reporting the last result update (whether it's addSuccess or any other) as more appropriate, since it'll properly deal with cases where someone decides to re-run a test until it succeeds.

We can come up with workarounds (generalized or more specific to flexmock) (eg: maybe ignore results if stopTest is in the call stack, since that shouldn't be providing results according to the original unittest case class), so I'm genuinely more curious about why addSuccess is called unconditionally.

@ollipa
Copy link
Member

ollipa commented Dec 16, 2023

It seems that that change was added 12 years ago in this commit: 670609e

I'm not actually sure why addSuccess is called at all because the default implementation doesn't do anything (unittest.TestResult.addSuccess) and addSuccess should have been called already before stopTest is called. My guess is that it was added for nose integration, but nose support has been dropped so we could just remove the addSuccess call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants