From patchwork Tue Jan 19 16:01:00 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 10450 Received: (qmail 40684 invoked by alias); 19 Jan 2016 16:01:12 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 40675 invoked by uid 89); 19 Jan 2016 16:01:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00, KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 spammy=Utility, factors, Step, 1937 X-HELO: usplmg21.ericsson.net Received: from usplmg21.ericsson.net (HELO usplmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 19 Jan 2016 16:01:06 +0000 Received: from EUSAAHC008.ericsson.se (Unknown_Domain [147.117.188.96]) by usplmg21.ericsson.net (Symantec Mail Security) with SMTP id 2C.99.32102.FAD5E965; Tue, 19 Jan 2016 17:00:47 +0100 (CET) Received: from [142.133.110.144] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.98) with Microsoft SMTP Server id 14.3.248.2; Tue, 19 Jan 2016 11:01:00 -0500 Subject: Re: [PATCH] testsuite: Add --status to runtest invocation To: Pedro Alves , References: <1453157560-7080-1-git-send-email-simon.marchi@ericsson.com> <569E1CF9.5010102@redhat.com> From: Simon Marchi Message-ID: <569E5DBC.4070408@ericsson.com> Date: Tue, 19 Jan 2016 11:01:00 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <569E1CF9.5010102@redhat.com> X-IsSubscribed: yes On 16-01-19 06:24 AM, Pedro Alves wrote: > On 01/18/2016 10:52 PM, Simon Marchi wrote: >> As discussed in this thread: >> >> https://sourceware.org/ml/gdb-patches/2016-01/msg00243.html >> >> By default, if a test driver (a test .exp) ends with an uncaught >> error/exception, the runtest command will still have a return code of 0 >> (success). I think that if a test (or the environment) is broken and >> the test ends up with an exception, it should be considered as failed so >> that we can notice it and fix it. >> >> Passing the --status flag to runtest will make it return an error if one >> of the test it runs ends up with an uncaught error. >> >> gdb/testsuite/ChangeLog: >> >> * Makefile.in (check-single): Pass --status to runtest. >> (check/%.exp): Likewise. > > Hmm, the perf bits of the runtest invocation already pass --status: > > @GMAKE_TRUE@ $(DO_RUNTEST) --status --outdir=gdb.perf/outputs/$* lib/build-piece.exp WORKER=$* GDB_PARALLEL=gdb.perf $(RUNTESTFLAGS) GDB_PERFTEST_MODE=compile GDB_PERFTEST_SUBMODE=build-pieces > ... > @GMAKE_TRUE@ $(DO_RUNTEST) --status --directory=gdb.perf --outdir gdb.perf/workers GDB_PARALLEL=gdb.perf $(RUNTESTFLAGS) GDB_PERFTEST_MODE=compile GDB_PERFTEST_SUBMODE=gen-workers > ... > @GMAKE_TRUE@ $(DO_RUNTEST) --status --directory=gdb.perf --outdir gdb.perf GDB_PARALLEL=gdb.perf $(RUNTESTFLAGS) GDB_PERFTEST_MODE=compile GDB_PERFTEST_SUBMODE=final > > But there's one that doesn't: > > check-perf: all $(abs_builddir)/site.exp > @if test ! -d gdb.perf; then mkdir gdb.perf; fi > $(DO_RUNTEST) --directory=gdb.perf --outdir gdb.perf GDB_PERFTEST_MODE=$(GDB_PERFTEST_MODE) $(RUNTESTFLAGS) > > Seems like an oversight? > > How about adding --status to DO_RUNTEST directly instead, so > that all invocations are always covered? That's fine with me. However, I did a wrong git command and ended up pushing this patch as we as "testsuite: Make check-parallel return non-zero if a test failed" without the ChangeLog. I added their ChangeLog entries in a separate commit. So here's a patch that goes on top of all that that factors out --status. From d64bd1d8ec33ec22ea6125e93bc670b3d19d5ec4 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Tue, 19 Jan 2016 10:53:20 -0500 Subject: [PATCH] testsuite: Factor out --status in DO_RUNTEST gdb/testsuite/ChangeLog: * Makefile.in (DO_RUNTEST): Add --status and update usages. --- gdb/testsuite/Makefile.in | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in index 50edf8a..f59acc3 100644 --- a/gdb/testsuite/Makefile.in +++ b/gdb/testsuite/Makefile.in @@ -169,7 +169,7 @@ DO_RUNTEST = \ if [ -f $${rootme}/../../expect/expect ] ; then \ TCL_LIBRARY=$${srcdir}/../../tcl/library ; \ export TCL_LIBRARY ; fi ; \ - $(RUNTEST) + $(RUNTEST) --status # TESTS exists for the user to pass on the command line to easily # say "Only run these tests." With check-single it's not necessary, but @@ -193,7 +193,7 @@ DO_RUNTEST = \ @GMAKE_TRUE@ $(MAKE) check TESTS="gdb.$*/*.exp" check-single: - $(DO_RUNTEST) --status $(RUNTESTFLAGS) $(expanded_tests_or_none) + $(DO_RUNTEST) $(RUNTESTFLAGS) $(expanded_tests_or_none) check-parallel: -rm -rf cache outputs temp @@ -229,7 +229,7 @@ do-check-parallel: $(TEST_TARGETS) @GMAKE_TRUE@check/%.exp: @GMAKE_TRUE@ -mkdir -p outputs/$* -@GMAKE_TRUE@ @$(DO_RUNTEST) GDB_PARALLEL=yes --outdir=outputs/$* $*.exp --status $(RUNTESTFLAGS) +@GMAKE_TRUE@ @$(DO_RUNTEST) GDB_PARALLEL=yes --outdir=outputs/$* $*.exp $(RUNTESTFLAGS) check/no-matching-tests-found: @echo "" @@ -239,7 +239,7 @@ check/no-matching-tests-found: # Utility rule invoked by step 2 of the build-perf rule. @GMAKE_TRUE@workers/%.worker: @GMAKE_TRUE@ mkdir -p gdb.perf/outputs/$* -@GMAKE_TRUE@ $(DO_RUNTEST) --status --outdir=gdb.perf/outputs/$* lib/build-piece.exp WORKER=$* GDB_PARALLEL=gdb.perf $(RUNTESTFLAGS) GDB_PERFTEST_MODE=compile GDB_PERFTEST_SUBMODE=build-pieces +@GMAKE_TRUE@ $(DO_RUNTEST) --outdir=gdb.perf/outputs/$* lib/build-piece.exp WORKER=$* GDB_PARALLEL=gdb.perf $(RUNTESTFLAGS) GDB_PERFTEST_MODE=compile GDB_PERFTEST_SUBMODE=build-pieces # Utility rule to build tests that support it in parallel. # The build is broken into 3 steps distinguished by GDB_PERFTEST_SUBMODE: @@ -259,11 +259,11 @@ check/no-matching-tests-found: @GMAKE_TRUE@ rm -rf gdb.perf/workers @GMAKE_TRUE@ mkdir -p gdb.perf/workers @GMAKE_TRUE@ @: Step 1: Generate the build .worker files. -@GMAKE_TRUE@ $(DO_RUNTEST) --status --directory=gdb.perf --outdir gdb.perf/workers GDB_PARALLEL=gdb.perf $(RUNTESTFLAGS) GDB_PERFTEST_MODE=compile GDB_PERFTEST_SUBMODE=gen-workers +@GMAKE_TRUE@ $(DO_RUNTEST) --directory=gdb.perf --outdir gdb.perf/workers GDB_PARALLEL=gdb.perf $(RUNTESTFLAGS) GDB_PERFTEST_MODE=compile GDB_PERFTEST_SUBMODE=gen-workers @GMAKE_TRUE@ @: Step 2: Compile the pieces. Here is the build parallelism. @GMAKE_TRUE@ $(MAKE) $$(cd gdb.perf && echo workers/*/*.worker) @GMAKE_TRUE@ @: Step 3: Do the final link. -@GMAKE_TRUE@ $(DO_RUNTEST) --status --directory=gdb.perf --outdir gdb.perf GDB_PARALLEL=gdb.perf $(RUNTESTFLAGS) GDB_PERFTEST_MODE=compile GDB_PERFTEST_SUBMODE=final +@GMAKE_TRUE@ $(DO_RUNTEST) --directory=gdb.perf --outdir gdb.perf GDB_PARALLEL=gdb.perf $(RUNTESTFLAGS) GDB_PERFTEST_MODE=compile GDB_PERFTEST_SUBMODE=final # The default is to both compile and run the tests. GDB_PERFTEST_MODE = both