Tom de Vries wrote:
> On 6/17/20 5:26 AM, Jacob Bachmeyer wrote:
>   
>> Tom de Vries wrote:
>>     
>>> On 6/16/20 6:05 AM, Jacob Bachmeyer wrote:
>>>  
>>>       
>>>> Tom de Vries wrote:
>>>>         
>>>>> A possible solution to this problem could be to make the exiting on
>>>>> error (which is done in dejagnu's version of proc unknown in
>>>>> framework.exp) optional.
>>>>>         
>>>>>           
>>>> This is obviously a good solution, but leads to the obvious question of
>>>> how (command-line option?  (Make normally aborts on error, but has a
>>>> --keep-going option.)  configuration variable?) that should be
>>>> determined, or if exit-on-unknown-procedure should ever be a default.
>>>> Could simply reporting an UNRESOLVED test (indicating that the testcase
>>>> script aborted due to calling an undefined command) be a better
>>>> option? It would show up in the summary report at the end.
>>>>
>>>> At first, I was planning to schedule this for the 1.7 series, but I
>>>> checked and runtest.exp:runtest already catches Tcl errors, so fixing
>>>> this does not require a significant change to the normal control
>>>> flow. Would simply reporting an "UNRESOLVED:  testcase $file aborted on
>>>> unknown command '$what'" result, then propagating the Tcl error in the
>>>> ::unknown proc be a workable solution?
>>>>     
>>>>         
>>> I think so, yes.  I've tried that out in attached gdb patch, which also
>>> solves the reliance on ::tcl_unknown.
>>>   
>>>       
>> That patch is subtly wrong in other ways.  First, it assumes that
>> ::unknown is a procedure and not a command; there is no guarantee of
>> this in the Tcl manual.  Second, it will fail if run inside a safe
>> interpreter where the default "unknown" does not exist.  Third, it
>> breaks Tcl's default autoloading because it adds an UNRESOLVED result
>> regardless of the result of Tcl's autoload search.
>>     
>
> Your last remark here got me thinking about dejagnu's unknown proc, and
> I've filed two bugs:
> - bug#41914: Propagate return value of auto-loaded command
> - bug#41918: Propagate error value of auto-loaded command
>   

I believe both of these were fixed while working on this issue in the 
attached patch.  Apologies for the delay; I wanted tests for the new 
feature.

>> The best solution is to just revert the hacks and wait for this to be
>> solved upstream, where it is expected to land for the 1.6.3 release.
>>
>>     
>>> [...]
>>> Note btw that in both cases I didn't put $file in the unresolved
>>> message, because that's already present in pf_prefix, but that might not
>>> always be true.
>>>   
>>>       
>> The use of pf_prefix appears to be a feature of the GDB testsuite, so
>> the upstream implementation of this feature will not be able to rely on it.
>>
>>
>> I am hesitant to alter default behavior for 1.6.3, so this will probably
>> be added as a "--keep-going" option to runtest.>  I should be able to
>> land this for 1.6.3; it is simple enough to do and we are already
>> getting some other new command-line options in 1.6.3 anyway that were
>> needed to fix DejaGnu's own testsuite.  As of yet, there is no
>> (supported) way planned to set the --keep-going option from an init
>> file, only on the actual command-line.  This may change in the future;
>> if it does, there will be some general API or just a special parse of
>> ~/.dejagnurc looking for command-line options.  These are planned for
>> 1.7 at the earliest.
>>     
>
> I guess it would be convenient if we could switch this on somehow from
> lib/${tool}.exp.

The problem here is that this detects an invalidity in the testsuite.  
Allowing testsuites to select --keep_going is asking (Murphy's Law 
again) for people to turn it on and then ignore the errors from broken 
test scripts "because it works" -- never mind that it is obviously 
broken, but an extra UNRESOLVED or three can still be easily ignored.  
The entire test run stopping cold unless a special command-line option 
is given that is specifically documented to suppress abort-on-error is 
much harder to explain away or even try to justify as acceptable.


-- Jacob