[RFC] A nestable test API for Alcotest

[craigfe]

This is a proof-of-concept for a new API for test registration in which tests can be nested arbitrarily with one-or-more levels of hierarchy, as described in #275. Currently, this looks as follows:

val test :
  ?pos:source_code_position ->
  ?tags:tag_set ->
  string ->
  ('a -> unit m) ->
  'a test
(** [test ~name f] is a named test that runs the function [f]. If [f] makes
    a failing assertion (e.g. via [Alcotest.check]) or raises an exception,
    then the test case will fail. Otherwise, the test case passes. *)

val group :
  ?pos:source_code_position ->
  ?tags:tag_set ->
  string ->
  'a test list ->
  'a test
(** [group ~name tests] is a group of tests with the given name. *)

... where the type 'a test is abstract, unlike the current one. Using these combinators, it's possible to build test suites with arbitrary tree structures:

Another benefit of this more abstract API is that it's extensible to different classes of test, such as the snapshot testing supported by e.g. Jest. This PR is still quite scrappy but demonstrates the following high-level features that I'd like to provide:

Test metadata

(Related issues: #124, #293.)

Firstly, the tests (and test groups) can now have attached source positions via ~here, allowing Alcotest to point to specific failing tests. Secondly, it's now possible to attach user-defined metadata to tests via ~tags. These are arbitrary typed values, similar in design to Logs.Tag. The main purpose of tags would be to allow running user-defined test filters at runtime, using something similar to:

val run : ?filter:(tag_set -> [ `run | `skip ]) -> 'a test list -> unit promise

(In reality I'm also proposing we hide the various config parameters to run inside a Config abstraction to stop them polluting the API as they do now.) This would supersede the existing [ `Quick | `Slow ] distinction, and also enable ways to conditionally disable tests (e.g. Sys.os_type, Sys.word_size, environment variables).

Another possible use of this metadata (that I haven't explored yet) would be to provide editor integration that allows running a selected / highlighted test (or set of tests) directly from the editor. I suspect that this tags mechanism could also handle attaching locks to tests in order to support a concurrent test runner (related issue: #177).

PPX-friendly test registration

(Related issues: #126, #256.)

A nice property of having a flexible structure for test suites is that it's simple to implement a PPX that directly translates code structure into Alcotest registration boilerplate. I think it's important that Alcotest have a clean API without requiring a PPX, so this could be considered a follow-up feature, but as a proof of concept this PR contains the shell of such a PPX.

The gist is that the following code:

module%test Lorem = struct
  let%test "ipsum" = ()

  let%test "dolor" = ()
end

module%test Sit = struct
  module%test Amet = struct
    let%test "consectetur" = ()
  end

  let%test "adipiscing" = ()
end

[%%run_tests]

can be executed to give the following result:

This PPX can also support: (a) suites defined over multiple files, and (b) test suites that instantiate functors.

module%test Some_other_file = Some_other_file

module Test_maker () = struct
  let%test 1 = ()
  let%test 2 = ()
end

module%test Unix = Test_maker ()
module%test Mirage = Test_maker ()

We could provide some other PPX features for making test assertions with proper location information but this is unrelated. (It should also be possible to attach tags to such tests; I haven't looked into this yet.)

Backwards compatibility

As proposed, this PR is a substantial breaking change to the Alcotest API. For the reasons described above, I think it's worthwhile to make this change, but fortunately it's possible to ease the pain of transition quite a bit. The old API can be implemented entirely in terms of the old one, so it's possible to upgrade the core test runner and its API separately.

I propose a migration path like the following:

• upgrade the core of the test runner to support the V2 API, but keep the same top-level combinators for the V1 API;

• alias the V1 API under Alcotest.V1 (which can be supported in that form even after the V2 switch), and provide the new API under something like V2_alpha with an instability warning;

• provide a PPX for generating the registration boilerplate, which would use the V2 API inside its own namespace. (The hope being that users adopt the PPX and so make the V1 → V2 transition completely silent for them);

• release an Alcotest 2.0 with the default API upgraded and the old one still provided under Alcotest.V1. For users who don't want to upgrade their registration boilerplate and don't want to use the PPX, they could add module Alcotest = Alcotest.V1 in their headers to run on 2.0.

It's possible to imagine other migration paths, for instance taking the Ounit2 route and providing a new top-level module, or having the new combinators sit inside the same API as the old one. Having considered it a bit, I decided that a more opinionated line is probably warranted provided we can provide a sensible opt-out.

[emillon]

Hi.

Thanks for working on this issue. The restrictions on nesting are in my mind alcotest's biggest limitation. I often try to work around this by adding a sort of namespacing on the name of the tests. In addition, if a group is a test, that means that the argument of Alcotest.run can be a single test and I don't have to remember what to put in that list :)

To reiterate my comment in #275, I think it'd be very good to have some simple syntax for the common case (quick test, no explicit loc, no filter, just a unit argument) and OUnit's API is a good example of that. (that can be done as a third party library of course)

[craigfe]

Thanks for the feedback @emillon :-) For clarity, the current proposed type of Alcotest.test is:

val test :
     ?here:source_code_position
  -> ?tags:tag_set
  -> name:string
  -> ('a -> unit)
  -> 'a test

(... though it's currently a bit obfuscated in the draft implementation.) I'm reticent to go further and drop the string name, but it's technically possible. Is your concern that the optional arguments add too much noise, or are you particularly fond of the infix operator provided by OUnit?

[mjambon]

tl;dr: I suggest keeping the test list flat and have the alcotest library recognize dotted paths.

I'd rather stay away from anything that makes the API more complicated. In particular:

• Don't make the use of ppx a requirement. Don't make the API more impractical without ppx.
• Don't require a dependency on the alcotest library to define tests. Currently, it's possible to define a test suite with no dependency on alcotest [1]. Please keep it that way.
• Remain compatible with the current API, adding optional arguments if needed.
• Avoid operators that are not already standard. Nobody should have to remember >:::. Give evocative names, please.

I think we could improve the current situation by providing a simple packing function, keeping the test suite flat:

(* pack "Foo.Bar.regression tests" tests *)
val pack : string -> 'a test list -> 'a test list

This would be coupled with the convention that . is a path separator just like / is commonly used for file paths and URLs. We could have names like Foo.Bar.Baz which are interpreted by alcotest as the path ["Foo"; "Bar"; "Baz"]. Alcotest could then present the tests as a hierarchy and allow selections in that hierarchy e.g. "Foo.*".

Footnotes:

• [1] in this case one would just fail the tests with assert false, with happens to also provide a location.

[craigfe]

In short, I think:

• there are principled reasons for moving away from the rigid 2-level API beyond just allowing PPXes to generate structured suites;
• the nestable API is simpler to reason about and making the core types abstract opens the door to many improvements;
• it's possible to provide an easy migration path for users of the old API, and we should take advantage of this to move to a more maintainable and extensible state.

In long...

I'd rather stay away from anything that makes the API more complicated.

I don't agree that the proposed API is more complicated than the existing one, except in the sense that it contains more optional arguments. To take examples/simple.ml from the repository:

let () =
  Alcotest.run "Utils"
    [
      ( "string-case",
        [
          Alcotest.test_case "Lower case" `Quick test_lowercase;
          Alcotest.test_case "Capitalization" `Quick test_capitalize;
        ] );
      ( "string-concat",
        [ Alcotest.test_case "String mashing" `Quick test_str_concat ] );
      ( "list-concat",
        [ Alcotest.test_case "List mashing" `Slow test_list_concat ] );
    ]

becomes:

let () =
  Alcotest.run "Utils"
    [
      Alcotest.group "string-case"
        [
          Alcotest.test "Lower case" test_lowercase;
          Alcotest.test "Capitalization" test_capitalize;
        ];
      Alcotest.group "string-concat"
        [ Alcotest.test "String mashing" test_str_concat ];
      Alcotest.group "list-concat"
        [
          Alcotest.test "List mashing" test_list_concat ~tags:Alcotest.Tags.slow;
        ];
    ]

This looks basically equivalent to me, with the differences being (a) any suite layout is possible, not just the previously-existing one (which was contrived to meet the pre-imposed structure anyway), and (b) the [ `Quick | `Slow ] distinction is hidden from users that don't need it.

If anything, I think the proposed API is simpler than the previous one. The following simple suites are now valid:

open Alcotest
let () = run "suite1" [ test "1" f; test "2" g ]
let () = run "suite1" [ test "equal" f; group "codec" [ test "encode" g; test "decode" h ]

whereas before it would have been necessary to wrap the tests of depth 1 inside some singleton group.

At a higher level, my experience of watching beginners interact with the Alcotest API is that they get bogged down in the concrete type aliases and imposed structure, with a thought process like:

• OK, I need a string and a unit test list to pass to Alcotest.run... what's a test?
• An 'a test is a string * 'a test_case list pair... what's a test_case?
• An 'a test_case is a string * speed_level * ('a -> return) triple... speed_level?
• Ah, I don't need this but I can just write `Quick. Finally.

The concrete types also make for some rather noisy and confusing type errors along the way. On the other hand, the proposed API asks the user for a list of tests, and there's a documented function called test for building them – done. They can start adding tests straight away and think about structuring them once that actually becomes necessary.

Don't make the use of ppx a requirement. Don't make the API more impractical without ppx.

PPX will never be a requirement of the Alcotest API, which should always be designed for hand-written code. If the motivation for this RFC were just to provide a better set of functions for PPXes to call, it could just hide these in an Alcotest.Exported_for_ppx module and not touch the main API at all.

As said above, I think the simpler model of test suites is a win for non-PPX users too. I also think attaching source locations is worthwhile for even for hand-written code – we've been doing this for Alcotest.check for a while now – hence my suggestion of a ?pos argument for test and group:

let test_thing =
  Alcotest.test ~pos:__POS__ "thing" (fun () ->
    (* ... test assertions here ... *))

let () = Alcotest.run __FILE__ [ test_thing ]

Don't require a dependency on the alcotest library to define tests. Currently, it's possible to define a test suite with no dependency on alcotest [1]. Please keep it that way.

Expanding the full type of an Alcotest suite, we get:

type suite = (string * (string * [ `Quick | `Slow ] * (unit -> unit)) list) list

This doesn't strike me as a particularly "natural" type of test suites existing in the wild: if someone puts `Quick in their test definition, it's probably because they intend to pass it to Alcotest. Given this, I'm not sure what the advantage of not using Alcotest functions directly for building the test suite is. Perhaps it's handy to be able to pull tests from libraries that don't depend on alcotest, but at best this avoids a small data manipulation at the call-site of Alcotest.run to migrate the concrete test suite into one written in terms of Alcotest functions. At worst, it obfuscates what these values are actually doing: the concrete types aren't very descriptive by themselves.

Regardless, this PR already supports Alcotest.V1.run which takes the previous concrete suite type (and it can go a step further by exposing a suite migration function, if necessary).

Remain compatible with the current API, adding optional arguments if needed.

The fact that the current API makes the test type fully concrete makes it difficult to make meaningful use of optional arguments. For instance, it's not possible to add optional source location information to tests without hacking it in somehow, e.g. by trying to pack them into test names and parse them out again later. The situation is worse for any non-serialisable metadata, which would have to be wrapped around the unit -> unit function and recovered later with global references / exceptions. At some point this becomes infeasible to maintain.

As said above, the RFC proposes retaining support for the current API under Alcotest.V1, allowing users to mix-and-match uses of the old API and the new one in the same switch. The low-effort migration process would then be to add module Alcotest = Alcotest.V1 to the top of each file; I'm not sure we can do better than this without just committing to never breaking the existing API.

[mjambon]

Actually, I'd rather specify my tests in the following format, which I've done in several projects:

type suite = (string * (unit -> unit)) list

[The current type being

type suite = (string * (string * [ `Quick | `Slow ] * (unit -> unit)) list) list

]

It's easy to convert from this format to whichever format alcotest actually uses. So, I don't think there's a real issue. It would be nice if alcotest provided such converter from the simpler (string * (unit -> unit)) list to Alcotest.suite but this can be done later.

[paurkedal]

While I can see the flattened API would also serve as a good simplification, the abstract API has an advantage that it will be possible to support test fixtures. An example use case is when en application needs to load a database schema and maybe some test data before running the tests, and then remove it again after the tests have run. Another example would be a fixture which constructs a big randomized data structure on which a number of quicker incremental tests are performed.

Not to solve this here, but to see how it may fit into the abstract API, I imaging the fixture to be something like a function paired with a finalizer which can be applied to a test:

val fixture : setup: ('a -> 'b promise) -> ?teardown: ('b -> unit promise) -> string -> ('a, 'b) fixture
val with_fixture : ('a, 'b) fixture -> 'b test -> 'a test

The string argument is for reporting.

Another issue I ran into when trying to use Alcotest for Caqti, is that I have a command line option which essentially evaluates to a list of database URIs on which I want to run the whole test suite, or part of it if I also merge in tests which don't require a database connection. The abstract API would allow adding something like:

val group_by : ?pos: source_code_position -> ?tags: ('a -> tag_set) -> 'a Fmt.t -> 'a test -> 'a list test

This can also be solved by transforming the test suite before passing it to Alcotest if I give up the convenient predefined argument parser.