spock icon indicating copy to clipboard operation
spock copied to clipboard

Mock/Spy interaction verification withing specific time frame (with timeout)

Open szpak opened this issue 2 years ago • 2 comments

Is your feature request related to a problem?

Testing asynchronous code (schedules, Kafka/RabbitMQ), an expected interaction might be triggered with some delay (e.g. 60ms). The regular verification construction fails immediately and an artificial wait before it is required (or some other ~~hacks~~ workarounds), what reduces the readability.

Describe the solution you'd like

An ability to define optional timeout to wait for an interaction before failing the assertion. E.g.

then:
   1.withTimeout(50) * orderFacadeMock.confirmOrder(ORDER_NUMBER)
   (1.._).withTimeout(Duration.ofSeconds(1)) * orderFacadeMock.getOrderStatus(_)

Describe alternatives you've considered

As a workaround, I've been usually using some (thread safe) boolean or counter in the stubbing, to verify the number of interactions. However, it makes code less readable and suppress the "Unmatched invocations" reporting.

Recently @Vampire proposed a nicer variant of that approach with CountDownLatch, which eliminates the second limitation, but still requires extra elements and obscures the code.

given:
  ...
  OrderFacade orderFacadeMock = Mock()
  def latch = new CountDownLatch(EXPECTED_NUMBER_OF_ORDER_CONFIRMATIONS)
when:
  kafkaTemplate(ORDER_CONFIRMED_TOPIC_NAME, ORDER_NUMBER)
then:
  latch.await(1000, SECONDS) || true
then:
  //orderConfirmed() should be called when Kafka consumer received a message from a designed topic
  EXPECTED_NUMBER_OF_ORDER_CONFIRMATIONS * orderFacadeMock.orderConfirmed(ORDER_NUMBER) >> { latch.countDown() }

Additional context

  1. I didn't check if the DSL modification (to add withTimeout() to a number of range) can be applied only within the scope of the mock assertion (not to every number "everywhere").
  2. I don't know, if there isn't any obstacles in the internal implementation of the interaction verification (e.g. that it is moved to the end of the when block.
  3. In Mockito it would be verify(orderFacadeMock, timeout(200)).orderConfirmed(ORDER_NUMBER).

szpak avatar May 04 '23 12:05 szpak

Spock assertions are not tested sequentially, unless you use separate then blocks. How would you see multiple timeouts interacting with each other?

  1. In the same then block
    1. Act independent from each other with the timeout for all starting when the then is being reached.
    2. The timeouts in a then block get added together.
  2. Across then blocks
    1. Like 1.i. all starting at the first then
    2. Like 1.i. but for each then, i.e., the second timeout starts on the reaching the second then
  3. How would 4.withTimeout(300) work?
    1. The timeout would apply 4 times
    2. The timeout would be to for all 4 calls
  4. For ranges (1..3).withTimeout(300) how long should it wait?
    1. Until the first hit
    2. or until either the maximum is hit or the timeout expires and check if it reached the minimum.
  5. How would _.withTimeout(300) behave?
  6. Something else

In the meantime I'd change the alternative to move the countDown() to the when block.

given:
  ...
  OrderFacade orderFacadeMock = Mock()
  def latch = new CountDownLatch(EXPECTED_NUMBER_OF_ORDER_CONFIRMATIONS)
when:
  kafkaTemplate(ORDER_CONFIRMED_TOPIC_NAME, ORDER_NUMBER)
  latch.await(1000, SECONDS)
then:
  //orderConfirmed() should be called when Kafka consumer received a message from a designed topic
  EXPECTED_NUMBER_OF_ORDER_CONFIRMATIONS * orderFacadeMock.orderConfirmed(ORDER_NUMBER) >> { latch.countDown() }

A downside of the timeout would be that it would require polling, the CountDownLatch would still be the fastest one.

leonard84 avatar May 05 '23 13:05 leonard84

  1. In the same then block i. Act independent from each other with the timeout for all starting when the then is being reached. ii. The timeouts in a then block get added together

i. It's easier to implement and for that case people could write some custom mechanism with threads and latches. Having 2 verifications, they should be performed one by one staring the counter for each of them. In general, in the majority of cases, there is just one verification with timeout (to wait for the condition to occur) and then immediate sequential assertions of the state.

  1. Across then blocks i. Like 1.i. all starting at the first then ii. Like 1.i. but for each then, i.e., the second timeout starts on the reaching the second then

ii. Which effectively makes it similar to i from 1 for the subsequent then: then: blocks, but can have more sense for the following sequence:

when: "send message 1 to Kafka"
then: "wait until consumer receives message 1"
when: "send message 2 to Kafka"
then: ""wait until consumer receives message 2 and check some extra related stuff"
  1. How would 4.withTimeout(300) work? i. The timeout would apply 4 times ii. The timeout would be to for all 4 calls

ii. 4 calls have to be performed withing a given timeout. If only 3 (or 5), the error about "too few interactions withing timeout" should be presented. For 4 interactions the wait should be finished with success (even though there could be more messages received in the future).

  1. For ranges (1..3).withTimeout(300) how long should it wait? i. Until the first hit ii. or until either the maximum is hit or the timeout expires and check if it reached the minimum.

That's a good question. I would probably go for ii to wait until the timeout is reached (unless the number of interactions is larger than the right bound of the interval - it probably doesn't make sense to wait until the end). Of course for 1.._ it doesn't make sense to wait if the current number of interactions is 1 or 2, but it would complicate the implementation, so I would just keep to the rule described in the previous sentence. Then (1..1).withTimeout(300) could be an overused syntax for when: sleep(300); then: 1*... (but probably it shouldn't be advised in docs).

  1. How would _.withTimeout(300) behave?

I don't see a sensible case for that construction. It should finish with success on the first check as "matched".

  1. Something else

Probably. We could write tests for know cases and describe it in docs. Having the feature marked as @Beta we could tweak the behavior for some time, if any inconsistency (nonsensicalness?) is found. Most likely there could be some other already detected at the design level corner cases.

Of course the implementation (and testing) effort might be not worth doing that (there is a workaround ↑↑↑). I've just reported an idea to discuss it and also to see (by :+1: ) if anyone else is interested in that feature (like that guy on SO years ago).

In the meantime I'd change the alternative to move the countDown() to the when block.

Thanks, it is slightly better.

A downside of the timeout would be that it would require polling, the CountDownLatch would still be the fastest one.

Sure, but at the cost of decreased readability.

szpak avatar May 18 '23 21:05 szpak