Tests runner questions

[sergeyklay]

Hello,

This is not a real issue, just question regarding run_tests.sh usage.

---

This looks like a typo for me:

re2c/run_tests.sh.in

Lines 238 to 239 in 50ac133

	[ $status -gt 0 ] && soft_errors=$(($soft_errors + 1))
	[ $status -gt 1 ] && hard_errors=$(($hard_errors + 1))

Are hard errors the sum of all exit codes greater than zero? Let's say we have 5 exit codes with 1 and also we have 5 exit codes with 2. What value will the variables have? I assume $soft_errors will be 5 and $hard_errors will be 10.

---

Why the standard output is not redirected here:

re2c/run_tests.sh.in

Line 217 in 50ac133

${valgrind} ${wine} ../${re2c} $incpaths $switches 2>"$outy.stderr"

But is redirected here:

re2c/run_tests.sh.in

Line 185 in 50ac133

$valgrind $wine ../$re2c $incpaths $switches 2>"$outy.stderr" 1>&2

---

Should this code remove *.re files (it is $outx here)?

re2c/run_tests.sh.in

Lines 233 to 235 in 50ac133

	if [ $status -le 1 -a $keep_tmp_files -eq 0 ]; then
	rm -f "$outx" "$outy"{,.line*.{input,keys},.stderr,.out}
	fi

If so, we'll have an empty test_* directory then, because all other files are deleted too (including output and generated ones). Why we need this empty directory after all?

What do you think about simplification:

- rm -f "$outx" "$outy"{,.line*.{input,keys},.stderr,.out}
+ rm -f "$outx" "$outy*"

---

The status code is not checked for negative number here, but I faced this situation in this issue #331:

re2c/run_tests.sh.in

Lines 238 to 239 in 50ac133

	[ $status -gt 0 ] && soft_errors=$(($soft_errors + 1))
	[ $status -gt 1 ] && hard_errors=$(($hard_errors + 1))

re2c.exe on Windows can return negative status code

[skvadrik]

Are hard errors the sum of all exit codes greater than zero? Let's say we have 5 exit codes with 1 and also we have 5 exit codes with 2. What value will the variables have? I assume $soft_errors will be 5 and $hard_errors will be 10.

Vice versa, $soft_errors will be 10 (all errors greater than 0), and $hard_errors will be 5 (all errors greater than 1). This is both how it works and the intended behavior.

The idea is that hard errors are a subset of soft errors, which are in turn a subset of all tests (in Venn diagrams all tests would be a big white circle, soft errors would be a grey circle inside of the white circle, and hard errors would be a black circle inside of the grey circle). If it makes more sense to partition hard and soft errors into disjoint subsets, it can surely be done.

Let me clarify the process. Soft errors make sense only for skeleton tests, which consist of multiple steps:

1. generate *.c, *.input and *.keys files with re2c
2. compile *.c with a C compiler
3. run the compiled executable (it will use *.input and *.keys files)

Unlike normal tests that compare the generated *.c file to some reference result, skeleton tests do not have a reference result. And in some tests re2c may fail with an error, which is fine, because these tests do test error messages. So this is a soft failure --- the failed test must stop at step 1, but the whole test run shouldn't fail because of it. On the other hand, if instead of 205 soft failures we get, say, 1000, that would be a bug (currently we don't ensure that the number of soft failures hasn't gone up, but this should be done). Also, soft failures are limited to return code 1 which re2c uses on exit when reporting an error. If the test fails on step 1 with some other error code, it is an "unknown error", which is considered a hard error. If the test did not fail on step 1, it proceeds to step 2, where the generated code is compiled. If it fails compilation, that indicates a bug in re2c (it generated bad code), so it is a hard failure. Otherwise the test proceeds to step 3, where it runs the resulting test binary on the generated files. If it fails here, it means that either skeleton has found an error, or there is a bug in skeleton logic (both are hard failures).

Return codes 0, 1 and * correspond to step 1, return code 2 corresponds to step 2, return code 3 corresponds to step 3:

re2c/run_tests.sh.in

Lines 224 to 230 in 50ac133

	case $status in
	0 ) local msg="OK" ;;
	1 ) local msg="OK (expected re2c error)" ;;
	2 ) local msg="FAIL (compilation error)" ;;
	3 ) local msg="FAIL (runtime error)" ;;
	* ) local msg="FAIL (unknown error)" ;;
	esac

[skvadrik]

Why the standard output is not redirected here [...] But is redirected here [...]

This is because for normal (non-skeleton) tests we check every bit of output that re2c generates against a reference result (it should coincide verbatim, modulo some newline issues on Windows). That includes all output files that re2c might generate, as well as stdout and stderr (note that there can be multiple files, e.g. an optional header or *.input/*.keys files for tests that use --skeleton option, not to be confused with skeleton tests). If re2c writes something on stdout, we want to know what that is and if it should be there (so we compare it against a reference result).

Unlike normal tests, for skeleton tests we do not check the generated C code. stderr is captured because we want something to look at in case the test fails. I think it won't harm to capture stdout as well, but it will be always empty and will only clutter the output directory.

[skvadrik]

Should this code remove *.re files (it is $outx here)?

It should: the idea is that all *.re files are copied into a temporary directory, and on a clean test run the temporary directory ends up empty. If, however, there was an error, the related *.re file remains (but no other *.re files, which would clutter the output directory).

What do you think about simplification: rm -f "$outx" "$outy*"

I think it should be fine, that's what non-skeleton tests do.

[skvadrik]

The status code is not checked for negative number here, but I faced this situation in this issue #331

Oh! That is unexpected. The code should be fixed to put the negative error code into "unknown failure" category. Something like this:

if [ $status -eq 0 ]; then
    # ok
elif [ $status -eq 1 ]; then
    # re2c reported an error => soft fail
    soft_errors=$(($soft_errors + 1))
else
    # compilation error / runtime error / unknown error => hard fail
    hard_errors=$(($hard_errors + 1))
fi

[skvadrik]

Thanks for working on this @sergeyklay ! I'm curious to see what those negative return codes are.

[sergeyklay]

I think it should be fine, that's what non-skeleton tests do.

I'm sorry, but I can find this. What I see is that keep_tmp_files is used exactly once for skeleton tests. Could you please indicate the place where this is used for non-skeleton tests?

[sergeyklay]

I think it won't harm to capture stdout as well, but it will be always empty and will only clutter the output directory.

Actually no. I see a lot of /* Generated by re2c */ strings in stdout on Windows and this is the main reason why I was interested in the absence of redirection.

[sergeyklay]

Oh! That is unexpected. The code should be fixed to put the negative error code into "unknown failure" category. Something like this:

Ok. Should we fix this, right?

[skvadrik]

I'm sorry, but I can find this. What I see is that keep_tmp_files is used exactly once for skeleton tests. Could you please indicate the place where this is used for non-skeleton tests?

I didn't mean that non-skeleton tests use keep_tmp_files, I meant that non-skeleton tests use glob here: https://github.com/skvadrik/re2c/blob/master/run_tests.sh.in#L206. In any case, I think using glob should be fine (I didn't actually try it).

Actually no. I see a lot of /* Generated by re2c */ strings in stdout on Windows and this is the main reason why I was interested in the absence of redirection.

Ah, I see. That is not windows-specific, I suspect it's the tests that generate header files. It might make sense to disable header generation for them, I'll have a look a bit later.

Ok. Should we fix this, right?

Yes, definitely.

[skvadrik]

This commit 745f6e3 removes the /* Generated by re2c */ lines from skeleton test output.

Still, changing run_tests.sh to grab stdout as well might be a good idea.

[sergeyklay]

I didn't mean that non-skeleton tests use keep_tmp_files,

Should we mention this fact here?

re2c/run_tests.sh.in

Line 10 in 50ac133

echo " --keep-temp-files don't delete temporary files after test run"

[skvadrik]

TBH I don't care much for --keep-tmp-files (although I added it myself, I never use it), so I don't mind dropping it altogether in favor of simplifying run_tests.sh a bit. But in case you find it useful, I think it would make more sense to make the behavior coherent between skeleton and non-skeleton tests instead of documenting the difference.

[sergeyklay]

Actually I'm OK with dropping it to simplify run_tests.sh a bit since this does not provide new features and process of finding problems does not become much more difficult due to lack of this option.

[skvadrik]

--keep-tmp-files is gone: ec9eecd

[sergeyklay]

Fine! Only one question is remains #334 (comment) Could you address this too?

[skvadrik]

Done: f31eb82

The other patch cleans up some intermediate files that remained after successful skeleton run and prevented the temporary directory from being removed: 4135d0c (it should only remain in case some tests failed).

[sergeyklay]

Good job! As I can see all my questions are answered. Should we close this issue?

[skvadrik]

Yes, I think it can be closed. Thanks for raising the issue!

[sergeyklay]

You're welcome :)