#1872: test system does not detect extra lines in output files -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: new Priority: critical | Milestone: ET_2016_11 Component: Cactus | Version: development version Keywords: testsuite | -----------------------+---------------------------------------------------- I accidentally removed some lines of output from the stored test suite data in git hash [https://bitbucket.org/eloisa/ctthorns/commits/79c5e0d32fb1e3d6b219d7b55211fa... 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".
#1872: test system does not detect extra lines in output files -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: new Priority: critical | Milestone: ET_2016_11 Component: Cactus | Version: development version Resolution: | Keywords: testsuite -----------------------+----------------------------------------------------
Comment (by knarf):
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.
#1872: test system does not detect extra lines in output files -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: new Priority: critical | Milestone: ET_2016_11 Component: Cactus | Version: development version Resolution: | Keywords: testsuite -----------------------+----------------------------------------------------
Comment (by knarf):
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) }}}
#1872: test system does not detect extra lines in output files -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: new Priority: critical | Milestone: ET_2016_11 Component: Cactus | Version: development version Resolution: | Keywords: testsuite -----------------------+----------------------------------------------------
Comment (by rhaas):
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.
#1872: test system does not detect extra lines in output files -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: new Priority: critical | Milestone: ET_2016_11 Component: Cactus | Version: development version Resolution: | Keywords: testsuite -----------------------+----------------------------------------------------
Comment (by knarf):
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.
#1872: test system does not detect extra lines in output files -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: review Priority: critical | Milestone: ET_2016_11 Component: Cactus | Version: development version Resolution: | Keywords: testsuite -----------------------+---------------------------------------------------- Changes (by rhaas):
* status: new => review
#1872: test system does not detect extra lines in output files -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: review Priority: critical | Milestone: ET_2016_11 Component: Cactus | Version: development version Resolution: | Keywords: testsuite -----------------------+----------------------------------------------------
Comment (by rhaas):
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.
#1872: test system does not detect extra lines in output files -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: reopened Priority: critical | Milestone: ET_2016_11 Component: Cactus | Version: development version Resolution: | Keywords: testsuite -----------------------+---------------------------------------------------- Changes (by rhaas):
* status: review => reopened
#1872: test system does not detect extra lines in output files -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: reopened Priority: critical | Milestone: ET_2016_11 Component: Cactus | Version: development version Resolution: | Keywords: testsuite -----------------------+----------------------------------------------------
Comment (by knarf):
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.
#1872: test system does not detect extra lines in output files -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: closed Priority: critical | Milestone: ET_2016_11 Component: Cactus | Version: development version Resolution: fixed | Keywords: testsuite -----------------------+---------------------------------------------------- Changes (by knarf):
* status: reopened => closed * resolution: => fixed
Comment:
Applied in 78975f987706
trac@lists.einsteintoolkit.org
-
Einstein Toolkit