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: >>> >>>> Hi, >>>> >>>> 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 > error.
Well, yes, it's respecting the difference in handling of proc-not-found vs other tcl errors that is present in current dejagnu. Since: - your implementation of --keep_going did not modify this, and - your description of keep-going in NEWS explictly respected that difference, 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. > I have attached a fixed patch (0004) that avoids modifying the source > tree by setting up autoloading in the build tree. > Ack, thanks. > 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. 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 /home/vries/dejagnu/devel/src/testsuite/runtest.main/abort/testsuite/abort.test/abort-undef.exp. ERROR: tcl error code TCL LOOKUP COMMAND bogus_command ERROR: invalid command name "bogus_command" while executing "bogus_command 1 2 3 4" (file "/home/vries/dejagnu/devel/src/testsuite/runtest.main/abort/testsuite/abort.test/abort-undef.exp" line 23) invoked from within "source /home/vries/dejagnu/devel/src/testsuite/runtest.main/abort/testsuite/abort.test/abort-undef.exp" ("uplevel" body line 1) invoked from within "uplevel #0 source /home/vries/dejagnu/devel/src/testsuite/runtest.main/abort/testsuite/abort.test/abort-undef.exp" 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. Anyway, I have two patches attached: - one that rewords the NEWS entries into something more intuitive for me - one that adds a test-case abort-dbz.exp, a copy of abort-al-dbz.exp, but without the auto-load bit. Thanks, - Tom
Reword NEWS entries --- NEWS | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 619f0ff..abae60b 100644 --- a/NEWS +++ b/NEWS @@ -7,11 +7,10 @@ Changes since 1.6.2: should use this proc. The 'is_remote' proc is deprecated. 2. runtest now accepts --local_init and --global_init options to override the default of reading "site.exp". See the manual for details. +X. runtest now aborts if a test script fails with any Tcl error. Previously, + only calling an undefined procedure would cause the test run to abort. X. runtest now accepts a --keep_going option to continue with other test - scripts after a test script invokes an undefined command. -X. Unless the --keep_going option is used, runtest now aborts if a test - script fails with any Tcl error. Previously, only calling an undefined - procedure would cause the test run to abort. + scripts after a test script fails with a Tcl error. 3. A utility procedure relative_filename has been added. This procedure computes a relative file name to a given destination from a given base. 4. The utility procedure 'grep' now accepts a '-n' option that
Add abort.test/abort-dbz.exp --- testsuite/runtest.main/abort.exp | 11 ++++++++ .../abort/testsuite/abort.test/abort-dbz.exp | 29 ++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/testsuite/runtest.main/abort.exp b/testsuite/runtest.main/abort.exp index b352b56..59a934c 100644 --- a/testsuite/runtest.main/abort.exp +++ b/testsuite/runtest.main/abort.exp @@ -50,6 +50,17 @@ set tests { "PASS: running abort-undef.exp.*\ *UNRESOLVED: .* aborted.*\ *expected passes\[ \t\]+1\n.*unresolved testcases\[ \t\]+1\n" } + { "stop at divide-by-zero without --keep_going" + "abort-dbz.exp simple.exp" + "PASS: running abort-dbz.exp.*\ + *UNRESOLVED: .* aborted.*\ + *expected passes\[ \t\]+1\n.*unresolved testcases\[ \t\]+1\n" } + { "continue after divide-by-zero with --keep_going" + "--keep_going abort-dbz.exp simple.exp" + "PASS: running abort-dbz.exp.*\ + *UNRESOLVED: .* aborted.*\ + *PASS: simple test.*\ + *expected passes\[ \t\]+2\n" } { "stop at auto-loaded divide-by-zero without --keep_going" "abort-al-dbz.exp simple.exp" "PASS: running abort-al-dbz.exp.*\ diff --git a/testsuite/runtest.main/abort/testsuite/abort.test/abort-dbz.exp b/testsuite/runtest.main/abort/testsuite/abort.test/abort-dbz.exp new file mode 100644 index 0000000..711347d --- /dev/null +++ b/testsuite/runtest.main/abort/testsuite/abort.test/abort-dbz.exp @@ -0,0 +1,29 @@ +# Copyright (C) 2020 Free Software Foundation, Inc. +# +# This file is part of DejaGnu. +# +# DejaGnu is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# DejaGnu is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with DejaGnu; if not, write to the Free Software Foundation, +# Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. + +# Cause a divide-by-zero error. + +pass "running abort-dbz.exp" + +proc throw_arith_error_div_by_zero { } { + expr { 1 / 0 } +} + +throw_arith_error_div_by_zero + +fail "script did not abort"
_______________________________________________ Bug-dejagnu mailing list Bug-dejagnu@gnu.org https://lists.gnu.org/mailman/listinfo/bug-dejagnu