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

[Jacob Bachmeyer]

Pedro Alves wrote:
> On 6/17/20 4:26 AM, Jacob Bachmeyer wrote:
>   
> > > I monitor ERROR and WARNING in my test runs, so I didn't think of this,
> > > but indeed, those are not tracked in the summary, so that could be a
> > > problem, agreed.
> > >   
> > >       
> > Blowing up the entire test run is a good way to ensure that problems (like 
> > typos in testcases) are noticed, but admittedly, that does not help when people 
> > fail to even run the tests before checking in their typos and then other 
> > developers' test runs blow up.  Murphy's Law says the offenders then carry on 
> > to check in more typos in other code without testing their own work there 
> > either...
> >     
>
> There's conditional execution in the gdb testsuite, where tests may
> run different code paths depending on the target.  Typos can happen in
> code paths that the developer isn't able to exercise.  You can
> imagine someone missing something when doing some large refactoring
> in the testsuite.
>   

This is a good point and is far more reasonable than checking in code 
without testing it at all.  That sounds like an architectural issue with 
the GDB testsuite, but could also be a legitimate problem if the 
per-target handling really is different for some targets, especially if 
simulators for those targets are unavailable or insufficient for the tests.

> I'd think that tracking ERROR and WARNING in the summary would sort
> out this "test run blew up but no one noticed" issue.

The problem is that the DejaGnu internals often generate ERROR and 
WARNING "directly" without using the procedures that update counters.  
The solution I am using in the current PR41824 patch (which has been 
rolled into the PR41918 patch) generates an extra UNRESOLVED result when 
a Tcl error is caught in runtest, which both appears in the summary and 
clearly indicates that the test run did not produce valid results.

I am currently considering also adding a 
'--no-keep-going'/'--no_keep_going' option and making '--keep-going' the 
default for the rest of the 1.6 series.  Aborting on uncaught Tcl errors 
will be the default in 1.7, but I am trying to decide whether changing 
it in 1.6 counts more as fixing a bug (DejaGnu quietly sweeping Tcl 
errors under the proverbial rug) or introducing an environment change 
(needing '--keep-going' to reach the end of some testsuites that 
"worked" before).  This is further complicated by the fact that any 
testsuite needing that option to reach the end means that, technically, 
that testsuite is invalid -- it *did* throw a Tcl error during testing.


-- Jacob