Message ID | 20180111190055.4875-1-andrew.burgess@embecosm.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 65107 invoked by alias); 11 Jan 2018 19:01:02 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 65052 invoked by uid 89); 11 Jan 2018 19:01:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-wr0-f194.google.com Received: from mail-wr0-f194.google.com (HELO mail-wr0-f194.google.com) (209.85.128.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 11 Jan 2018 19:01:00 +0000 Received: by mail-wr0-f194.google.com with SMTP id 100so3171325wrb.7 for <gdb-patches@sourceware.org>; Thu, 11 Jan 2018 11:01:00 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=PoGOz20RJjbIZJCQPZ4zgb4oRWHmbX/+OeXhygA9OvA=; b=X9aRt+Bglfl/PCWZ3axzSIjxm2nFomw968byeYhrk27gHfsHffYA3WKEGU6zB3693S Rszp227BH0TjrxDS4qckbq0fwJ5i7fvjE6rqmmkslh7k+oHcde+MFNwKS6oI4bDsqUAQ X8Q8AoMmrgY06oj3lpPWrbv/rn3mU+3eLleGof2YucxtnoU/jj1H2njXKar5MZHBxGxL xCR1zJIQc24HR2Zsi/M43M/PapsAK/vl6bw/t3Z/A3+EDU1WojKZcLYRIrBuDjqSj1o+ emQUCZqyII54oSjKeqBoLZQ+MTviJf2Gy4fu+TS7/HVsI8WO3dFeJR3aJ5KLB2kMGW+6 KRSg== X-Gm-Message-State: AKwxytdo9Gm1jsgE4vn6rCwzZmZR7FWYwL2vSbjfbAsYmanSO8PWtavP eqJfraDt4SDd9tN9lToUd1Ahg+ah X-Google-Smtp-Source: ACJfBotUVvcF57Sj0HfHd7k3ZuQ1UHujqOj+ASKtS94SBmp2mdVgOuefAbPtjqR2ttSLlvN1BN5jsg== X-Received: by 10.223.154.77 with SMTP id z71mr3048189wrb.282.1515697258229; Thu, 11 Jan 2018 11:00:58 -0800 (PST) Received: from localhost ([81.141.199.69]) by smtp.gmail.com with ESMTPSA id s45sm32362945wrc.89.2018.01.11.11.00.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 11 Jan 2018 11:00:57 -0800 (PST) From: Andrew Burgess <andrew.burgess@embecosm.com> To: gdb-patches@sourceware.org Cc: Andrew Burgess <andrew.burgess@embecosm.com> Subject: [PATCH] gdb/testsuite: Don't attempt tests if they fail to compile Date: Thu, 11 Jan 2018 19:00:55 +0000 Message-Id: <20180111190055.4875-1-andrew.burgess@embecosm.com> X-IsSubscribed: yes |
Commit Message
Andrew Burgess
Jan. 11, 2018, 7 p.m. UTC
In the gdb.base/whatis-ptype-typedefs.exp test, if the test program fails to compile, don't run the tests. gdb/testsuite/ChangeLog: * gdb.base/whatis-ptype-typedefs.exp: Don't run tests if we failed to prepare. (prepare): Return 0 on error, 1 on success. --- gdb/testsuite/ChangeLog | 6 ++++++ gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp | 9 ++++++--- 2 files changed, 12 insertions(+), 3 deletions(-)
Comments
On 2018-01-11 02:00 PM, Andrew Burgess wrote: > In the gdb.base/whatis-ptype-typedefs.exp test, if the test program > fails to compile, don't run the tests. Hi Andrew, That would make the test similar to other test, in that if we fail to build the test program it's not a failure (it shows as UNTESTED, doesn't make the test run fail). I find it's a strange behavior though. If a test program starts not building for some reason, I'd certainly like to know (e.g. it could be UNRESOLVED), instead of it silently failing. Any other opinion? Simon
* Simon Marchi <simon.marchi@ericsson.com> [2018-01-11 17:03:51 -0500]: > On 2018-01-11 02:00 PM, Andrew Burgess wrote: > > In the gdb.base/whatis-ptype-typedefs.exp test, if the test program > > fails to compile, don't run the tests. > > Hi Andrew, > > That would make the test similar to other test, in that if we fail to > build the test program it's not a failure (it shows as UNTESTED, doesn't > make the test run fail). I find it's a strange behavior though. If a > test program starts not building for some reason, I'd certainly like to > know (e.g. it could be UNRESOLVED), instead of it silently failing. > > Any other opinion? If the test fails to compile we don't get a silent failure, as you mention, we get the UNTESTED. Changing this to something stronger, like UNRESOLVED, would I fear make cases where we legitimately can't compile a test program seem worse than they really are. The concern about missing the case where a test program goes from compiling to not compiling is fair, however, I don't think that it's something we need to worry about. My understanding of the "normal" testing flow for GDB is to compare against a baseline set of results, a few hundred tests disappearing should raise a red flag, and once the developer has realised that this particular test script has something weird going on, the extra UNTESTED should guide them to the cause of the problem. The failed to prepare leading to skipping the tests seems like the "standard" pattern within the GDB testsuite, so, if you agree, I think having this test fall in line with that is probably a good thing. That doesn't mean we can't change the standard pattern in the future if we can come up with a better model (though I don't have any good suggestions). Thanks, Andrew
> > That would make the test similar to other test, in that if we fail to > > build the test program it's not a failure (it shows as UNTESTED, doesn't > > make the test run fail). I find it's a strange behavior though. If a > > test program starts not building for some reason, I'd certainly like to > > know (e.g. it could be UNRESOLVED), instead of it silently failing. > > > > Any other opinion? FWIW, no strong opinion on my side.
On 2018-01-12 05:23, Andrew Burgess wrote: > If the test fails to compile we don't get a silent failure, as you > mention, we get the UNTESTED. Changing this to something stronger, > like UNRESOLVED, would I fear make cases where we legitimately can't > compile a test program seem worse than they really are. An UNTESTED result does not make Dejagnu's runtest return a non-zero return code. So if a test file that we expect to compile does not compile, the test exits with success. That's why I consider it a silent failure. If we know that a test program does not make sense under the current conditions, I think it should be up to the .exp file to determine that and manually mark the test as UNTESTED. > The concern about missing the case where a test program goes from > compiling to not compiling is fair, however, I don't think that it's > something we need to worry about. My understanding of the "normal" > testing flow for GDB is to compare against a baseline set of results, > a few hundred tests disappearing should raise a red flag, and once the > developer has realised that this particular test script has something > weird going on, the extra UNTESTED should guide them to the cause of > the problem. Diffing gdb.sum against the baseline is indeed our current workflow, but it's not the ideal one. Ideally, you'd just run make check, and if it returns 0 then you can be confident that GDB is working ok. However, given that we do have failures in the testsuite, diffing gdb.sum is the next best solution. At Ericsson, for example (and I'm sure others do something similar), we have a CI for our GDB port where we run a stable subset of the tests, which we expect will pass. The builds fails when "make check" fails, that is when there's a FAIL, UNRESOLVED or KPASS. There are many (expected) UNTESTED results, because some tests (sometimes just a part of a .exp) doesn't apply/make sense on our platform. In this case, a test case that starts not building (it could even be due to an external factor, like a compiler upgrade) will probably go unnoticed. > > The failed to prepare leading to skipping the tests seems like the > "standard" pattern within the GDB testsuite, so, if you agree, I think > having this test fall in line with that is probably a good thing. > That doesn't mean we can't change the standard pattern in the future > if we can come up with a better model (though I don't have any good > suggestions). Indeed, I haven't been clear on that. I think your patch is good, because it makes the test look like the rest of the testsuite. I was just reflecting on that general pattern (hoping we can continue the discussion :)). So, please push, and thanks for the patch! Simon
diff --git a/gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp b/gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp index 763d2a43952..3d910df5d02 100644 --- a/gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp +++ b/gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp @@ -45,13 +45,15 @@ proc prepare {lang} { if { [prepare_for_testing "failed to prepare" \ ${out} [list $srcfile] $options] } { - return -1 + return 0 } if ![runto_main] then { fail "can't run to main" return 0 } + + return 1 } # The following list is layed out as a table. It is composed by @@ -300,6 +302,7 @@ proc run_tests {lang} { } foreach_with_prefix lang {"c" "c++"} { - prepare $lang - run_tests $lang + if { [prepare $lang] } then { + run_tests $lang + } }