minor usability issues with cram tests

[gasche]

I just wrote my first cram test and encountered some minor usability issues, reported below. (Let me know if I should split them in three separate issues.)

Git error

The first is very minor: running the test (before it has been promoted to include proper output) shows a rather scary

         git (internal) (exit 1)

near the top of the output. (git and (exit 1) are in red.) This is confusing as it looks like cram tests interact with my git workflow somehow, when I don't expect them to. There is a grayed-out line right below

(cd _build/.sandbox/91100af1cb755fb629459b1f7df1f2d7/default && /usr/bin/git diff --no-index --color=always -u ../../../default/bin/test_errors/json.t/run.t bin/test_errors/json.t/run.t.corrected)

from which I can actually understand that you are using git diff as your diff utility, so the error is not related to an interaction with my git repository. Still, the first message, emphasized as an error, is unnecessarily confusing, and it would be better to not show it. Maybe a proper error message like: "Test output mismatch, see foo.corrected" could be used instead.

Hard-to-enable cram tests

To enable cram tests (to check error display in a binary), I created a new test_errors directory with the following stanza:

(cram
 (deps %{bin:mustache}))

Then I wrote my first cram test (without putting the correct output, so I know it would fail), and I ran dune runtest, and then nothing happened:

Done: 0/0 (jobs: 0)

I assumed that there was something wrong with my cram syntax, so I tried various variations (single-file test instead of directory test, two-spacing of commands or not, etc.); each time Dune would do absolutely nothing.

Then I realized that I had forgotten to include (cram enable) in my dune-project file.

This is a usability bug. I'm not sure if Dune should show a warning if .t files occur without cram tests enabled (this sounds like a usability disaster for a project using .t extensions for something else), but it should definitely say something if I use a (cram ..) stanza in a dune file, without cram tests enabled.

Not-so-clear documentation for cram directory tests

When you create a directory-style cram test, should run.t commands be 2-indented like in normal cram tests, or should they start at the beginning of the line? (Sounds like a silly question, but it's the sort of question you ask yourself when your test is not doing anything and you don't know why.)

According to the current documentation, commands in run.t in directory tests should not be 2-indented:

We may then
access their contents in the test script ``run.t``:

.. code:: bash

   $ wc -l foo | awk '{ print $1 }'
   4
   $ wc -l $(ls bar) | awk '{ print $1 }'
   1231

We may then access their content in the test script run.t

$ wc -l foo | awk '{ print $1 }'
4
$ wc -l $(ls bar) | awk '{ print $1 }'
1231

According to my testing, this is incorrect and 2-indentation should be used. (Which is more consistent with single-file cram tests, and more convenient to write comments.)

Specifications

• Version of dune: 2.7.1

[ghost]

I got bitten by the second point recently. Fix in #4245.

BTW, cram tests are not enabled by default precisely in case a project already using dune uses .t for something else. However, we could consider enabling them by default for (lang dune 3.x) (/cc @rgrinberg). And if it turns out that some project do use .t for something else and can't move to version 3.x of the language because of it, then we could add an option to allow users to choose a different extension.

[rgrinberg]

Yeah, that seems good to me.

…

On Feb 17, 2021, 2:10 AM -0800, Jérémie Dimino ***@***.***>, wrote: I got bitten by the second point recently. Fix in #4245. BTW, cram tests are not enabled by default precisely in case a project already using dune uses .t for something else. However, we could consider enabling them by default for (lang dune 3.x) (/cc @rgrinberg). And if it turns out that some project do use .t for something else and can't move to version 3.x of the language because of it, then we could add an option to allow users to choose a different extension. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[gasche]

The "Not-so-clear documentation" issue is still present today, the documentation example is wrong as it is not 2-indented.

[Alizter]

The first issue will be fixed by #6994. When we do internal git calls, we will no longer display them to the user.

[Alizter]

Regarding your three issues:

1. Is fixed by feature: Silence display in non-user processes #6994.
2. Was fixed by Print an error when seeing a cram stanza and cram tests are not enabled #4245 and since Dune 3.0, Cram tests are enabled by default.
3. Is fixed by doc: fix cram test in doc #6995.

Sorry these took a while to address.

[gasche]

Thanks! Sounds good.

For the second issue, I think that there may be a systemic issue worth fixing: the general issue is that we write some specific stanzas in our dune fine, relying on opt-in features (here cram tests), but those specific stanzas are ignored without a warning if the opt-in feature is not enabled. Intuively I would always expect to see some sort of warning if some parts of my dune files are ignored completely with the current configuration -- or some variant of that idea which actually works. The fix here are nice, but only for cram tests, there may be similar issues lurking for other opt-in feature.

[Alizter]

The cram stanza is a bit of a special case here, since it had it's own option for enabling it in a project file and has some ad-hoc interaction with the parser (i.e. it was completely ignored). This is no longer the case since cram tests are understood by default, and AFAIK cannot be disabled.

Comparing with every other stanza that gets added for various features, typically a user is required to write (using coq 0.7) in their project file to enable even parsing of the stanzas. If those stanzas are encountered when the feature is not enabled, they get a nice error message suggesting that they enable it.

[gasche]

Thanks for the explanation! Sounds good.

[rgrinberg]

. This is no longer the case since cram tests are understood by default, and AFAIK cannot be disabled.

It's possible to write (cram disabled) in the project file.

[Alizter]

@rgrinberg OK then we should check we have a nice error message for the user when this is the case.

[Alizter]

For completeness, here is the error message when you disable and try to use cram tests: #7007