- removed comment
test system does not detect extra lines in output files
I accidentally removed some lines of output from the stored test suite data in git hash [https://bitbucket.org/eloisa/ctthorns/commits/79c5e0d32fb1e3d6b219d7b55211fa3c388f7268 79c5e0d32fb1e3d6b219d7b55211fa3c388f7268] of ctthorns in the files CT_MultiLevel/test/constraints_spherical/*_norm_eqn?.asc
. Yet the test all passed. I have since restored the changed files.
This I would consider quite serious since extra output lines should always cause the test to fail rather than being silently ignored.
I am marking this as critical since it potentially renders the test suites useless when detecting changes. If we fell we can "document away" this then, it can be downgraded to "major".
Keyword: testsuite
Comments (10)
-
-
- removed comment
What about the following? This fixes the problem for me:
@@ -1783,6 +1783,16 @@ sub CompareTestFiles print "TESTSUITE ERROR: Didn't catch case in CompareFiles\n"; } } #while + while ($nline = <INNEW>) + { + # ignore trailing comment lines in new file + last unless ($nline =~ /^\s*(["#].*)?$/); + } + if (!eof(INNEW)) + { + $rundata->{"$thorn $test $file NFAILWEAK"}++; + $rundata->{"$thorn $test $file NFAILSTRONG"}++; + } } elsif (!-e $newfile && -s $oldfile)
-
reporter - removed comment
Do you know how this affects the maximum difference value that the test suite reports? I always find it strange if the test system reports a failure but all differences are below the threshold.
-
- removed comment
I does report one line above tolerances (I only changed one), but I don't remember it reporting the value of the difference. Even if it would, there isn't really a difference to report here.
-
reporter - changed status to open
- removed comment
-
reporter - removed comment
Looking at the actual file
lib/sbin/RunTestUtils.pl
this also does not seem to handle the case where the new file is too short properly. As far as I can tell a too-short new file results in its lines being assumed empty which perl will interpret as "all fields have value 0.". This may pass in case the expected data is small.I think we need at least four new error types: 1. lines missing 1. extra lines 1. columns missing 1. extra columns all of which are strong failures.
-
reporter - changed status to open
- removed comment
-
- removed comment
I agree that all essentially all differences should be somehow handled. And in general I am convinced that the whole parsing/analyzing of the files should be rewritten. But that is another issue entirely. Can we agree that the patch above is sufficient to handle the current problem in this ticket? If so, I would like to apply it, and close this ticket.
-
- changed status to resolved
- removed comment
Applied in 78975f987706
-
reporter - edited description
- changed status to closed
- Log in to comment
True - currently the test mechanism only compares as many lines as the original testsuite file has (line 1690 in RunTestUtils.pl (while ($oline = <INORIG>)). There is no check for more data in the new output file. And I agree that this should be there.