Skip to content

Issue 916/ordered spies bug

Created by: johnceh

# Need some discussion (Info Below)

Why?

Fix for Issue #916 - Spies were not tracking order properly and raised error incorrectly for alternating groups. It was including all messages/expectations/invocations when evaluating order on each expectation.

What Changed?

Added limiting range for messages/expectations/invocations for order checks so just current expectations from group are evaluated. Tests by @myronmarston in issue added in.

# Discussions Needed 1) By limiting range to running expectation, the error message now focuses on what it's expecting vs what happened. Changing this will cause failures to occur on previously successful tests. Before, the error would say that one is out of order, but it will change to say two is out of order since the range limits what messages/expectations it checks for. Previous code left commented in for now for visibility.
allow(the_dbl).to receive(:one)
allow(the_dbl).to receive(:two)

the_dbl.one
the_dbl.two

expect(the_dbl).to have_received(:two).ordered
expect(the_dbl).to have_received(:one).ordered
          
# Raised error before - received :one out of order
# Raised error after - received :two out of order

Before I dedicate any more time to this, I thought I'd check if I'm even on the right track with this fix. Also I didn't want to make assumptions on if we want to make this change or not, so if it needs to be backwards compatible, then I will keep looking into it after some feedback.

  1. Had to swap checks in expected_messages_received_in_order for expected messages and order so order is checked first. This was needed so that if an unexpected message comes in first, it doesn't error on the fact that 0 expected messages were received, but instead that the expectation was for a different message. It's a similar problem as above with breaking existing tests since an order error will be displayed now vs a count error. The travis ci failure is due to this change, and it's use case makes sense for count I think. Anyway, I thought I'd push it up for now and see what everybody thought before I made to many more changes. Previous code left commented in for now for visibility.

  2. This did bring to light a potential issue for receive messages as well (which had the passing test in the issue). I added a pending test that is failing and was hoping someone can verify what the expected result should be and if a similar change will be needed where order is checked before message expectations.

I found some other inconsistent responses with varied scenarios, so just ignore my comments for now and I'll try to get this to respond more like the receive.

Merge request reports