[PATCH 1/3] tests/: Do not use `sed -i`

[PATCH 1/3] tests/: Do not use `sed -i`

[cgit]

"-i" isn't part of the POSIX standard and doesn't work on several
platforms such as OpenBSD. Use a temporary file instead.

Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
---
 tests/t0010-validate-html.sh  | 3 +--
 tests/t0020-validate-cache.sh | 9 ++++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/tests/t0010-validate-html.sh b/tests/t0010-validate-html.sh
index 3fe4800..a27c7bb 100755
--- a/tests/t0010-validate-html.sh
+++ b/tests/t0010-validate-html.sh
@@ -7,8 +7,7 @@ test_url()
 {
 	tidy_opt="-eq"
 	test -z "$NO_TIDY_WARNINGS" || tidy_opt+=" --show-warnings no"
-	cgit_url "$1" >trash/tidy-$test_count || return
-	sed -ie "1,4d" trash/tidy-$test_count || return
+	cgit_url "$1" | sed -e "1,4d" trash/tidy-$test_count || return
 	"$tidy" $tidy_opt trash/tidy-$test_count
 	rc=$?
 
diff --git a/tests/t0020-validate-cache.sh b/tests/t0020-validate-cache.sh
index 53ec2eb..19db93d 100755
--- a/tests/t0020-validate-cache.sh
+++ b/tests/t0020-validate-cache.sh
@@ -7,7 +7,8 @@ prepare_tests 'Validate cache'
 run_test 'verify cache-size=0' '
 
 	rm -f trash/cache/* &&
-	sed -i -e "s/cache-size=1021$/cache-size=0/" trash/cgitrc &&
+	sed -e "s/cache-size=1021$/cache-size=0/" trash/cgitrc >trash/cgitrc.tmp &&
+	mv trash/cgitrc.tmp trash/cgitrc &&
 	cgit_url "" &&
 	cgit_url "foo" &&
 	cgit_url "foo/refs" &&
@@ -27,7 +28,8 @@ run_test 'verify cache-size=0' '
 run_test 'verify cache-size=1' '
 
 	rm -f trash/cache/* &&
-	sed -i -e "s/cache-size=0$/cache-size=1/" trash/cgitrc &&
+	sed -e "s/cache-size=0$/cache-size=1/" trash/cgitrc >trash/cgitrc.tmp &&
+	mv trash/cgitrc.tmp trash/cgitrc &&
 	cgit_url "" &&
 	cgit_url "foo" &&
 	cgit_url "foo/refs" &&
@@ -47,7 +49,8 @@ run_test 'verify cache-size=1' '
 run_test 'verify cache-size=1021' '
 
 	rm -f trash/cache/* &&
-	sed -i -e "s/cache-size=1$/cache-size=1021/" trash/cgitrc &&
+	sed -e "s/cache-size=1$/cache-size=1021/" trash/cgitrc >trash/cgitrc.tmp &&
+	mv trash/cgitrc.tmp trash/cgitrc &&
 	cgit_url "" &&
 	cgit_url "foo" &&
 	cgit_url "foo/refs" &&
-- 
1.8.2.675.gda3bb24.dirty

[PATCH 2/3] t0107: Skip ZIP tests if unzip(1) isn't available

[cgit]

Skip tests using unzip(1) if the binary isn't available instead of
erroring out.

Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
---
 tests/t0107-snapshot.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/t0107-snapshot.sh b/tests/t0107-snapshot.sh
index 132d2e9..14ea62d 100755
--- a/tests/t0107-snapshot.sh
+++ b/tests/t0107-snapshot.sh
@@ -55,6 +55,13 @@ run_test 'strip off the header lines (zip)' '
 	tail -n +6 trash/tmp >trash/master.zip
 '
 
+unzip=`which unzip`
+test -n "$unzip" || {
+	echo "Skipping tests: unzip not found"
+	tests_done
+	exit
+}
+
 run_test 'verify zip format' '
 	unzip -t trash/master.zip
 '
-- 
1.8.2.675.gda3bb24.dirty

[PATCH 3/3] t0107: Use `tar -z` for gzip'ed archives

[cgit]

Some tar(1) versions do not support auto detection of the compression
type. Explicitly specify "-z" to decompress a ".tar.gz" archive.

Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
---
 tests/t0107-snapshot.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/t0107-snapshot.sh b/tests/t0107-snapshot.sh
index 14ea62d..2a472ac 100755
--- a/tests/t0107-snapshot.sh
+++ b/tests/t0107-snapshot.sh
@@ -26,7 +26,7 @@ run_test 'verify gzip format' '
 
 run_test 'untar' '
 	rm -rf trash/master &&
-	tar -xf trash/master.tar.gz -C trash
+	tar -xzf trash/master.tar.gz -C trash
 '
 
 run_test 'count files' '
-- 
1.8.2.675.gda3bb24.dirty

[PATCH 1/3] tests/: Do not use `sed -i`

[john]

On Mon, Apr 08, 2013 at 08:18:18PM +0200, Lukas Fleischer wrote:
> "-i" isn't part of the POSIX standard and doesn't work on several
> platforms such as OpenBSD. Use a temporary file instead.
> 
> Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
> ---
>  tests/t0010-validate-html.sh  | 3 +--
>  tests/t0020-validate-cache.sh | 9 ++++++---
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/t0010-validate-html.sh b/tests/t0010-validate-html.sh
> index 3fe4800..a27c7bb 100755
> --- a/tests/t0010-validate-html.sh
> +++ b/tests/t0010-validate-html.sh
> @@ -7,8 +7,7 @@ test_url()
>  {
>  	tidy_opt="-eq"
>  	test -z "$NO_TIDY_WARNINGS" || tidy_opt+=" --show-warnings no"
> -	cgit_url "$1" >trash/tidy-$test_count || return
> -	sed -ie "1,4d" trash/tidy-$test_count || return
> +	cgit_url "$1" | sed -e "1,4d" trash/tidy-$test_count || return

Please don't pipe the output like this, it loses the exit code from
cgit_url (and hence cgit) and may cause us to miss crashes.

The rest of this patch looks sensible - I'd probably use a suffix of "+"
rather than ".tmp", but that's just taste. ;-)

>  	"$tidy" $tidy_opt trash/tidy-$test_count
>  	rc=$?
>  
> diff --git a/tests/t0020-validate-cache.sh b/tests/t0020-validate-cache.sh
> index 53ec2eb..19db93d 100755
> --- a/tests/t0020-validate-cache.sh
> +++ b/tests/t0020-validate-cache.sh
> @@ -7,7 +7,8 @@ prepare_tests 'Validate cache'
>  run_test 'verify cache-size=0' '
>  
>  	rm -f trash/cache/* &&
> -	sed -i -e "s/cache-size=1021$/cache-size=0/" trash/cgitrc &&
> +	sed -e "s/cache-size=1021$/cache-size=0/" trash/cgitrc >trash/cgitrc.tmp &&
> +	mv trash/cgitrc.tmp trash/cgitrc &&
>  	cgit_url "" &&
>  	cgit_url "foo" &&
>  	cgit_url "foo/refs" &&
> @@ -27,7 +28,8 @@ run_test 'verify cache-size=0' '
>  run_test 'verify cache-size=1' '
>  
>  	rm -f trash/cache/* &&
> -	sed -i -e "s/cache-size=0$/cache-size=1/" trash/cgitrc &&
> +	sed -e "s/cache-size=0$/cache-size=1/" trash/cgitrc >trash/cgitrc.tmp &&
> +	mv trash/cgitrc.tmp trash/cgitrc &&
>  	cgit_url "" &&
>  	cgit_url "foo" &&
>  	cgit_url "foo/refs" &&
> @@ -47,7 +49,8 @@ run_test 'verify cache-size=1' '
>  run_test 'verify cache-size=1021' '
>  
>  	rm -f trash/cache/* &&
> -	sed -i -e "s/cache-size=1$/cache-size=1021/" trash/cgitrc &&
> +	sed -e "s/cache-size=1$/cache-size=1021/" trash/cgitrc >trash/cgitrc.tmp &&
> +	mv trash/cgitrc.tmp trash/cgitrc &&
>  	cgit_url "" &&
>  	cgit_url "foo" &&
>  	cgit_url "foo/refs" &&
> -- 
> 1.8.2.675.gda3bb24.dirty

[PATCH 2/3] t0107: Skip ZIP tests if unzip(1) isn't available

[john]

On Mon, Apr 08, 2013 at 08:18:19PM +0200, Lukas Fleischer wrote:
> Skip tests using unzip(1) if the binary isn't available instead of
> erroring out.
> 
> Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
> ---
>  tests/t0107-snapshot.sh | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tests/t0107-snapshot.sh b/tests/t0107-snapshot.sh
> index 132d2e9..14ea62d 100755
> --- a/tests/t0107-snapshot.sh
> +++ b/tests/t0107-snapshot.sh
> @@ -55,6 +55,13 @@ run_test 'strip off the header lines (zip)' '
>  	tail -n +6 trash/tmp >trash/master.zip
>  '
>  
> +unzip=`which unzip`
> +test -n "$unzip" || {
> +	echo "Skipping tests: unzip not found"
> +	tests_done

Is this based on the jk/use-git-test-suite?  If so can we use skip_all
and test_done instead of tests_done?

I have a WIP series that does the global replacements of
s/tests_done/test_done/ and s/run_test/test_expect_success/.

> +	exit
> +}
> +
>  run_test 'verify zip format' '
>  	unzip -t trash/master.zip
>  '
> -- 
> 1.8.2.675.gda3bb24.dirty

[PATCH 2/3] t0107: Skip ZIP tests if unzip(1) isn't available

[Jason]

On Mon, Apr 8, 2013 at 8:26 PM, John Keeping <john at keeping.me.uk> wrote:
> Is this based on the jk/use-git-test-suite?  If so can we use skip_all
> and test_done instead of tests_done?

Indeed it would be nice to stack up all the test patches against
jk/use-git-test-suite.


> I have a WIP series that does the global replacements of
> s/tests_done/test_done/ and s/run_test/test_expect_success/.

Looking forward.

[PATCH 2/3] t0107: Skip ZIP tests if unzip(1) isn't available

[cgit]

On Mon, Apr 08, 2013 at 07:26:32PM +0100, John Keeping wrote:
> On Mon, Apr 08, 2013 at 08:18:19PM +0200, Lukas Fleischer wrote:
> > Skip tests using unzip(1) if the binary isn't available instead of
> > erroring out.
> > 
> > Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
> > ---
> >  tests/t0107-snapshot.sh | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/tests/t0107-snapshot.sh b/tests/t0107-snapshot.sh
> > index 132d2e9..14ea62d 100755
> > --- a/tests/t0107-snapshot.sh
> > +++ b/tests/t0107-snapshot.sh
> > @@ -55,6 +55,13 @@ run_test 'strip off the header lines (zip)' '
> >  	tail -n +6 trash/tmp >trash/master.zip
> >  '
> >  
> > +unzip=`which unzip`
> > +test -n "$unzip" || {
> > +	echo "Skipping tests: unzip not found"
> > +	tests_done
> 
> Is this based on the jk/use-git-test-suite?  If so can we use skip_all
> and test_done instead of tests_done?
> 
> I have a WIP series that does the global replacements of
> s/tests_done/test_done/ and s/run_test/test_expect_success/.

It would be great if you could submit that patch and merge into
jk/use-git-test-suite first, so that we won't get a conflict...

> 
> > +	exit
> > +}
> > +
> >  run_test 'verify zip format' '
> >  	unzip -t trash/master.zip
> >  '
> > -- 
> > 1.8.2.675.gda3bb24.dirty

[PATCH 2/3] t0107: Skip ZIP tests if unzip(1) isn't available

[john]

On Mon, Apr 08, 2013 at 08:47:14PM +0200, Lukas Fleischer wrote:
> On Mon, Apr 08, 2013 at 07:26:32PM +0100, John Keeping wrote:
> > On Mon, Apr 08, 2013 at 08:18:19PM +0200, Lukas Fleischer wrote:
> > > Skip tests using unzip(1) if the binary isn't available instead of
> > > erroring out.
> > > 
> > > Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
> > > ---
> > >  tests/t0107-snapshot.sh | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/tests/t0107-snapshot.sh b/tests/t0107-snapshot.sh
> > > index 132d2e9..14ea62d 100755
> > > --- a/tests/t0107-snapshot.sh
> > > +++ b/tests/t0107-snapshot.sh
> > > @@ -55,6 +55,13 @@ run_test 'strip off the header lines (zip)' '
> > >  	tail -n +6 trash/tmp >trash/master.zip
> > >  '
> > >  
> > > +unzip=`which unzip`
> > > +test -n "$unzip" || {
> > > +	echo "Skipping tests: unzip not found"
> > > +	tests_done
> > 
> > Is this based on the jk/use-git-test-suite?  If so can we use skip_all
> > and test_done instead of tests_done?
> > 
> > I have a WIP series that does the global replacements of
> > s/tests_done/test_done/ and s/run_test/test_expect_success/.
> 
> It would be great if you could submit that patch and merge into
> jk/use-git-test-suite first, so that we won't get a conflict...

I was hanging onto it to avoid sending lots of incremental patches on
top because I keep thinking of new things to change :-)

But I'll send it out shortly after giving it a quick review.

[PATCH 2/3] t0107: Skip ZIP tests if unzip(1) isn't available

[Jason]

Lukas --

Would you resubmit these latest 3 patches based on the current master?
Quite a bit has just been merged.

Thanks,
Jason

[PATCH 3/3] t0107: Use `tar -z` for gzip'ed archives

[Jason]

Merged, thanks.

[PATCH 2/3] t0107: Skip ZIP tests if unzip(1) isn't available

[Jason]

On Mon, Apr 8, 2013 at 8:26 PM, John Keeping <john at keeping.me.uk> wrote:
> Is this based on the jk/use-git-test-suite?  If so can we use skip_all
> and test_done instead of tests_done?

Actually, skip_all isn't allowed in the case where a few tests from
that file have already run.

[PATCH 2/3] t0107: Skip ZIP tests if unzip(1) isn't available

[john]

On Tue, Apr 09, 2013 at 12:19:16AM +0200, Jason A. Donenfeld wrote:
> On Mon, Apr 8, 2013 at 8:26 PM, John Keeping <john at keeping.me.uk> wrote:
> > Is this based on the jk/use-git-test-suite?  If so can we use skip_all
> > and test_done instead of tests_done?
> 
> Actually, skip_all isn't allowed in the case where a few tests from
> that file have already run.

I hadn't spotted that.  Since we're part way through a test suite, we
should be using test_set_prereq here (like Git does for GUNZIP in
t5000-tar-tree.sh).