Feature request: "wait until" operator for async testing

[itzg]

It's hard to always avoid including goroutines within a unit test, so when I do, it would be really nice to block the unit test until an expected call is made and then proceed with verifications. Otherwise, I'm having to throw in a time.Sleep(100*time.Millisecond) to ensure goroutines have had a chance to do their thing before doing verifications.

I'm thinking the call would follow the Verify style and look something like:

mockedService.WaitUntil(1*time.Second).SomeServiceCall()

where the argument to WaitUntil specifies the maximum amount of time to wait.

I would glad to help contribute the implementation of this if the feature seems viable.

[petergtz]

Hi @itzg, definitely sounds like a reasonable request. I think there's nothing wrong in including tests that involve goroutines.

Do you happen to use Ginkgo for your tests? If so, there is a workaround (but an ugly one). Here's what I did in the past:

https://github.com/cloudfoundry-incubator/bits-service/blob/817337c2337d2eec4c42f96b3d0500673c5cc845/resource_handler_test.go#L108-L114

And the definition for interceptPegomockFailures looks like this: https://github.com/cloudfoundry-incubator/bits-service/blob/817337c2337d2eec4c42f96b3d0500673c5cc845/resource_handler_test.go#L320-L329

As I said, it's not really nice, and I'd much rather have a proper solution for it. What do you think about using VerifyWasCalledWithTimeout(timeout time.Duration) instead of WaitUntil. I know it's a bit long, but it would be consistent with the other versions of verification. What do you think?

[itzg]

I don't yet use Ginkgo, but I see what you mean in the code you referenced. That's actually not too bad IMHO; however, would be even cooler within pegomock itself :)

Yes, I like the VerifyWasCalledWithTimeout naming since I'm actually thinking in those cases I do want to verify the call..but just block to allow for the call to happen asynchronously.

[petergtz]

@itzg Just thought about this again, and I think

VerifyWasCalledEventually(invocationCountMatcher Matcher, timeout time.Duration)

reads even better, especially because we probably want to include the invocationCountMatcher to stay flexible. With that we'd be able to do things like:

mymock.VerifyWasCalledEventually(Once(), 2*time.Seconds).SomeFunc()

Implementation-wise, I think we'd want to introduce a new timeout param in https://github.com/petergtz/pegomock/blob/278dd9bee02596f5347dcffe7736d1ff1794223c/dsl.go#L94-L98

When it is -1, we just ignore it, otherwise we'd use a loop to do the verification until the timeout expired. Of course this new parameter must be set in the generated files.

I can try to implement this over next few weeks, but you're more than welcome to give it a shot. I'd be happy to merge it as a PR. I'm happy to assist as well, if needed.

[petergtz]

Hi @itzg, could you please try latest master? You'll have to recreate your mocks. You should then be able to use the VerifyWasCalledEventually method as described in my comment above.

[itzg]

@petergtz I sure can. I was just earlier today in the code where I needed this functionality.

[itzg]

@petergtz the generated mock file ended up with a duplicate import of the time package:

import (
    context "context"
    pegomock "github.com/petergtz/pegomock"
    agents "github.com/racker/telemetry-envoy/agents"
    telemetry_edge "github.com/racker/telemetry-envoy/telemetry_edge"
    "reflect"
    syscall "syscall"
    "time"
    time "time"
)

given this input file https://github.com/racker/salus-telemetry-envoy/blob/master/agents/command_handler.go

[petergtz]

@itzg Ah, yes. Haven't thought of that one. Fixed in latest master. Could you please try again?

[itzg]

Yes, that fixed it! Thanks @petergtz . This improvement has eliminated time.Sleep's that could have been brittle down the road.