Best-effort automatic reporting of the location of `Alcotest.check`

[MisterDA]

This is an attempt of automatically discover the location of calls to Alcotest.check, in order to not force the user to pass ~here or ~pos arguments. It uses Printexc.get_callstack. It's an early attempt that is fragile and requires OCaml 4.11. The test need to end with the unit value in order to prevent the compiler from optimizing the last call to a tail-call, erasing the location in the process. Is there a way to change Alcotest' code so that this optimization doesn't happen? A more advanced attempt should probably walk the call stack to find recognizable enclosing functions calls, add regression testing, and make it work (or default to no-op) on pre-4.11. I don't know whether the code is efficient enough.

Try it out with

dune exec --display=quiet -- examples/bad/bad.exe -e

cc @TheLortex

[MisterDA]

Reworked the code a bit thanks to @TheLortex review. It now supports pre-4.11, is a bit more robust, and includes tests. This only works with Alcotest.check and Alcotest.check', not with Alcotest.fail and related functions, because they're always in tail-call position since they raise, and adding a statement after these is unsound… There's no way to force the compiler not to optimize the tail-call of Alcotest.check, so I've added a note explaining how Alcotest may "guess" the location.

[TheLortex]

Thanks for this PR. It's a free improvement of the location reporting feature of alcotest.

The drawback is that it's not simple to predict when it works (backtrace enabled and call to Alcotest.check is not a tailcall). The addition of a return unit instruction after the check to eliminate the tailcall might be somewhat brittle (maybe it will be optimized away at some point) and it's hard to discover its reason without having the context (meaning the unsuspecting user will want to remove it)

- Alcotest.check ... 
+ Alcotest.check ... ;
+ ()

I wonder what other alcotest users think about it.

[MisterDA]

I've changed the code to use the latest Printexc API, introduced in 4.12. Alcotest will fallback to a no-op before that. I've also added an environment variable to disable position guessing: for instance Alcotest uses expect tests on its output, and the tests would fail on earlier OCaml versions. I don't expect they'll be many users of it apart from Alcotest itself. I've added regression tests. Maybe they won't really work and strip_randomness should be modified to remove that extra information instead?

[samoht]

Thanks!

Could you show an example of what this changes in the output?

Also, for the new files, could you keep the same license as the rest of the project?

[MisterDA]

This just shows the position of the call to Alcotest.check in the test source file, if available. The position is almost identical to using Alcotest.check ~here:[%here] from ppx here or Alcotest.check ~pos:Stdlib.__POSITION__ but the user doesn't have to specify the location themselves. It only works if the call to Alcotest.check is a tailcall (usually the last one is not), and backtraces have been enabled (usually the case); and with OCaml >= 4.12.

An output with the location reported:

ASSERT strings
File "test/e2e/alcotest/source_code_position/test_source_code_position.ml", line 37, character 2:
FAIL strings

   Expected: `"A"'
   Received: `"B"'

one with disabled:

ASSERT strings
FAIL strings

   Expected: `"A"'
   Received: `"B"'

Also, for the new files, could you keep the same license as the rest of the project?

Done.

[samoht]

That looks like a great feature!

ASSERT strings
--
507 | -File "test/e2e/alcotest/source_code_position/test_source_code_position.ml", line 37, character 2:
508 | +File "test/e2e/alcotest/source_code_position/test_source_code_position.ml", line 24, character 2:
509 | FAIL strings

However, it doesn't seem very stable across OCaml versions 😁 https://ci.ocamllabs.io/github/mirage/alcotest/commit/7fad5b2e3cb68e8c3d2f29128850f16b166bbd4b/variant/alpine-3.15-4.14_opam-2.1#L507-507

Do we care? Should we just disable it for older OCaml versions?

[MisterDA]

However, it doesn't seem very stable across OCaml versions grin https://ci.ocamllabs.io/github/mirage/alcotest/commit/7fad5b2e3cb68e8c3d2f29128850f16b166bbd4b/variant/alpine-3.15-4.14_opam-2.1#L507-507

Do we care? Should we just disable it for older OCaml versions?

It's already the case, I just forgot to promote the tests when I added the copyright notice…

[samoht]

Looks great, thanks for the patch!