[Top][All Lists]

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

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

From: Jacob Bachmeyer
Subject: bug#41824: Dejagnu's unknown proc aborts testsuite run when triggered in test-case
Date: Tue, 16 Jun 2020 22:26:24 -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/16/20 6:05 AM, Jacob Bachmeyer wrote:
Tom de Vries wrote:
If I add a call to foobar at the end of a test-case in that testsuite,
and I do a run of the testsuite, then the test-case aborts, and
subsequently the testsuite run aborts, that is, no further tests are run.

While it is as expected that the test-case aborts, it's not desirable
that the testsuite run aborts.
The main problem I have with this is that calling an unknown procedure
is presumably a very serious error.  Aborting the entire run is a good
way to ensure that the problem is noticed, but perhaps with a testsuite
as large (and long-running) as the GDB testsuite has become, continuing
may also be reasonable to salvage at least some results.

Where in the GDB testsuite is this a problem?  When is it appropriate to
continue after such a serious error?

It's not a problem in a specific test-case, but a generic problem. If
somebody makes a typo in a test-case and checks it in, there's no need
to abort the test run.

So the root cause problem is people committing changes to the GDB testsuite without actually testing their own changes first, at all?

How to ensure that the error is
not buried in the logs amidst the normal output from other tests and is
actually noticed?

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...

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.

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.

-- Jacob

reply via email to

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