Subject: Re: bug#41824: Dejagnu's unknown proc aborts testsuite run when triggered in test-case

From: Jacob Bachmeyer <[email-masked]>
Date: Thu, 25 Jun 2020 20:53:46 -0500

Rob Savoye wrote:
> On 6/25/20 10:01 AM, Pedro Alves wrote:
>   
>   Adding an additional call makes it harder to diff the .sum files. Not
> a huge big deal, but still... UNRESOLVED says a human has to look at
> this test case output and decide, the actual error should be in the .log
> file. UNTESTED means something went wrong with the test, and the test is
> invalid, which is probably what you'd want as the test state for a Tcl
> error.
>   

The original rationale for using UNRESOLVED here was that DejaGnu 
converts a test result to UNRESOLVED if too many errors or warnings were 
produced.  The DejaGnu manual indicates (section "A POSIX Compliant Test 
Framework") that UNRESOLVED is correct for a test where execution was 
interrupted or was set up incorrectly.  UNTESTED is specifically listed 
as a placeholder for an as-yet-unwritten testcase.  A "typical" GDB 
testsuite run has almost a hundred UNTESTED results but (without this 
patch) zero UNRESOLVED results.

To me, this seems that UNRESOLVED is correct here, or that the manual 
has an error.

>> Currently with DejaGnu only the case of calling an unknown function stops
>> the test run AFAIK.  Any other tcl error like calling a proc with the wrong
>> number of arguments, or treating a non-array variable as an array, etc. is
>> caught by the catch in the runtest proc in runtest.exp, and the testrun
>> continues.  That to me clearly indicates that the original intention was
>> to catch errors and continue.
>>     
>
>   The original intent was to catch some errors, nd ignore others. :-)
> Catch is used for errors that can be ignored or worked around. Unknown
> was basically to catch programming errors, since as noted, there are
> many paths though the code as you change options or environments. They'd
> get lost in the output unless it aborted.
>   

So continuing with the next test script could be reasonable if the error 
(or at least an indication that the error occurred) is "replayed" at the 
end of the test run?

>> It is just that the "unknown" case wasn't thought of.  So I argue that not
>> making unknown proc calls abort the run is a bug fix, and making DejaGnu
>> abort the run for other errors by default is a behavior change that no real
>> testsuite built around DejaGnu is asking for.
>>     
>
>   I think the default should be to abort on a unknown Tcl error, and
> have an option to ignore them and continue. As a person that has run
> many long test runs, I agree I don't want some errors to abort the test
> run. But to be honest, a dependable test run should mean no bugs in the
> test framework. A buggy test framework means you can trust the test results.
>
>   Aborting was for use during debugging of test framework code itself,
> so supposedly you should should never have an Tcl error during a test
> run. There are times when even during debugging you'd want it to
> continue, and then collect all the bugs later to fix in batch mode.
>   

This succinctly states the original motivation for having the 
--keep-going option, but having "make check" always pass --keep-going 
defeats the purpose.

-- Jacob