Documenting the (negative) behaviour of a package (and `mdx` FTW)

Ecosystem
,
1

Recently I decided to do some … cleanup to the error-messages for a number of packages I develop, and in the process I started writing mdx tests that elicit each error-message. And in the process, I realized that for some of the errors, I didn’t actually know how (or even -whether-) they were triggered. So I started systematically figuring out how to elicit each error-message. Since these are mostly PPX rewriters, that meant from a command-line (compiler invocation) and the top-level.

As I kept working thru this, I thought: “this is complementary to unit-tests: they document positive behaviour, and this documents negative behaviour”. I wondered if others had had this thought.

In any case, BRAVO MDX! It really makes this tractable!

2

At my former employer I wrote a bunch of “negative compilation tests” for a library with an especially complex interface. This was a C++ library using template metaprogramming, and intelligible error messages were absolutely essential for good UX.

I had never considered doing the same for OCaml using MDX, but that seems like a reasonable use case. Do I remember correctly that MDX supports regex-based output assertions? I would be inclined to distill error messages down to their essence, to make the assertions robust against minor wording and formatting changes in later versions.

3

I do not recall that MDX supports that, and a quick perusal of the GitHub homepage confirms – but I could be wrong (and would be happy to be so!) You bring up an important point – about how to make the “assertions” robust against minor wording/formatting changes. I do two things:

  • run a Perl script against the “.corrected” file to remove all lines that are likely to change – e.g. version-numbers, directories, etc
  • wrap long text in error-messages in “#<| … |>” so that I can again use that Perl script to rip those out.

This tends to leave only the most relevant bits of the error-messages.

4

I’ve done a simpler version of this task using Dune’s built-in expect test feature. My test executables write all of the errors to stdout, and Dune checks them against their .expected files. For a project that doesn’t use many dependencies then it’s highly simple and effective. (I don’t need PPX or regex support.) The only laborious part is methodically finding every possible error state.

(Example here and here.)

5

I’ve used expect-tests in the past. What I like about MDX is:

  • when using bash, you’re also documenting how to actually use the package from a command-line. That’s useful, esp. since I’m a Make (not dune) bigot.
  • when using the ocaml toplevel, it documents how to use the package from the toplevel. And (haha) also for PPX rewriters. I’m a big aficionado of using the toplevel for debugging, so this is (to me) another plus.

But I really should use more expect-tests: I think the difference is that MDX is better for … “larger grain” stuff, than expect-tests. But really, de gustibus and all that. Also, I really did this with the intent of devising a test that elicited every nontrivial “failwith” in the code. That way, I could know what the path is to each such error.

Now, what I -haven’t- done, is lots of unit-tests for the internals of some of my more complex PPX rewriters. That seems … tricky: I should think about how to do that.

6

whilst mdx is great for local tests and even for your ci, i’d recommend against adding the tests to your opam releases. same goes for some but not all cram tests.

the reason is that mdx tests are brittle. they have caused many errors in the opam-repository ci (and the same errors would appear on user machines who would use the testing feature of opam).

  • errors because the output depends on timing or other environment (pipe the output through sed to remove all timing, path, etc. information)
  • errors because the output of mdx changed (they started escaping some characters, they changed the line-wrapping logic, etc.)
  • errors because some formatting rule changed (line-wrapping, filename quoting, float printing, etc.)

from what i can tell reviewing opam-repository prs, it is difficult to write mdx that’s robust to all these.

btw there are several ways to do this: you can not include the test files in the tarball, you can have them triggerred by a different alias, you can detect your ci with a env var you control, etc. i think that one i’d recommend is to have a different alias and to call both from the opam file (with {with-test}): that way it’s possible to remove the offending one by only altering the opam file rather than redoing a release.

see Please stop running `dune runtest` in the CI · Issue #29166 · ocaml/opam-repository · GitHub for more discussion on this topic

7

Having repeatedly asked for people to stop snapshot testing cmdliner outputs, I think the ways you mention are too complicated for end-users to care.

I think the dune tests shoud have some kind of integration-test boolean property and only those tests that are labelled as such should be run when dune runtest is run in the opam-repository CI and all tests are run otherwise.

A few more details need to be flushed out on how exactly and which is the right default, but I think this should be the user experience.

8

i think that’s a good idea.

i’d prefer having a system of tags on the test/tests stanzas. the main issue is that dune has a builtin notion of tests but no way to attach separate aliases to separate subsets. i’d be happy defining the ci to run dune build @runtest.ci and have the opam file mention "@runtest.release" {with-test}.

maybe that’s too complex though.

9

Tags are fine they are just boolean properties in disguise. From a specification point of view though, they are usually positive, you list them. So it’s a bit more cumbersome if you want to have a default that you disable.

I’m not familiar with the alias system of dune but you can also simply have a -tTAG[,TAG]* option to select them. Then the opam test build instruction become something like:

dune build @runtest -t public-ci
10

I think that @JohnJ meant was actually Dune’s support for Cram tests. Which is pretty similar to MDX. Dune does also support expect tests, though that is provided by an external library, ppx_expect.

We use the built-in cram tests in Dune development itself extensively, support for regex can be done via grep or through specific helper executables, in our case dune_cmd which implements portable versions of some Unix commands.

Basically all bug reports get reproducing tests in the form of cram tests, where then the fix changes the expected output and the fixed test serves as a regression test to make sure the bug is not reintroduced later on by accident.

11

I use Dune’s cram tests a lot too, but here I did actually mean Dune’s built-in expect tests, which only require Dune with no external library. Dune confusingly does not give this feature a real name, although it describes its behavior under its “custom tests” documentation. All you need to do is have your test executable write to stdout and then give it a matching .expected file, and Dune manages it for you. (See also the example files linked in my previous post.)

12

My response below the pull-quote is … kinda in the weeds, so I thought I’d preface with a high-level thought,

I (think I) get your thought here, and it seems like there are two issues: (a) opam-repo brittleness and the labor cost incurred by opam-repo maintainers (and less importantly by package maintainers/developers), and (b) system costs to -running- those tests in opam-ci.

You didn’t mention #b, so I’ll set it aside, but I’ve wondered if that’s an issue at all.

So to take #a, I’ve felt for a number of years, that the use of extensive testing at at several OCaml versions, and several versions of deps/revdeps,
makes the opam package-collection feel more like a “monorepo” developed by a single organization with a single design and testing philosophy, than what it is, which is a collection of packages developed by people who rarely meet, might differ dramatically in many opinions, and generally are just not well-connected to each other except thru the repo.

I’ve always felt like it is a major strength of the opam-ci process, that (what might appear to be) anal-retentive and draconian (but isn’t!) testing has achieved this level of robustness.

To that end, I felt like adding CRAM tests was a good thing, b/c it tests a different dimension of behaviour of packages – and I’ve seen myself that those tests elicit errors with different versions of OCaml, and different versions of supporting packages, so it doesn’t seem to be superfluous. That said, for sure just doing CRAM/mdx testing blindly, without carefully -cleaning- the output of those tests before comparing, is an antipattern. For sure.

If it’s a consensus view that one should not run mdx tests in opam-ci, I can -easily- disable that. I’ve got separate Make targets for mdx and regular tests.

whilst mdx is great for local tests and even for your ci, i’d recommend against adding the tests to your opam releases. same goes for some but not all cram tests.

the reason is that mdx tests are brittle. they have caused many errors in the opam-repository ci (and the same errors would appear on user machines who would use the testing feature of opam).

  • errors because the output depends on timing or other environment (pipe the output through sed to remove all timing, path, etc. information)
  • errors because the output of mdx changed (they started escaping some characters, they changed the line-wrapping logic, etc.)
  • errors because some formatting rule changed (line-wrapping, filename quoting, float printing, etc.)

from what i can tell reviewing opam-repository prs, it is difficult to write mdx that’s robust to all these.

This is really interesting. On the one hand, it’s an easy thing to modify the Makefiles in the packages I develop, to not run mdx tests when invoked via opam. On the other hand … and in this I’m going to put a big, big caveat here, that I’m not a maintainer, so I don’t see the pain mdx causes you all. I could be very, very wrong about what I’m about to write.

First, you’re absolutely right that mdx output can be and is mercurial. Some examples (of which you’re very well aware, but I list them to show that I’ve dealt with them):

  • output of the OCaml typechecker can change from version-to-version.
  • filenames, hostnames, timestamps
  • irrelevant error-messages that might or might not show up on other platforms

My attempt to address these (and other) problems is to use MDX by:

  • run MDX against an .asciidoc (or .md) file
  • take the .corrected output file and run a script against it that cleans up many of the above list
  • for particular packages, run further cleaners. This produces a .corrected.CLEANED file
  • only then compare that file against the original.

I use Make, so this is embodied in rules :

  • first a standard script (“clean-ocaml-mdx-output”) to clean:
$s =~ s,.*OCaml version.*,,g ;
$s =~ s,.*#help.*,,g ;

$s =~ s,.*Command line:.*\n,,g ;
$s =~ s,.*: added to search path.*\n,,g ;
$s =~ s,.*: loaded.*\n,,g ;
$s =~ s,.*Cannot read directory.*stublibs.*\n,,g ;
$s =~ s,.*Interface.*occurs in several directories.*\n,,g ;

$s =~ s,#<\|(?:[^\|]|\|[^>])*\|>\n?,,gs ;

That script:

(a) nukes version-numbers, various noise, commandlines output, paths, and superfluous error-messages
(b) sometimes error-messages produce -significant- diagnostic output in addition to the text we would want to match against for the test. The last line removes text of the form #<|....|> and the idea is, when I output large texts in error-messages, I try to wrapper it with these eyecatchers, so I can nuke that output before comparing.
(c) NOTE: one thing I haven’t dealt with, is stack tracebacks. I might need to figure out how to nuke them systematically, but they don’t seem to show up (yet).

But (as you know well) the above script isn’t enough. For example, in some OCaml versions the typechecker unfolds a type-abbreviation, and that means the output in the toplevel can vary. So in one case, I added:

pa_ppx-error-handling.asciidoc.corrected.CLEANED: pa_ppx-error-handling.asciidoc.corrected
$(TOP)/tools/clean-ocaml-mdx-output $< > $@.NEW && mv $@.NEW $@
perl -p -i -e 's|(.*migrate_z1.*)int \* bool,(.*)|$${1}z1,$${2}|' $@

This was to deal with a case where a type-signature of a member of a record contained int * (int * bool) where it needed to be int * z1 (z1 was of course an abbreviation for int * bool). This behaviour change showed up between OCaml versions (5.3, 5.4, IIRC, but I might be getting the version-numbers wrong).

13

[I’m writing this in a separate reply, b/c don’t want it to get conflated with the more-general issue if opam-repo brittleness due to “over testing”]

Your thoughts on this make me wonder if it would be useful to write a tool that allowed to -clean- MDX output in programmatic ways? That is to say, a souped-up version of the Perl scripts I described in my other note, but

(a) of course written in OCaml

(b) driven by hints/annotations in the mdx file itself

I’m sure you can tell that I’m a big fan of mdx, and I’d like to make it as robust as possible for testing purposes. So if you had ideas on how to do that, I’d be happy to try to implement some of them.

14

For what it’s worth, I have had this thought, and I put it in practice when writing dolmen , which is a parser and typechecker for some languages used in automated deduction.

For instance, this folder : dolmen/tests/parsing/smtlib/v2.7/errors at master · Gbury/dolmen · GitHub contains examples files that should cover all of the errors that the parser for a given language and version (in this case the SMT-LIB language, version 2.7) can produce, to ensure that some examples do fail when the input is outside the spec of a language, but also to check that the produced error message is reasonable.

To make writing these tests easier, I actually use a script to generate the dune files (see dolmen/tools/gentests.ml at master · Gbury/dolmen · GitHub ), so that adding a test is as easy as adding the input file.