test system does not detect extra lines in output files

Create issue
Issue #1872 closed
Roland Haas created an issue

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)

  1. Frank Löffler
    • removed 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.

  2. Frank Löffler
    • 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)
    
  3. Roland Haas 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.

  4. Frank Löffler
    • 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.

  5. Roland Haas 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.

  6. Frank Löffler
    • 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.

  7. Log in to comment