Poor error reporting in cram tests

[aalekseyev]

In cram tests, there are cases where dune blows up with a confusing error where it could have reported a better error.

Example 1 (sh syntax error):

Define a function with a dash in its name

  $ foo-bar () {
  >   true
  > }

There is no error message at all, even though sh reports an error correctly.

Dune:

          sh (internal) (exit 2)             
(cd _build/.sandbox/a76f004532a7d5adefdceb308f650161/default/test/blackbox-tests/test-cases/watching/test-1.t && /usr/bin/sh /tmp/dune.cram.bf2305run.t/main.sh)

Sh:

$ sh -c 'foo-bar () { true; }'
sh: `foo-bar': not a valid identifier
EXIT STATUS 2

Example 2 (script terminates early):

  $ true
  $ exit 0

Observed error:

Internal error, please report upstream including the contents of _build/log.
Description:
  ("command without metadata", {})
Raised at Stdune__Code_error.raise in file "src/stdune/code_error.ml", line
  9, characters 30-62
Called from Dune_rules__Cram_exec.run in file "src/dune_rules/cram_exec.ml",
  line 411, characters 12-48
... (many more useless stack frames) ...

Expected error: replacing all sections that weren't reached with a text explaining that this section was not reached.

Specifications

• Version of dune: b9f0ad8 (2.7.1+)

[rgrinberg]

For the first example, I don't know how to fix the first properly in sh because I'm not sure how to recover from source errors with something other than trap. I can at least improve the error messages a little.

Perhaps using sh was a bad idea after all? Maybe we should just use bash and be done with it? Wdyt @jeremiedimino

Is the error message better with bash?

It seems to me that using bash will make cram tests less reproducible.

[rgrinberg]

It depends. IMO, relying on the posix compatibility of your shells to maintain reproducibility isn't going to work anyway. A more reliable is just to fix which version of the shell you expect on every machine.

[aalekseyev]

I don't know how to fix the first properly in sh

I don't know who is swallowing stderr from sh, but one great way to recover would be to stop swallowing it and instead show it to the user.
I don't necessarily expect a correction proposed here.

[rgrinberg]

I forgot to answer this question:

Is the error message better with bash?

Yes, bash allows us to recover from the error and proceed to the next user command.

I don't know who is swallowing stderr from sh, but one great way to recover would be to stop swallowing it and instead show it to the user.

The current error message doesn't include the location command in cram fail caused the failure. That's the main problem. I suppose we can inject the location ourselves?

[aalekseyev]

I'd say both the stderr and the location are at least equally important, and if I had to choose I'd choose the stderr, but anything is better than the message in the issue.

It depends. IMO, relying on the posix compatibility of your shells to maintain reproducibility isn't going to work anyway. A more reliable is just to fix which version of the shell you expect on every machine.

How would that work? I expect that people will have whatever bash version their OS installed, and these are going to vary between users. So fixing the version of the shell is going to make the test not work for most people.