Subject: Re: [PATCH] t7510: Skip all if GPG isn't installed

From: Junio C Hamano <[email-masked]>
Date: Wed, 25 Jun 2014 14:16:55 -0700

Brian Gernhardt <[email-masked]> writes:

> Since the setup requires the GPG prerequisite, it doesn't make much
> sense to try and run any tests without it.  So rather than using a
> prereq on each individual test and possibly forgetting it on new ones
> (as just happened), skip the entire file if GPG isn't found.
>
> Signed-off-by: Brian Gernhardt <[email-masked]>
> ---

I think by "just happend" you mean aa4b78d4 (pretty: avoid reading
past end-of-string with "%G", 2014-06-16), which adds one that is
not protected (Cc'ed peff).

As there are a few additional test pieces to this file in flight
that come from another topic (which by the way protects them with
the prerequiste), I'd rather fix it up with the necessary GPG
prerequisite, at least for now, instead of doing it this way.

After the dust settles, we should definitely consider taking the
approach of this patch to simplify everything, but not now.

Another thing we may want to take into account is that we would also
want to make sure that builds of Git without GPG installed still
behave sensibly (with some definition of sensible) when faced with
GPG signatures in existing commit objects and tag objects.  I do not
think we currently test that combination at all, but we may want to
introduce a new directory t/t7510/ to hold store pre-existing commit
objects in the loose form (or in the textual form, suitable for
fast-import) and use them to populate the test repository in the
set-up step.  And new test pieces that do not require GPG (or those
that do require that GPG is *not* installed) would make sure that
various commands like "show --show-signature", "verify-commit" would
say "I cannot verify them" but still do what they are asked to do in
a sensible way (e.g. "show --show-signature" may not be able to show
the signature obviously but still will give you the header, the log
message and the patch; "verify-commit" should fail because it cannot
verify).  If that will happen in this same script, then skipping all
by requiring GPG upfront may not be a good change, but it is likely
that we would want a NOGPG prerequisite for "No GPG installed" case
and have a separate test script, in which case, this will just skip
all without GPG, and the other new one will just skip all without
NOGPG.  We'll see.

>  t/t7510-signed-commit.sh | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
> index 9810242..414f9d1 100755
> --- a/t/t7510-signed-commit.sh
> +++ b/t/t7510-signed-commit.sh
> @@ -4,7 +4,13 @@ test_description='signed commit tests'
>  . ./test-lib.sh
>  . "$TEST_DIRECTORY/lib-gpg.sh"
>  
> -test_expect_success GPG 'create signed commits' '
> +if ! test_have_prereq GPG
> +then
> +	skip_all='skipping signed commit tests; gpg not available'
> +	test_done
> +fi
> +
> +test_expect_success 'create signed commits' '
>  	test_when_finished "test_unconfig commit.gpgsign" &&
>  
>  	echo 1 >file && git add file &&
> @@ -48,7 +54,7 @@ test_expect_success GPG 'create signed commits' '
>  	git tag eighth-signed-alt
>  '
>  
> -test_expect_success GPG 'show signatures' '
> +test_expect_success 'show signatures' '
>  	(
>  		for commit in initial second merge fourth-signed fifth-signed sixth-signed seventh-signed
>  		do
> @@ -79,7 +85,7 @@ test_expect_success GPG 'show signatures' '
>  	)
>  '
>  
> -test_expect_success GPG 'detect fudged signature' '
> +test_expect_success 'detect fudged signature' '
>  	git cat-file commit seventh-signed >raw &&
>  
>  	sed -e "s/seventh/7th forged/" raw >forged1 &&
> @@ -89,7 +95,7 @@ test_expect_success GPG 'detect fudged signature' '
>  	! grep "Good signature from" actual1
>  '
>  
> -test_expect_success GPG 'detect fudged signature with NUL' '
> +test_expect_success 'detect fudged signature with NUL' '
>  	git cat-file commit seventh-signed >raw &&
>  	cat raw >forged2 &&
>  	echo Qwik | tr "Q" "\000" >>forged2 &&
> @@ -99,7 +105,7 @@ test_expect_success GPG 'detect fudged signature with NUL' '
>  	! grep "Good signature from" actual2
>  '
>  
> -test_expect_success GPG 'amending already signed commit' '
> +test_expect_success 'amending already signed commit' '
>  	git checkout fourth-signed^0 &&
>  	git commit --amend -S --no-edit &&
>  	git show -s --show-signature HEAD >actual &&
> @@ -107,7 +113,7 @@ test_expect_success GPG 'amending already signed commit' '
>  	! grep "BAD signature from" actual
>  '
>  
> -test_expect_success GPG 'show good signature with custom format' '
> +test_expect_success 'show good signature with custom format' '
>  	cat >expect <<-\EOF &&
>  	G
>  	13B6F51ECDDE430D
> @@ -117,7 +123,7 @@ test_expect_success GPG 'show good signature with custom format' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_success GPG 'show bad signature with custom format' '
> +test_expect_success 'show bad signature with custom format' '
>  	cat >expect <<-\EOF &&
>  	B
>  	13B6F51ECDDE430D
> @@ -127,7 +133,7 @@ test_expect_success GPG 'show bad signature with custom format' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_success GPG 'show unknown signature with custom format' '
> +test_expect_success 'show unknown signature with custom format' '
>  	cat >expect <<-\EOF &&
>  	U
>  	61092E85B7227189
> @@ -137,7 +143,7 @@ test_expect_success GPG 'show unknown signature with custom format' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_success GPG 'show lack of signature with custom format' '
> +test_expect_success 'show lack of signature with custom format' '
>  	cat >expect <<-\EOF &&
>  	N