Re: [PATCH v2 9/9] gpg-interface t: extend the existing GPG tests with GPGSM

From: Jeff King <peff@peff.net>
Date: Jul 10, 2018, 17:09 UTC
Message-ID: <20180710170901.GH23624@sigill.intra.peff.net>
In-Reply-To: <f97d2d79f6a46ddffcd0065239f99b084708e813.1531208187.git.henning.schild@siemens.com>

On Tue, Jul 10, 2018 at 10:52:31AM +0200, Henning Schild wrote:

Show 11 quoted lines

> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> index a5d3b2cba..9dcb4e990 100755
> --- a/t/lib-gpg.sh
> +++ b/t/lib-gpg.sh
> @@ -38,7 +38,14 @@ then
>  			"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
>  		gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null 2>&1 \
>  			--sign -u committer@example.com &&
> -		test_set_prereq GPG
> +		test_set_prereq GPG &&
> +		echo | gpgsm --homedir "${GNUPGHOME}" -o "$TEST_DIRECTORY"/lib-gpg/gpgsm.crt.user --passphrase-fd 0 --pinentry-mode loopback --generate-key --batch "$TEST_DIRECTORY"/lib-gpg/gpgsm-gen-key.in &&

Is this --generate-key going to require high-quality entropy? That could slow the test suite down (and maybe even stall it quite a bit on servers doing automated CI).

Can we save a dummy generated key and just import it? That's what we do for the regular gpg case.

Show 6 quoted lines

> +		gpgsm --homedir "${GNUPGHOME}" --import "$TEST_DIRECTORY"/lib-gpg/gpgsm.crt.user &&
> +		gpgsm --homedir "${GNUPGHOME}" -K | grep fingerprint: | cut -d" " -f4 | tr -d '\n' > ${GNUPGHOME}/trustlist.txt &&
> +		echo " S relax" >> ${GNUPGHOME}/trustlist.txt &&
> +		(gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) &&
> +		echo hello | gpgsm --homedir "${GNUPGHOME}" -u committer@example.com -o /dev/null --sign - 2>&1 &&
> +		test_set_prereq GPGSM

This &&-chain means we can't have GPGSM without GPG. In theory the two could be tested independently. I don't know if it's worth the trouble to make that work, though. I wouldn't be surprised if there are some subtle dependencies within the test scripts, and I'm not sure how common it is for somebody to have gpgsm and not gpg. So it may make sense to just punt on it until such a person appears.

Show 11 quoted lines

>  test_expect_success GPG 'log --graph --show-signature' '
>  	git log --graph --show-signature -n1 signed >actual &&
>  	grep "^| gpg: Signature made" actual &&
>  	grep "^| gpg: Good signature" actual
>  '
>  
> +test_expect_success GPGSM 'log --graph --show-signature x509' '
> +	git log --graph --show-signature -n1 signed-x509 >actual &&
> +	grep "^| gpgsm: Signature made" actual &&
> +	grep "^| gpgsm: Good signature" actual
> +'

We're going to have a lot of duplicated tests here. That's a maintenance burden when one of them needs fixes later. And when new tests are added, we won't automatically get them tested under each format.

Can we move the battery of tests into a function that takes a few parameters (prereq name, branch to look at, etc) and then call it for both the gpg/gpgsm cases?

-Peff