rollbear / trompeloeil

Header only C++14 mocking framework
Boost Software License 1.0
802 stars 85 forks source link

Slightly confusing error message when using .TIMES() in combination with .IN_SEQUENCE() #296

Closed siggisv closed 1 year ago

siggisv commented 1 year ago

When running a test similar to the following one (the actual test I was writing is a lot longer, therefore hiding the actual problem better), I got an error message that correctly complained about a call to m.func(2) being made while the sequence was still expecting a different one.

But what confused me was that the error message stated that m.func(0) was the expected next step in the sequence while not mentioning the real problem: m.func(1) being expected but not called.

That looked to me like .TIMES(0, 1) was being ignored on the first expectation and sent me on a wild chase trying to figure out why that was not working in that test. Until a couple of days later I finally thought: "Oh, wait! Did I forget to set the second call to being optional also?"

class Mock
{
public:
  MAKE_MOCK1(func, void(int));
};

void test_me(Mock m, bool something)
{
  if (something) {
    m.func(0);
    m.func(1);
  }
  m.func(2);
}

TEST(a_test)
{
  bool something = false;
  Mock m;
  trompeloeil::sequence seq;

  REQUIRE_CALL(m, func(0))
    .IN_SEQUENCE(seq)
    .TIMES(0, 1); // Optionally call this once.

  REQUIRE_CALL(m, func(1))
    // Oups! Forgot to set this one to also be optional.
    .IN_SEQUENCE(seq);

  REQUIRE_CALL(m, func(2))
    .IN_SEQUENCE(seq);

  test_me(m, something);
}

I understand that not only could it be difficult to generate an error message that list all the allowed next steps when .TIMES() is used in combination with .IN_SEQUENCE(), but also other people might complain about it being to verbose.

But it might save other people from also getting the wrong impression if at least this could be mentioned in the documentation as a possible gotcha.

rollbear commented 1 year ago

.TIMES combined with .IN_SEQUENCE is confusing at the best of times. I'll see what I can do to improve the message. Thanks for reporting.

rollbear commented 1 year ago

This doesn't seem to be that difficult, surprisingly. I'm working a bit with how to format the message. What do you think about this, for your example?

Sequence mismatch for sequence "seq" with matching call of m.func(2) at tst.cpp:32.
Sequence "seq" has m.func(0) at tst.cpp:24 first in line
and has m.func(1) at tst.cpp:28 as first required expectation
rollbear commented 1 year ago

If you could try out branch sequence_messaging, it would be much appreciated. I believe it solves your issue.

siggisv commented 1 year ago

Thank you. This is looking good.

I should have the time to try it out sometime in the next couple of days.

siggisv commented 1 year ago

If you could try out branch sequence_messaging, it would be much appreciated. I believe it solves your issue.

Have tried that branch and can confirm that it solves my issue.

Thank you again.

rollbear commented 1 year ago

Thanks for checking. I'm leaving this open until a release has been tagged. Probably won't be very long.