search

ocaml-flambda / flambda-backend

The Flambda backend project for OCaml
104 stars 76 forks source link

ocamltest improvements + make sure all tests actually do something #2755

Closed mshinwell closed 3 months ago

mshinwell commented 3 months ago

The main change is to add a "does this action do something?" flag to all actions defined by ocamltest for the use of test scripts. Then it is enforced that all branches of a test must do something. This should avoid what's happening at the moment, which is that a chunk of tests aren't running at all, because of stanzas like:

(* TEST
   runtime5;
*)

In the absence of any other explicit directives like bytecode; or native; this actually removes the default bytecode/native compile+run actions, and just leaves a predicate test and nothing else. The test therefore does nothing.

It is terrible that we only discovered this because of a locals bug (https://github.com/ocaml-flambda/flambda-backend/pull/2736). There, a new test case could not be made to fail even with a syntax error, which was because in fact the test directives did not describe anything substantive to do.

The "does this action do something?" logic isn't perfect; for example if you are going to compile then run, then arguably the compilation step should not "do something". However this is already much better than what we have, and I think will catch the fundamental errors.

Two other improvements are included, which can be split into separate PRs if necessary:

  1. Introduce an explicit notion of "predicate", so that diagnostics for these ones can be improved. For example we now get things like the following (this is an example when configured for runtime5): 
    ... testing 'test_instrumented.ml' with 1 (runtime4)
=> predicate "4.x runtime being used" is not satisfied
  2. Stop printing dozens of n/a lines where test directives are skipped because of a previous predicate which was false.

Finally there are fixes to various tests to make them actually run. Two of the runtime_events ones fail (one because of some build failure where stubs.c is apparently not being linked, and another because of something semantic in the test itself) but I've disabled these for now, since we're not using that library. The fixes for statmemprof tests are from @stedolan with thanks.

mshinwell commented 3 months ago

One other change here that I forgot about: this builds ocamltest as part of make hacking, which makes it much easier to work on (e.g. merlin works). It's very fast to build so I don't think this should be problematic.