Re: [PATCH] testsuite: Add --status to runtest invocation

[Simon Marchi <simon dot marchi at ericsson dot com>]

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