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

AssertExpectations should not fail if the test was skipped #1329

Closed
ianrose14 opened this issue Jan 26, 2023 · 1 comment · Fixed by #1331
Closed

AssertExpectations should not fail if the test was skipped #1329

ianrose14 opened this issue Jan 26, 2023 · 1 comment · Fixed by #1331
Labels
bug pkg-mock Any issues related to Mock

Comments

@ianrose14
Copy link
Contributor

ianrose14 commented Jan 26, 2023

If you set up a test with something like defer myval.AssertExpectations(t) and then later skip the test (t.Skip()), the AssertExpectations check can still fail your test. I propose that AssertExpectations should check whether the test has already been marked as "skipped" and, if so, return nil (success). Here's the change I'm envisioning:

type tSkipped interface {
	Skipped() bool
}

// AssertExpectations asserts that everything specified with On and Return was
// in fact called as expected.  Calls may have occurred in any order.
func (m *Mock) AssertExpectations(t TestingT) bool {
	if s, ok := t.(tSkipped); ok {     // <--- START OF PROPOSED NEW CODE
		if s.Skipped() {
			return true
		}
	}                                  // <--- END OF PROPOSED NEW CODE
	if h, ok := t.(tHelper); ok {
		h.Helper()
	}
	m.mutex.Lock()
	defer m.mutex.Unlock()
	[...]
}

If y'all are in agreement with this, I'm very happy to create a PR.

@ianrose14
Copy link
Contributor Author

I went ahead and made a PR here: #1331

@dolmen dolmen added bug pkg-mock Any issues related to Mock labels Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug pkg-mock Any issues related to Mock
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants