|
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:1.8.1.22) Gecko/20090807 MultiZilla/1.8.3.4e SeaMonkey/1.1.17 Mnenhy/0.7.6.0 |
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 bedetermined, 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 fixingthis does not require a significant change to the normal control flow. Would simply reporting an "UNRESOLVED: testcase $file aborted onunknown 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.
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.[...] 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.
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
[Prev in Thread] | Current Thread | [Next in Thread] |