Cc @diml

FWIW, this PR passes Inria's precheck in addition to AppVeyor and Travis.

Yes, thanks for the clarification on the setup{,_simple}_build_env functions.

Re. input_file ^ ".corrected"

No the intermediate file is generated by the first run of the command.
It's used as input for the second run of the command.

I understand that, but I wonder what becomes of this intermediate file after the second run. Is it removed / cleaned up ? In the original Makefile, mv foo.corrected.corrected foo.corrected is performed after the second run, which guarantees that, by the time the script terminates, only two files are left on the disk, the original input file and the output file (of the second run).

@gasche why did you add a Changes entry for this PR?

Do you intent to do that for all the test migration PRs?

I thought that there was enough non-trivial work in this PR that you deserved to be credited for the work (and I for the review!). I guessed that you had opted out of the the Changes-entry-writing business because you might not be so comfortable with it, so instead of pinging you to write one before I merge I decided to just write it.

We can consolidate this entry for other test migrations, and/or only mention the ones that require significant amount of adaptation work.

I just rebased past this.
Can ocamltest put TERM=dumb in the environment of ocaml-expect, otherwise the tests break because colors? Also is there any way to see the diff of the failure now, and to promote the corrected file?

Promoting matters in practice when operating on a large number of tests whose output has to change. For example when working on #1511, I had to update the reference files of all the expect tests in the testsuite, and this was done with a for f in tests/*; do cd $f; make promote; done.

Right now you can compute a diff between the expected output and the output; it may be possible to add a --promote mode to ocamltest where the output is copied into the expected output instead of being compared.

I think that the file(s) that should be promoted is precisely the one(s) that cause(s) the test to fail. So I'd wait until ocamltest has decided which reference file to use, and promote this one.

If the test that I am running fails because the output (however it is computed) does not correspond to foo.compilers.reference (however this reference file is determined), then foo.compilers.reference should be modified. If it fails because the output does not correspond to bar.ocamlc.byte.reference, then that file should be modified. Doing anything less simple than that would not be a good idea.

Then the user will notice that there is an issue (because the tests will still fail after promotion) and will have to think harder about the problem, which is what we want to happen: if all compiler outputs used to be the same and now they are not, something interesting occurred and we need human supervision.

#1491 is what I was rebasing.

Normally the log file of the tests should include a diff when the result differs from the reference.

I don't get it. It sounds like you're saying the way to see the output of a failing test, beyond whether it failed or not, is something like:

$ ocamltest *.ml
$ grep '' _ocamltest/*/*.log

(well actually that would show both failing and passing tests, so this command is actually 90% noise)

So in practice, it looks like the way to go for now is:

$ ocamltest *.ml
$ for v in _ocamltest/*; do diff $v/*.ml $v/*.corrected.corrected; done

which is not noisy and allows me to use the diff program I want.

and to promote the corrected file?
No, that has not been implemented. But promoting is just a matter of moving / renaming a file, right?

Yes, but given that the .corrected files are hidden in the _ocamltest directory, it's annoying to actually find which command to type.

Also: ocamltest is leaking all these ocamltestXXXXXXdiff files, in filecompare.ml.

make promote does more than showing the diff, it also does exactly what is needed to update the reference files to have the test pass, so then it suffices to git commit when you did a change that affects (in a benign way) many tests.

Just not now, if nobody minds. Theplan is really to migrate as many tests as possible, as quickly as possible, so that we can get rid of the legacy framework.

It is a bit uncomfortable to break the workflow of people writing tests today when we migrate.

(Another workflow that is not available anymore in the new system is to perform the tests in parallel (I realized while trying to reproduce the color issue), but I think that this is easy to add and I'll try to send a PR soon.)

Yeah the workflow is kind of broken right now.
Before:

• cd testsuite/bla/bla/bla
• run make DIFF="diff -u" deps default, to see the test result
• run make promote, accept the changes

Now:

• make world etc
• cd testsuite/bla/bla/bla
• run ../../../ocamltest/ocamltest *.ml; for v in _ocamltest/*; do diff -u $v/*.ml $v/*.corrected.corrected; done to see the test result
• run some other script that finds all the .corrected.corrected and renames to their source files, to accept the changes

I can live with the current state of things, because now I know enough to script around it, but I'm guessing other people will run into this (or run all the tests I suppose, but you can't iterate with that, it's horribly slow).

In case it's useful to others, the script below should allow one to work on a single expect-test (doesn't help with @gasche's problem of promoting all tests).

The workflow becomes (even from a clean repo):

• cd testsuite/bla/bla/bla
• ocamltest, to see the test result
• ocamltest promote, to accept the change

No need for configure, or make world, or ../../../ocamltest/ocamltest, etc

$ cat ocamltest
#!/bin/bash

# Help:
# ocamltest: runs the expect tests in the current directory, after building
#            everything that's necessary
# ocamltest promote: promote the output of last run of tests to be the expected output

set -e -u -o pipefail

if [ $# -ge 1 ]; then
    case "$1" in
	promote)
	    shopt -s nullglob
	    for v in _ocamltest/*/*.corrected.corrected; do
		mv $v $(basename $v .corrected.corrected)
	    done
	    exit 0
	    ;;
	*) echo >&2 "unknown command $1"
	   exit 1
	   ;;
    esac
fi

root=$(git rev-parse --show-toplevel)
start_dir=$PWD

cd "$root"
if ! [ -f utils/config.ml ]; then
  ./configure -prefix "$PWD"/installed
fi
make coldstart ocaml ocamlc runtime
make -C testsuite/tools expect_test
make -C lex
make -C ocamltest

cd "$start_dir"
rm -rf _ocamltest
"$root"/ocamltest/ocamltest *.ml
function diff {
    if ! cmp -s "$@"; then
	if which patdiff &> /dev/null; then
	    patdiff "$@" && patdiff -keep-whitespace "$@" && command diff -u "$@"
	else
	    command diff -u "$@"
	fi
    fi
}
for v in _ocamltest/*; do ${DIFF-diff} $v/*.ml $v/*.corrected.corrected; done