Re: [PATCH 2/2] test pread (using the new init.sh framework)

[Jim Meyering]

Eric Blake wrote:
> Jim Meyering <jim <at> meyering.net> writes:
>
>>     #!/bin/sh
>>     : ${srcdir=.} ${builddir=.}
>
> According to the autoconf manual, $builddir is rigourously equal to '.', from
> within autoconf-generated files (such as Makefile built from Makefile.in).  I
> can see keeping this line so that you can run the script by hand from some
> other directory after the fact...
>
>> +TESTS_ENVIRONMENT +=        \
>> +  srcdir='$(srcdir)'        \
>> +  builddir='$(builddir)'
>
> ...but since builddir is constant within the Makefile, then why do we need to
> bother passing it via TESTS_ENVIRONMENT?  I could, however, see the potential
> use of $(abs_builddir).

init.sh derives abs_builddir from --set-path=$builddir,
so at least for now, it's not needed.  Initially I had
--set-path=. but with $builddir, it's not hard-coded.

The sole advantage of exporting builddir is that then it's
not imperative to provide the default definition.
So, choose: require everyone to run tests via "make check ..."
and remove the convenience definition, thus saving a line in
each of the *.sh scripts?  Or remove one maybe-unnecessary
line in each modules/* file, thus requiring that any

>>  create mode 100644 tests/test-pread.sh
>
> Oops; this needs an executable bit.

Hmm... it worked fine for me, via this:

  ./gnulib-tool --create-testdir --with-tests --test pread

so it's obviously not always required.
That's because of the way the script is invoked
via "make check".  Obviously if you want to invoke
it directly via ./test-pread.sh, we'll want the executable bit to be
set, so I fixed that.

>> +: ${srcdir=.} ${builddir=.}
>> +. $srcdir/init.sh --set-path=$builddir
>> +
>> +fail=0;
>
> Should we hoist fail=0 into init.sh, as was done in coreutils?

I'm not sure.  Just a day or two ago, I nearly
copied a coreutils test into a stand-alone file without
that initialization, so perhaps there *is* benefit to leaving
it in each file and adding a check to ensure it's there.

I noticed one more problem: test-pread.sh was not listed
in the test module's Files list.  All fixed now, I think:

>From 9b1723bc19a2770a3f57e6432347dab683e2f027 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 25 Nov 2009 16:52:47 +0100
Subject: [PATCH 1/2] test-pread.sh: clean up

* tests/test-pread.sh: Don't refer to $builddir. Just use equivalent ".".
* modules/pread-tests (TESTS_ENVIRONMENT): Don't export builddir.
That is unnecessary, since it's always ".".
Suggestion from Eric Blake.
---
 ChangeLog           |    6 ++++++
 modules/pread-tests |    3 +--
 tests/test-pread.sh |    4 ++--
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 04898b0..09a03dd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2009-11-25  Jim Meyering  <address@hidden>

+       test-pread.sh: clean up
+       * tests/test-pread.sh: Don't refer to $builddir. Just use equivalent 
".".
+       * modules/pread-tests (TESTS_ENVIRONMENT): Don't export builddir.
+       That is unnecessary, since it's always ".".
+       Suggestion from Eric Blake.
+
        test-pread.sh: make executable
        * tests/test-pread.sh: Set executable bit.
        Reported by Eric Blake.
diff --git a/modules/pread-tests b/modules/pread-tests
index 7a4e7a7..9d3dec8 100644
--- a/modules/pread-tests
+++ b/modules/pread-tests
@@ -10,5 +10,4 @@ Makefile.am:
 TESTS += test-pread.sh
 check_PROGRAMS += test-pread
 TESTS_ENVIRONMENT +=   \
-  srcdir='$(srcdir)'   \
-  builddir='$(builddir)'
+  srcdir='$(srcdir)'
diff --git a/tests/test-pread.sh b/tests/test-pread.sh
index fba415e..98971a4 100755
--- a/tests/test-pread.sh
+++ b/tests/test-pread.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
-: ${srcdir=.} ${builddir=.}
-. $srcdir/init.sh --set-path=$builddir
+: ${srcdir=.}
+. $srcdir/init.sh --set-path=.

 fail=0;
 test-pread || fail=1
--
1.6.6.rc0.236.ge0b94


>From dbf9a707eb31bf2d28651fa96df510913e3fada0 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 25 Nov 2009 16:59:15 +0100
Subject: [PATCH 2/2] test-pread.sh: distribute the test script

* modules/pread-tests (Files): Include test-pread.sh.
---
 ChangeLog           |    3 +++
 modules/pread-tests |    1 +
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 09a03dd..63af1d7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2009-11-25  Jim Meyering  <address@hidden>

+       test-pread.sh: distribute the test script
+       * modules/pread-tests (Files): Include test-pread.sh.
+
        test-pread.sh: clean up
        * tests/test-pread.sh: Don't refer to $builddir. Just use equivalent 
".".
        * modules/pread-tests (TESTS_ENVIRONMENT): Don't export builddir.
diff --git a/modules/pread-tests b/modules/pread-tests
index 9d3dec8..9b2a94a 100644
--- a/modules/pread-tests
+++ b/modules/pread-tests
@@ -1,5 +1,6 @@
 Files:
 tests/test-pread.c
+tests/test-pread.sh
 tests/init.sh

 Depends-on:
--
1.6.6.rc0.236.ge0b94