mockery icon indicating copy to clipboard operation
mockery copied to clipboard

Improve failure output when using pinned variables

Open sascha-wolf opened this issue 5 years ago • 4 comments

As the title suggests, the output is currently not very helpful when using pinned variables.

See an example below:

assert_called Redix, :pipeline, [
  :redix,
  [
    ["HSET", ^access_token_namespace, ^id, ^access_token],
    ["EXPIRE", ^access_token_namespace, ^id, _],
    ["HSET", ^refresh_token_namespace, ^id, ^refresh_token],
    ["EXPIRE", ^refresh_token_namespace, ^id, _]
  ]
]

If then the test fails the output looks like this (output shortened by using <placeholders>):

Redix.pipeline was not called with given arguments

Given:
[:redix, [["HSET", ^access_token_namespace, ^id, ^access_token], ["EXPIRE", ^access_token_namespace, ^id, _], ["HSET", ^refresh_token_namespace, ^id, ^refresh_token], ["EXPIRE", ^refresh_token_namespace, ^id, _]]]

History:
[:redix, [["HSET", "<namespace>", "<id>", "<access token>"], ["EXPIRE", "<namespace>", "<id>", <expire>], ["HSET", "<namespace>", "<id>", "<refresh token>"], ["EXPIRE", "<namespace>", "<id>", <expire>]]]

code: assert_called Redix, :pipeline, [
stacktrace:
  ...

This makes it very, very hard to find out why a particular test fails.

Compare this to how ex_unit handles pinned variables:

match (=) failed
The following variables were pinned:
  access_token = "<access token>"
  access_token_namespace = "<namespace>"
  refresh_token = "<refresh token>"
  refresh_token_namespace = "<namespace>"
  id = <id>
code:  assert [:redix, [["HSET", ^access_token_namespace, ^id, ^access_token], ["EXPIRE", ^access_token_namespace, ^id, _], ["HSET", ^refresh_token_namespace, ^id, ^refresh_token], ["EXPIRE", ^refresh_token_namespace, ^id, _]]] = commands
right: [
         ["HSET", "<namespace>",
          "<id>",
          "<access token>"],
         ["EXPIRE", "<namespace>",
          "<id>", <expiry>],
         ["HSET", "<namespace>",
          "<id>",
          "<refresh_token>"],
         ["EXPIRE", "<namespace>",
          "<id>", <expiry>]
       ]
stacktrace:
  ...

sascha-wolf avatar Jul 04 '19 13:07 sascha-wolf

Thank you for a detailed example, I always used assert_called with short patterns so it wasn't an issue. I'll try to improve output in the next few days.

amatalai avatar Jul 04 '19 13:07 amatalai

If you need any assistance, I'm happy to help. 🙂

sascha-wolf avatar Jul 04 '19 13:07 sascha-wolf

Any progress on this issue? It's one of the pain points of using mockery. I have time to contribute these next few days (hacktoberfest yay 🎉 ), so if you could give me some directions, I'll open a PR.

rhnonose avatar Sep 26 '19 19:09 rhnonose

Hello @rhnonose Unfortunately, there's no significant progress on this. I didn't have much time for open source recently.

Output for assertion failure is generated in Mockery.History module. It's probably the ugliest and most hackish part of the whole library.

Few challenges makes this nontrivial:

  • in contrast to ex_unit formatting (single match with pinned vars), Mockery.History must support multiple matches, because expected function can be called with invalid arguments several times (screenshot in readme shows this)
  • current implementation checks only if function call arguments on the same position are equal and colorize them as a whole, e.g. following calls:
    foo(1, {:a, :b}, "x")
    foo(1, {:a, :c}, "x")
    
    differs on the second argument (first arg will be green, second red and third green), but Mockery.History won't do any deep analysis on how exactly {:a, :b} and {:a, :c} differs.
  • flawed implementation of Mockery.History is also the cause of another reported issue #31 (I tracked it down to this module, but I didn't have time to check details)

At this point, I would prefer to rewrite whole Mockery.History module, but I fear that the first moment when I will have enough free time to even consider doing this will be around Christmas

amatalai avatar Sep 26 '19 20:09 amatalai