[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#41918: [PATCH] Propagate error value of auto-loaded command

From: Jacob Bachmeyer
Subject: bug#41918: [PATCH] Propagate error value of auto-loaded command
Date: Fri, 19 Jun 2020 19:32:12 -0500
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20090807 MultiZilla/ SeaMonkey/1.1.17 Mnenhy/

Tom de Vries wrote:
On 6/19/20 2:00 AM, Jacob Bachmeyer wrote:
Tom de Vries wrote:
On 6/18/20 1:19 AM, Jacob Bachmeyer wrote:
Tom de Vries wrote:

I think I found a bug in proc unknown in lib/framework.exp.

Patch describing the problem and fixing it attached below.
I found a similar issue while patching bug #41824; please check whether
that patch addresses this issue adequately.
AFAICT, it does not.  Dejagnu test-case attached.
That test is incorrect:  it does not specify the --keep_going option,
but expects runtest to continue after a test script aborts with a Tcl

Well, yes, it's respecting the difference in handling of proc-not-found
vs other tcl errors that is present in current dejagnu.

- your implementation of --keep_going did not modify this, and
- your description of keep-going in NEWS explictly respected that
I had no reason to deviate from that.

Now that you've decided to change the handling of proc-not-found vs
other tcl errors in patch 0005, obviously the test-case needs to be
updated to accomodate for that.

That handling was actually very inconsistent: an error in a regular testcase aborts the script, but testing (almost) silently continued, but an error if auto-loading had been used aborted the entire test run. That difference in handling proc-not-found and other Tcl errors was a long-standing bug in DejaGnu. Any Tcl error means that the results are not really valid, so almost silently continuing is wrong and was wrong.

I have also attached another patch (0005) that fixes an inconsistency in
DejaGnu's handling of Tcl errors in test scripts.  Previously, DejaGnu
would abort the test run if an undefined procedure is called, but would
only issue an ERROR message and continue with the next test script for
all other Tcl errors.  This patch solves that by ensuring that DejaGnu
will abort on any uncaught Tcl error in a test script unless the
--keep_going option is given on the command line.  An UNRESOLVED result
appears in the log in any case if a test script aborts due to an error.

I couldn't apply this cleanly, so instead I'm now looking at branch PR41918.

Odd, but that is why I pushed that branch.

I see that for abort-undef.exp we have:
ERROR: (DejaGnu) proc "bogus_command 1 2 3 4" does not exist.
The error code is TCL LOOKUP COMMAND bogus_command
The info on the error is:
invalid command name "bogus_command"
    while executing
"::tcl_unknown bogus_command 1 2 3 4"
    ("uplevel" body line 1)
    invoked from within
"uplevel 1 ::tcl_unknown $args"
ERROR: tcl error sourcing
ERROR: tcl error code TCL LOOKUP COMMAND bogus_command
ERROR: invalid command name "bogus_command"
    while executing
"bogus_command 1 2 3 4"
line 23)
    invoked from within
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 source
    invoked from within
"catch "uplevel #0 source $test_file_name""

Should we error twice on this? FWIW, just removing ::unknown seems to be
the easiest way to fix that.

I have since realized that DejaGnu's ::unknown seems to be about useless other than producing that first error. I am thinking about removing it entirely now that runtest aborts after catching a Tcl error from a test script.

Anyway, I have two patches attached:
- one that rewords the NEWS entries into something more intuitive for me
Agreed; the NEWS entries were written separately and the later change did make the first one wrong. Thanks.

- one that adds a test-case abort-dbz.exp, a copy of
  abort-al-dbz.exp, but without the auto-load bit.
Agreed; that was a missing case in the testsuite.  Thanks.

-- Jacob

reply via email to

[Prev in Thread] Current Thread [Next in Thread]