* Re: [PATCH] testsuite: Add --status to runtest invocation
2016-01-19 11:24 ` Pedro Alves
@ 2016-01-19 16:01 ` Simon Marchi
2016-01-19 16:04 ` Pedro Alves
0 siblings, 1 reply; 5+ messages in thread
From: Simon Marchi @ 2016-01-19 16:01 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
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 <simon.marchi@ericsson.com>
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
--
2.5.1
^ permalink raw reply [flat|nested] 5+ messages in thread