Re: [PATCH 8/9] Tests on extensibility of silent-rules made stricter.

[Stefano Lattarini]

At Monday 26 April 2010, Ralf Wildenhues <address@hidden> 
wrote:
> * Stefano Lattarini wrote on Mon, Apr 26, 2010 at 12:15:33AM CEST:
> > --- a/tests/silent6.test
> > +++ b/tests/silent6.test
> >
> > @@ -99,5 +99,3 @@ $AUTOMAKE
> >  grep 'AM_V_GEN' Makefile.in
> >  $AUTOMAKE --force -Wno-all -Wportability
> >  grep 'AM_V_GEN' Makefile.in
> > -
> > -:
> 
> Please don't remove trailing ':' or 'Exit 0'.  They exist for a
>  reason: when somebody changes the test by adding a failing command
>  after the last command, the test will spuriously fail because $?
>  is nonzero at the end.  Keeping a trailing ':' prevents that. 
Good point, I dindn't think of that.  What about an addition to 
tests/README explicitly telling that apparently reduntant trailing 
commands are there for a good reason indeed?  My try is attached.

>  (And no, please also don't newly introduce these things just for
>  the heck of it either (i.e., when you wouldn't otherwise be
>  editing the test), that's not the point.)
Yes, that would not be worth doing, since a missing trailing `:' does 
not change the semantic of the test (while e.g. a missing `set -e'
does).  Moreover, the absence of `:' could cause false positives only, 
which are much much easier to spot than the false negatives that could 
be introduced by a missing `set -e'.

By the way, I didn't understand if you're going to amend the patches 
yourself, or if I should do that instead...

Regards,
    Stefano

From 7e393d5192690d69ea03b19a46f20073b8d2eea6 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Mon, 26 Apr 2010 10:46:43 +0200
Subject: [PATCH] Extend tests/README (trailing `:' in test scripts)

* tests/README (section "Writing test cases", subsection "Do"):
Explain why apparently redundant trailing `:' and `Exit 0' in
test scripts can indeed be useful.
---
 ChangeLog    |    8 ++++++++
 tests/README |    7 +++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 71b0d9b..b587d29 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2010-04-26  Stefano Lattarini  <address@hidden>
+           Ralf Wildenhues  <address@hidden>
+
+       Extend tests/README w.r.t. trailing `:' in test scripts.
+       * tests/README (section "Writing test cases", subsection "Do"):
+       Explain why apparently redundant trailing `:' and `Exit 0' in
+       test scripts can indeed be useful.
+
 2010-04-25  Ralf Wildenhues  <address@hidden>
 
        Warning and error message formatting cleanups.
diff --git a/tests/README b/tests/README
index c34bfd8..7037591 100644
--- a/tests/README
+++ b/tests/README
@@ -129,6 +129,13 @@ Do
 
   Use `set -e' to catch failures you might not have thought of.
 
+  End the test script with a `:' or `Exit 0'.  Otherwise, when somebody
+  changes the test by adding a failing command after the last command,
+  the test will spuriously fail because $? is nonzero at the end.
+  Note that this is relevant also for tests using `set -e', if they
+  contain commands like "grep ... Makefile.in && Exit 1" (and there
+  are indeed a lot of such tests).
+
   Use $ACLOCAL, $AUTOMAKE, $AUTOCONF, $AUTOUPDATE, $AUTOHEADER,
   $PERL, $MAKE, $EGREP, and $FGREP, instead of the corresponding
   commands.
-- 
1.6.5