search

eranpeer / FakeIt

C++ mocking made easy. A simple yet very expressive, headers only library for c++ mocking.
MIT License
1.24k stars 173 forks source link

FeatureRequest: Improve the verification output for invocation sequences #150

Open alibabashack opened 6 years ago

alibabashack commented 6 years ago

While FakeIt works like a charm for me (Thanks, for this powerful framework!), I noticed some minor annoyance when testing longer invocation sequences as the one below:

tests/TrxPathIntegration.cpp:115: FAILED:
  Verify( listener.trxGroupStateChanged(01, 00) ... trxArray[0].startDispatch( Any arguments ) ... watchdogTimer.setTimeoutRelative(12345) ... trxArray[0].cancelDispatch( Any arguments ) ... trxArray[1].cancelDispatch( Any arguments ) ... watchdogTimer.setTimeoutRelative(735234) ... watchdogTimer.disable( Any arguments ) ... listener.trxGroupStateChanged(00, 01) )
with message:
  tests/TrxPathIntegration.cpp:115: Verification error
  Expected pattern: listener.trxGroupStateChanged(01, 00) ... trxArray[0].
  startDispatch( Any arguments ) ... watchdogTimer.setTimeoutRelative(12345) ..
  . trxArray[0].cancelDispatch( Any arguments ) ... trxArray[1].cancelDispatch(
  Any arguments ) ... watchdogTimer.setTimeoutRelative(735234) ...
  watchdogTimer.disable( Any arguments ) ... listener.trxGroupStateChanged(00,
  01)
  Expected matches: exactly 1
  Actual matches  : 0
  Actual sequence : total of 7 actual invocations:
    listener.trxGroupStateChanged(01, 00)
    trxArray[0].startDispatch(?)
    watchdogTimer.setTimeoutRelative(12345)
    trxArray[0].cancelDispatch()
    trxArray[1].cancelDispatch()
    ...
  1. Observation: The "Expected pattern" list is unreadable, especially with wider terminals, because line breaks are missing.
  2. Observation: The "Expected pattern" list is actually redundant, because it is a direct duplication of the source code. In my (Catch2-based) output it is even printed twice. It is not helping (me) one single bit to understand the failure. (This might be a result of my narrow view of only testing according to TDD.)
  3. Observation: It is hard to find the first unexpected invocation in the actual sequence list, because it is not highlighted in any way and a manual comparison of the expected and actual sequences is required.
  4. Observation: The actual sequence list is limited to a certain size (see #149). While this might be necessary as discussed in #149, it is far from optimal for the user. If the list is so long that it is truncated and the unexpected invocation occurs after the limit, the reason for the failed test cannot be examined. If the list is not truncated but still very long (50 lines are the limit), then Observation 3 kicks in.

Consequently, I have the following suggestion:

  1. Do not output the expected pattern list.
  2. Highlight the first unexpected invocation within the actual pattern list. This could be for example a simple -> arrow or +/- prefixes as seen in a diff output.
  3. Do not simply truncate the actual pattern list, but display a reasonably sized window around the first unexpected invocation. In this way the output is limited and the user can still reason about the failed test.

An example of the suggested output format might look like this:

tests/TrxPathIntegration.cpp:115: FAILED:
  Verify( ... method call sequence ...)
with message:
  tests/TrxPathIntegration.cpp:115: Verification error
  Expected pattern matches: exactly 1
  Actual pattern matches  : 0
  Sequence diff : in total of 71934 actual invocations:
    (skipping 734 matching method calls)
      aCorrectMethodCall(1234)
      anotherCorrectMethodCall(1234)
    - theFirstUnexpectedMethodCall()
    + anExpectedButMissingMethodCall()
      aCorrectMethodCall(1234)
      aCorrectMethodCall(34)
    + aSecondMissingMethodCall()
      aCorrectMethodCall(0)
    (skipping 71194 other method calls)

Any thoughts or comments?