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

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

Open
szpak opened this issue May 4, 2023 · 2 comments
Open
Labels
new feature request-for-comments Indicates that a issue needs input from stakeholders

Comments

@szpak
Copy link
Member

szpak commented May 4, 2023

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 szpak added new feature request-for-comments Indicates that a issue needs input from stakeholders labels May 4, 2023
@leonard84
Copy link
Member

leonard84 commented May 5, 2023

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.

@szpak
Copy link
Member Author

szpak commented May 18, 2023

  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 👍 ) 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature request-for-comments Indicates that a issue needs input from stakeholders
Projects
None yet
Development

No branches or pull requests

2 participants