testsuite/tests/formatting: remove native-compiler location tests

[gasche]

The tests for -dlocations are painful to update for native compiler
backends. They were previously restricted to 64bit architectures only
( e577855 ), and disabled on AFL
( 829b00b ), but the fact that they
have to be updated for both clambda and flambda backends is annoying
in practice. This commit disables location-testing completely for the
native backend, and only checks locations in the bytecode compiler
intermediate representations, from -dparsetree to -dlambda.

Note: now the we have bytecode-only versions of the test, it should be
more portable. The test has been re-enabled for 32bit and AFL
configurations. It will still need tweaking in the future if people
perform configuration-dependent changes on the Lambda representation
(but hopefully those changes could be disabled by command-line options
to be added to the test configuration).

(no change entry needed)

[gasche]

I was the one who introduced those tests when I introduced the -dno-locations option. Other people are too polite to say it out loud, but: they are annoying. What I found particularly annoying is to have different clambda and flambda output; in practice this requires that we reconfigure the compiler once with clambda and the other with flambda whenever there is a change that requires promotion of those tests.

I looked at past changes to the tests, and determined that (just like clambda/flambda issues) they are all related to the native-compiler tests. This PR removes those native-compiler tests but keep the bytecode tests.

One may ask whether the tests shouldn't be removed entirely, or changed in some way so that they almost never fail (one idea for example would be to grep for the sourcefile name, and check that it does occur in the output with -dlocations and does not with -dno-locations). For now I stopped on "just bytecode testing" as a compromise. A few extra comments:

• I'm not fond of dropping the tests completely. Admittedly -dno-locations is a niche feature that should not annoy anyone, but this kind of niche features can also be easy to break without noticing. See "unknown location" in backtraces in ocaml script #9701 for example, where a similar user-convenience feature (backtraces in the toplevel) regressed silently in 4.10 without anyone noticing.
• On one hand, having the whole -dlambda output in the test is unproductive (it augments the failure surface in ways that are not necessary, see the grepping idea), but on the other, having examples of -dlambda outputs in the testsuite can be nice. For example if someone implements a PR to change -dlambda formatting, they have to update the testsuite, but PR reviewers will see diffs of what the change affects in practice. I would compare this to type-checking tests that include a lot of error messages, even though they are not strictly-speaking testing error messages; having them in the output (instead of just success/failure information) is in fact quite interesting when we do a type-system change that affects error messages, we will catch it early thanks to this a-priori unnecessary test surface.

For these reasons I propose a reactive approach to test output shrinking. I started with tests for each part of my change (each intermediate-output touched by -dno-locations), removed some of them who were too fragile before submitting the PR. Now some other tests are proving annoying, so I'm getting rid of them selectively. They may disappear completely over time, but I would rather wait until the remaining parts of the test have proved actively cumbersome.

[xavierleroy]

I agree we have too many fragile tests in the test suite, where the reference output needs to be updated all the time.

Another example is tests that record a full exception backtrace in the reference output. These tests break every time line numbers change in the source of the tests, e.g. because the magic ocamltest comment at the top is changed. This is annoying indeed.

[xavierleroy]

Restricting this test to bytecode only should make it easier to maintain, indeed.

[gasche]

Thanks!

For exception backtraces and ocamltest headers: this may be solvable with a good old lexer line-number directive. I tried them for the present test and they don't work here, because locations as currently printed contain absolute positions that are not reset by line directives.

[shindere]

Xavier Leroy (2020/09/02 02:38 -0700):

I agree we have too many fragile tests in the test suite, where the reference output needs to be updated all the time. Another example is tests that record a full exception backtrace in the reference output. These tests break every time line numbers change in the source of the tests, e.g. because the magic ocamltest comment at the top is changed. This is annoying indeed.

The test could be written in an other module than the main one so the length of the test block wouldn't matter any longer.

[damiendoligez]

Or maybe change the ocamltest parser to allow placing the test block at the end of the file?

[shindere]

Damien Doligez (2021/01/13 02:54 -0800):

Or maybe change the `ocamltest` parser to allow placing the test block at the end of the file?

Hmm, not sure I find this very intuitive, but if there is a ocnsensus that it may help, I can work on it.