[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] test pread (using the new init.sh framework)
From: |
Jim Meyering |
Subject: |
Re: [PATCH 2/2] test pread (using the new init.sh framework) |
Date: |
Wed, 25 Nov 2009 17:01:32 +0100 |
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