diff mbox

Don't ignore testsuite errors in Makefile

Message ID 201110091632.12412.mikael.morin@sfr.fr
State New
Headers show

Commit Message

Mikael Morin Oct. 9, 2011, 2:32 p.m. UTC
Hello,

currently, the testsuite return value is ignored by make. It is a little 
annoying if one wants to check automatically for regressions as we have to 
parse the testsuite output.
This patch reverts to the normal make behaviour, which is to not ignore 
commands' return values. Note: As a result the -k flag has to be added to the 
make command line if one wants the tests to continue after one failure.

OK for trunk?

Mikael

PS: Jakub, I CCed you as you are the author of the Makefile chunk.
2011-10-09  Mikael Morin  <mikael.morin@sfr.fr>

	* Makefile.in (check-parallel-%): Don't ignore testsuite errors.

Comments

Jakub Jelinek Oct. 9, 2011, 7:12 p.m. UTC | #1
On Sun, Oct 09, 2011 at 04:32:12PM +0200, Mikael Morin wrote:
> currently, the testsuite return value is ignored by make. It is a little 
> annoying if one wants to check automatically for regressions as we have to 
> parse the testsuite output.
> This patch reverts to the normal make behaviour, which is to not ignore 
> commands' return values. Note: As a result the -k flag has to be added to the 
> make command line if one wants the tests to continue after one failure.
> 
> OK for trunk?

Please no.  This is a very bad idea, most of the testsuites on many
architectures contain some FAILs and a failure from check-parallel-% would
mean the *.log/*.sum files would be never merged in that case.

If you really need to propagate the return value (I fail to see how it is
useful), then you should e.g. store the $? value from $(RUNTEST) in
check-parallel-% into some file in that directory and have the
parallelization goal after the merging collect those from the individual
files and or them all together into the final return value.

> 2011-10-09  Mikael Morin  <mikael.morin@sfr.fr>
> 
> 	* Makefile.in (check-parallel-%): Don't ignore testsuite errors.

> Index: Makefile.in
> ===================================================================
> --- Makefile.in	(révision 179710)
> +++ Makefile.in	(copie de travail)
> @@ -5116,10 +5124,10 @@ $(patsubst %,%-subtargets,$(lang_checks_paralleliz
>  # Otherwise check-$lang isn't parallelized and runtest is invoked just with
>  # the $(RUNTESTFLAGS) arguments.
>  check-parallel-% : site.exp
> -	-test -d plugin || mkdir plugin
> -	-test -d $(TESTSUITEDIR) || mkdir $(TESTSUITEDIR)
> +	test -d plugin || mkdir plugin
> +	test -d $(TESTSUITEDIR) || mkdir $(TESTSUITEDIR)
>  	test -d $(TESTSUITEDIR)/$(check_p_subdir) || mkdir $(TESTSUITEDIR)/$(check_p_subdir)
> -	-(rootme=`${PWD_COMMAND}`; export rootme; \
> +	(rootme=`${PWD_COMMAND}`; export rootme; \
>  	srcdir=`cd ${srcdir}; ${PWD_COMMAND}` ; export srcdir ; \
>  	cd $(TESTSUITEDIR)/$(check_p_subdir); \
>  	rm -f tmp-site.exp; \


	Jakub
Mikael Morin Oct. 9, 2011, 8:08 p.m. UTC | #2
On Sunday 09 October 2011 21:12:12 Jakub Jelinek wrote:
> On Sun, Oct 09, 2011 at 04:32:12PM +0200, Mikael Morin wrote:
> > currently, the testsuite return value is ignored by make. It is a little
> > annoying if one wants to check automatically for regressions as we have
> > to parse the testsuite output.
> > This patch reverts to the normal make behaviour, which is to not ignore
> > commands' return values. Note: As a result the -k flag has to be added to
> > the make command line if one wants the tests to continue after one
> > failure.
> > 
> > OK for trunk?
> 
> Please no.  This is a very bad idea, most of the testsuites on many
> architectures contain some FAILs and a failure from check-parallel-% would
> mean the *.log/*.sum files would be never merged in that case.
> 
> If you really need to propagate the return value (I fail to see how it is
> useful), then you should e.g. store the $? value from $(RUNTEST) in
> check-parallel-% into some file in that directory and have the
> parallelization goal after the merging collect those from the individual
> files and or them all together into the final return value.
Thanks for the tips. I will just keep the patch locally for now.
I don't use parallel testing anyway.

Mikael
diff mbox

Patch

Index: Makefile.in
===================================================================
--- Makefile.in	(révision 179710)
+++ Makefile.in	(copie de travail)
@@ -5116,10 +5124,10 @@  $(patsubst %,%-subtargets,$(lang_checks_paralleliz
 # Otherwise check-$lang isn't parallelized and runtest is invoked just with
 # the $(RUNTESTFLAGS) arguments.
 check-parallel-% : site.exp
-	-test -d plugin || mkdir plugin
-	-test -d $(TESTSUITEDIR) || mkdir $(TESTSUITEDIR)
+	test -d plugin || mkdir plugin
+	test -d $(TESTSUITEDIR) || mkdir $(TESTSUITEDIR)
 	test -d $(TESTSUITEDIR)/$(check_p_subdir) || mkdir $(TESTSUITEDIR)/$(check_p_subdir)
-	-(rootme=`${PWD_COMMAND}`; export rootme; \
+	(rootme=`${PWD_COMMAND}`; export rootme; \
 	srcdir=`cd ${srcdir}; ${PWD_COMMAND}` ; export srcdir ; \
 	cd $(TESTSUITEDIR)/$(check_p_subdir); \
 	rm -f tmp-site.exp; \