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.

Sign up for GitHub

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

Jump to bottom

static functions' expectations don't get cleared after a panic. #442

Closed
DGolubets opened this issue Dec 21, 2022 · 1 comment · Fixed by #443
Closed

static functions' expectations don't get cleared after a panic. #442

DGolubets opened this issue Dec 21, 2022 · 1 comment · Fixed by #443
Labels
bug Something isn't working

Comments

@DGolubets
Copy link

I have an issue using mocks for static methods.
I was able to reproduce it with a slight modification of the tests (https://github.com/asomers/mockall/blob/master/mockall/examples/synchronization.rs):

  1. Replace returning with return_once.
  2. Change the expectation in the first test to fail it.
  3. Now the second test starts to fail with MockThing::one: Expectation(<anything>) called twice, but it returns by move
@asomers
Copy link
Owner

asomers commented Dec 22, 2022

This happens because Mockall tries to avoid double-panicing. If a thread is already panicing, then we don't attempt to verify that all expectations have been satisfied. However, that has a side effect of not clearing expectations after a panic. You never would've noticed this behavior if you'd been using the standard Mutex, because subsequent tests would've gotten a PoisonError. But apparently you're using the unpoisonable Mutex from the example. I'll fix it.

@asomers asomers changed the title Context synchronization static functions' expectations don't get cleared after a panic. Dec 22, 2022
@asomers asomers added the bug Something isn't working label Dec 22, 2022
asomers added a commit that referenced this issue Dec 23, 2022
@asomers
Always drop static methods' Expectations during Drop
8479511 
Even if a panic is in-progress.  Originally we didn't drop them during
panic, for fear of double-panicing.  But there is already protection
from double-panic in Common::drop.  Not clearing expectations during a
panic can cause surprising failures in one test case after a different
test case panics.

Fixes #442
asomers added a commit that referenced this issue Dec 23, 2022
@asomers
Always drop static methods' Expectations during Drop
f8559f7 
Even if a panic is in-progress.  Originally we didn't drop them during
panic, for fear of double-panicing.  But there is already protection
from double-panic in Common::drop.  Not clearing expectations during a
panic can cause surprising failures in one test case after a different
test case panics.

Fixes #442
@asomers asomers mentioned this issue Dec 23, 2022
Merged
asomers added a commit that referenced this issue Mar 26, 2023
@asomers
Always drop static methods' Expectations during Drop
779e316 
Even if a panic is in-progress.  Originally we didn't drop them during
panic, for fear of double-panicing.  But there is already protection
from double-panic in Common::drop.  Not clearing expectations during a
panic can cause surprising failures in one test case after a different
test case panics.

Fixes #442
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Always drop static methods' Expectations during Drop asomers/mockall
2 participants
@asomers @DGolubets