Re: Add test coverage for deleted header files.

[Ralf Wildenhues]

Hi Stefano,

* Stefano Lattarini wrote on Sun, Jan 09, 2011 at 02:28:28PM CET:
> On Sunday 09 January 2011, Ralf Wildenhues wrote:
> > I noticed that we don't even have testsuite coverage for the deleted
> > header file bug yet, except for an accidental one.  So here's a patch
> > to address that.
> > 
> > I think it's safe enough for maint; but anyway I will wait a bit for
> > comments or review before pushing.
> >
> See my comments inline below.

Thanks for the quick review.  Guess to make up for that I better review
a few more of your patches ... ;-)

> > By the way, if you would like me to send patches in 'git am'-suitable
> > format, e.g., for testing them, please speak up, I'm all for making
> > things equal for all if that helps (I just need to reorder my workflow
> > a bit then).
> >
> Considering that most patches don't need to be tested by the reviewer,
> I'd say you should use 'git am'-suitable format only if that's no more
> work for you, or for patches you think should be applied and tested
> during the review process.

OK, good point.

> > --- a/tests/depcomp6.test
> > +++ b/tests/depcomp6.test

> >  # check that dependency tracking works
> > -if grep 'depmode=none' Makefile; then :
> > +if grep 'depmode=none' Makefile; then
> > +  Exit 77
> >  else
> At this point, I'd rather remove the "if" altogether:
>   
> grep 'depmode=none' Makefile && Exit 77

ACK.

> > --- a/tests/depcomp7.test
> > +++ b/tests/depcomp7.test

> >    # check that dependency tracking works
> > -  if grep 'depmode=none' Makefile; then :
> > +  if grep 'depmode=none' Makefile; then
> > +    st=77
> >    else
> >      cd sub2
> >      $sleep

> > @@ -114,4 +122,4 @@ for staticshared in --disable-shared "" 
> > --disable-static; do
> >  
> >  done
> >  
> > -:
> > +Exit $st
> > 
> > 
> Hmmm... I really dislike having a testcase being marked as "skipped" when
> it might indeed have run most of its checks.

Agreed.  But it really hasn't run most of its checks in this case, only
the mere minimal set.

> BUT looking more carefully, this shouldn't be the case here, because if
> I'm not mistaken the 'depmode' value should be the same in all the three
> configure runs.  Maybe it would be nice to be more explicit about this
> expectation; WDYT?

Sure, why not.  Also, let's use ANSI C prototypes and GNU style spacing.
I'm folding in the diff below (diff -w output to hide reindenting), and
pushing to maint.

Thanks,
Ralf

diff --git a/ChangeLog b/ChangeLog
index c819ea6..0e2c990 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -4,7 +4,7 @@
        * tests/depcomp6.test, tests/depcomp7.test: Update tests to
        also check for the deleted header bug.  If no dependency
        tracking mechanism could be found, SKIP rather than exit
-       successfully.
+       successfully.  Use GNU style spacing and ANSI C prototypes.
 
        Fix typos in Rule.pm comments.
        * lib/Automake/Rule.pm: Fix typos in comments.
diff --git a/tests/depcomp6.test b/tests/depcomp6.test
index 5713e7e..ea15e20 100755
--- a/tests/depcomp6.test
+++ b/tests/depcomp6.test
@@ -47,7 +47,7 @@ cat >foo.c <<'END'
 #include "foo.h"
 #include "sub2/baz.h"
 #include <stdlib.h>
-int main() { printf("foo"); return bar() + baz(); }
+int main (void) { printf ("foo"); return bar () + baz (); }
 END
 
 cat >foo.h <<'END'
@@ -57,28 +57,28 @@ END
 
 cat >sub/bar.c <<'END'
 #include "sub/bar.h"
-int bar() { return 0; }
+int bar (void) { return 0; }
 END
 
 touch sub2/sub3/ba3.h
 
 cat >sub/bar.h <<'END'
 #include <stdio.h>
-extern int bar();
+extern int bar (void);
 END
 
 cat >sub2/baz.c <<'END'
 #include "baz.h"
-int baz() { return 0; }
+int baz (void) { return 0; }
 END
 
 cat >sub2/baz.h <<'END'
-extern int baz();
+extern int baz (void);
 END
 
 cat >sub2/sub3/ba3.c <<'END'
 #include "ba3.h"
-int ba3() { return 0; }
+int ba3 (void) { return 0; }
 END
 
 $ACLOCAL
@@ -88,10 +88,11 @@ $AUTOMAKE -a
 ./configure --enable-dependency-tracking
 $MAKE
 
-# check that dependency tracking works
+# Check that dependency tracking works.
 if grep 'depmode=none' Makefile; then
   Exit 77
-else
+fi
+
   cd sub2
   $sleep
   echo 'choke me' > sub3/ba3.h
@@ -106,6 +107,5 @@ else
   sed 1d sub3/ba3.c >sub3/ba3.t
   mv -f sub3/ba3.t sub3/ba3.c
   $MAKE
-fi
 
 :
diff --git a/tests/depcomp7.test b/tests/depcomp7.test
index 3edbd71..47f09fc 100755
--- a/tests/depcomp7.test
+++ b/tests/depcomp7.test
@@ -48,7 +48,7 @@ cat >foo.c <<'END'
 #include "foo.h"
 #include "sub2/baz.h"
 #include <stdlib.h>
-int main() { printf("foo"); return bar() + baz(); }
+int main (void) { printf ("foo"); return bar () + baz (); }
 END
 
 cat >foo.h <<'END'
@@ -58,28 +58,28 @@ END
 
 cat >sub/bar.c <<'END'
 #include "sub/bar.h"
-int bar() { return 0; }
+int bar (void) { return 0; }
 END
 
 echo 'extern int x;' > sub2/sub3/ba3.h
 
 cat >sub/bar.h <<'END'
 #include <stdio.h>
-extern int bar();
+extern int bar (void);
 END
 
 cat >sub2/baz.c <<'END'
 #include "baz.h"
-int baz() { return 0; }
+int baz (void) { return 0; }
 END
 
 cat >sub2/baz.h <<'END'
-extern int baz();
+extern int baz (void);
 END
 
 cat >sub2/sub3/ba3.in <<'END'
 #include "ba3.h"
-int ba3() { return 0; }
+int ba3 (void) { return 0; }
 END
 
 libtoolize
@@ -94,7 +94,9 @@ for staticshared in --disable-shared "" --disable-static; do
   ./configure --enable-dependency-tracking $staticshared
   $MAKE
 
-  # check that dependency tracking works
+  # If we cannot enable dependency tracking, perform only the most basic
+  # checks, and don't consider this test to be PASSed but SKIPped, because
+  # the main purpose of this test is exposing the depmode features.
   if grep 'depmode=none' Makefile; then
     st=77
   else