Equality with pretty printer

[sir4ur0n]

Hello,

We are considering moving to QCheck from Crowbar on a project I contribute to and so far it looks good, thank you for this library!

There are however some things that are either possible in Crowbar (and we couldn't find an equivalent in QCheck), or that we find is lacking. Since we are new to QCheck we don't know yet if we missed the features or if they don't exist (yet? :smile: ), so I propose to open a few issues for discussion, and maybe propose changes.

Custom equality function + pretty printer

A feature of both Alcotest and Crowbar is the custom equality check function, which takes a custom printer. This is pretty useful in test failures to see the 2 values.

• Crowbar check_eq
• Alcotest check (which uses the pretty printer configurable in the TESTABLE module declaration)

Note that this need is different from QCheck's print function in arbitrary because it only prints values generated in case of test failure.

If my PBT generates a foo, then does some stuff to build 2 different values of bar, I often want to test those 2 values are equal, and if they are not, print both values so that I see the difference (note: currently it prints the value of foo, which is useful; but it's not enough).

What do you think about adding a function similar to Alcotest/Crowbar?

[c-cube]

That's an interesting idea. I've mostly used QCheck.Test.fail_reportf in the past (example in containers) but a dedicated function would make a lot of sense. Building on top of fail_reportf internally would be a good start since it already allows to write a custom message.

I'm not sure if the check_eq function can reuse the generator's printer as you might compare types that are not the same as those generated, so you'll have to provide an equality function and a printer in general. Optional arguments are helpful.

We are considering moving to QCheck from Crowbar on a project I contribute to and so far it looks good, thank you for this library!

That's nice to hear, thank you! I see crowbar as more complementary to QCheck than a competitor, btw: crowbar is slower but easier to use and probably more thorough; qcheck gives you a lot of control and can typically be run quickly on every build.

[sir4ur0n]

Well, you are an excellent candidate for fastest answer to an issue :open_mouth: Thank you for the reply!

I'm not sure if the check_eq function can reuse the generator's printer as you might compare types that are not the same as those generated, so you'll have to provide an equality function and a printer in general.

I fully agree: there may or may not be an overlap between generator printers and the value being compared, so I can't think of a solution to reuse those.

I kind of like how Alcotest takes a TESTABLE but taking optional labeled arguments sounds good too (and more in line with the rest of QCheck design, so it would feel more natural for developers).

[c-cube]

:grin: yes, optional arguments would be perfect. Would you be game for implementing that?

[sir4ur0n]

I can't make any promise, but if we commit on using QCheck, this may happen indeed (sorry for the lack of visibility :sweat_smile: )

[sir4ur0n]

Here's a first draft (that seems to work in my tests). Note that I took some inspiration from Crowbar.

(** [check_eq pp cmp eq a b] evaluates whether [a] and [b] are equal, and if they
    are not, raises a failure and prints an error message.
    Equality is evaluated as follows:
    {ol
    {- use a provided [eq]}
    {- if no [eq] is provided, use a provided [cmp]}
    {- if neither [eq] nor [cmp] is provided, use Stdlib.compare}}
    If [pp] is provided, use this to print [a] and [b] if they are not equal. *)
let check_eq ?(pp : (Format.formatter -> 'a -> unit) option)
    ?(cmp : ('a -> 'a -> int) option) ?(eq : ('a -> 'a -> bool) option)
    (a : 'a) (b : 'a) : bool =
  let pass =
    match (eq, cmp) with
    | (Some eq, _) ->
        eq a b
    | (None, Some cmp) ->
        cmp a b = 0
    | (None, None) ->
        Stdlib.compare a b = 0
  in
  if pass then true
  else
    match pp with
    | None ->
        QCheck.Test.fail_reportf
          "@[<h 0>Values are not equal, but no pretty printer was provided.@]"
    | Some pp ->
        QCheck.Test.fail_reportf
          "@[<v 2>Equality check failed:@,%a@,is different from@,%a@]"
          pp
          a
          pp
          b

Here's what an error message looks like in case of inequality:

test `My awesome test` failed on ≥ 1 cases:
0L (after 61 shrink steps)
Equality check failed:
  1
  is different from
  0

You may notice in particular that this takes a Format pretty printer rather than the 'a -> string type that is currently widespread in QCheck. I will open another issue to propose supporting both (or even better: only support Format style?). Here, this is pretty (pun intended :grin: ) useful to have indentation (the pretty printer may have sub-indentation too, and Format would take care of it).

What's your thought?

[c-cube]

Please open a PR, that looks good!

I'm not fan of the vertical alignment but who cares :-). The API seems good.

[sir4ur0n]

About the vertical alignment: I put it to better visually identify where the value printing ends and the rest of the message starts. Ideally I would maybe put a line separator instead? I think the QCheck runner uses those.

Feel free to propose (or even impose :smile: ) whatever visual hint (or absence thereof) you feel is better!

I will try to open an MR in the coming hours/days (heavy workload right now) :wink:

[c-cube]

no worries, this stuff can be changed later. Bikeshedding is a suboptimal use of time.

[sir4ur0n]

A feedback that popped during a code review on my project that now uses the function above: Since this function returns a bool (that is always true if it doesn't fail and raise an error) to comply with the result of a test, putting assertions before the end of a test feels weird:

QCheck.Test.make arb (fun x -> 
  let _ = check_eq ~pp x (f x) in (* feels weird *)
  let x2 = do_something_else in
  check_eq ~pp x x2
)

Some ideas to improve the UX that come to my mind:

• change the return type of a test (and of check_eq) from bool to unit to better express that errors are handled through side effects
  • it means check_eq now becomes the privileged way of asserting in QCheck
  • there is a minor risk of writing tests that do 0 assertions, something that is much less likely with a non-unit return type

The code above would become:

QCheck.Test.make arb (fun x -> 
  check_eq ~pp x (f x);
  let x2 = do_something_else in
  check_eq ~pp x x2
)

• fully embrace a monadic return type (a la QuickCheck/Hedgehog) instead of relying on exceptions to short circuit.
  • slightly more verbose: because of strictness we are forced to carry those annoying fun () ->
  • adds complexity for users (more stuff to understand to write non-trivial tests)

The code above would become:

QCheck.Test.make arb (fun x ->
  check_eq ~pp x (f x)
  >>= fun () -> let x2 = do_something_else in
  check_eq ~pp x x2
)

I would be in favor of solution 1