Open Groxx opened 6 years ago
Ah, excellent news: https://github.com/stretchr/testify/issues/608
There is a way to inject testing.T into the mock, which should absolutely be done... but it's not yet released :|
Bah.
Once that's released, this should be done within Cadence (if possible). If not, at least it's exposed to callers since NewTestWorkflowEnvironment
returns a *Mock
- loads of docs plz! This is a fairly dangerous potential pitfall.
Will look into this issue once that mock testing.T support is released in testify.
Testify had a release! 1.2.2 has this feature: https://github.com/stretchr/testify/blob/v1.2.2/mock/mock.go#L220-L239
I can probably make a pull request to upgrade and use it internally. Since this is exposed to callers via Mock
and not Cadence
, there shouldn't be any need to make a breaking change / require users to upgrade Testify. Users on older Testify versions just won't have the method.
(it unfortunately doesn't output logs when tests panic, like in #562, but that's mostly a separate issue)
Hi @Groxx! Sorry to dig up a very old issue, but we are facing a similar issue with unit tests passing despite underlying activity panics when there are missing expectations. Did you happen to come up with a clean workaround to enforce failures when there are panics back when you were looking at this?
In a nutshell: this test passes, despite the second call panicking.
Obviously I'm suppressing the error that's returned from the second
.Get
, but that should be irrelevant. In fact, the test passes whether I do that or not.Everything except
env.AssertExpectations(t)
passing is fine. In this case the mock expectations do not match the calls, but because it panicked it didn't record the fact that it was called, so insufficiently-mocked behavior does not fail tests like it should.So far as I can tell, the issue is this code in testify.mock:
Since the panic occurs before the call count is incremented, it "forgets" that it failed.
So... granted this is basically a bug in testify (and I intend to bring it up with them), but it's much more relevant to Cadence code than most code since panics are suppressed and wrapped in even simple tests. Is there another mock framework that could be tried instead, or something that could be done to work around this?