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 panic in AssertExpectations for mocks without expectations #1207

Merged
merged 1 commit into from Jun 20, 2022

Conversation

tabboud
Copy link
Contributor

@tabboud tabboud commented Jun 20, 2022

Summary

Regression was introduced in #1182 (released in v1.7.3) which updated the mock mutex to be a pointer and updated most call sites that check this mutex to initialize if it's not yet, however the mock.AssertExpectationsForObjects and thus the mockObject.AssertExpectations did not take this into account which results in a panic. It's common to initialize mocks at the start of a test and then defer asserting expectations for the end of the test regardless of if a mock had expectations set on it or not. This happens in cases where the test mock does not set any On() methods or does not call mockedObject.Test(t) which would initialize the mutex if it's not set. Calling mockedObject.Test(t) is a fine workaround, but regardless this is a regression.

I added a test case that repro's the issue without the change. I'm happy to update this to be it's own test, but for now, I just left a comment about why it's there.

Changes

Updates the mock.AssertExpectationsForObjects to initialize the mutex if it's not created yet.

Motivation

Regression introduced in #1182 which causes a panic for previously working test code

Related issues

Closes #1206

Copy link
Collaborator

@boyan-soubachov boyan-soubachov left a comment

Choose a reason for hiding this comment

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

Thank you! I must've missed that one.

@boyan-soubachov boyan-soubachov merged commit 48391ba into stretchr:master Jun 20, 2022
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.

Release 1.7.3 call AssertExpectations without On cause panic
2 participants