[v2,5/5] tests: increase the verbosity of the GPG-related prereqs
diff mbox series

Message ID 5e89b512513af0e2e16dc93c86ae3d1145061a82.1585114881.git.gitgitgadget@gmail.com
State New
Headers show
Series
  • Enable GPG in the Windows part of the CI/PR builds
Related show

Commit Message

starlord via GitGitGadget March 25, 2020, 5:41 a.m. UTC
From: Johannes Schindelin <johannes.schindelin@gmx.de>

Especially when debugging a test failure that can only be reproduced in
the CI build (e.g. when the developer has no access to a macOS machine
other than running the tests on a macOS build agent), output should not
be suppressed.

In the instance of `hi/gpg-prefer-check-signature`, where one
GPG-related test failed for no apparent reason, the entire output of
`gpg` and `gpgsm` was suppressed, even in verbose mode, leaving
interested readers no clue what was going wrong.

Let's fix this by no longer redirecting the output not to `/dev/null`.
This is now possible because the affected prereqs were turned into lazy
ones (and are therefore evaluated via `test_eval_` which respects the
`--verbose` option).

Note that we _still_ redirect `stdout` to `/dev/null` for those commands
that sign their `stdin`, as the output would be binary (and useless
anyway, because the reader would not have anything against which to
compare the output).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/lib-gpg.sh | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

Comments

Jeff King March 26, 2020, 8:50 a.m. UTC | #1
On Wed, Mar 25, 2020 at 05:41:21AM +0000, Johannes Schindelin via GitGitGadget wrote:

> Let's fix this by no longer redirecting the output not to `/dev/null`.
> This is now possible because the affected prereqs were turned into lazy
> ones (and are therefore evaluated via `test_eval_` which respects the
> `--verbose` option).
> 
> Note that we _still_ redirect `stdout` to `/dev/null` for those commands
> that sign their `stdin`, as the output would be binary (and useless
> anyway, because the reader would not have anything against which to
> compare the output).

This looks good. You could drop the redirection of stdin in one case,
too (since test_eval_ already does so) but I don't mind leaving it as
documentation.

-Peff
Johannes Schindelin March 26, 2020, 2:36 p.m. UTC | #2
Hi Peff,

On Thu, 26 Mar 2020, Jeff King wrote:

> On Wed, Mar 25, 2020 at 05:41:21AM +0000, Johannes Schindelin via GitGitGadget wrote:
>
> > Let's fix this by no longer redirecting the output not to `/dev/null`.
> > This is now possible because the affected prereqs were turned into lazy
> > ones (and are therefore evaluated via `test_eval_` which respects the
> > `--verbose` option).
> >
> > Note that we _still_ redirect `stdout` to `/dev/null` for those commands
> > that sign their `stdin`, as the output would be binary (and useless
> > anyway, because the reader would not have anything against which to
> > compare the output).
>
> This looks good. You could drop the redirection of stdin in one case,
> too (since test_eval_ already does so) but I don't mind leaving it as
> documentation.

I considered that, but decided that I'd rather save myself the brain
cycles in the future when reading that code (I would ask myself "*what* is
signed here?").

That's why I left the `</dev/null` in.

Also, I could imagine that there might be some unexpected interaction with
`GIT_DEBUGGER` if I removed that `</dev/null`, and I'd just like to stay
on the safe side here.

Ciao,
Dscho

Patch
diff mbox series

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 7a78c562e8d..9fc5241228e 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -40,12 +40,12 @@  test_lazy_prereq GPG '
 		#		> lib-gpg/ownertrust
 		mkdir "$GNUPGHOME" &&
 		chmod 0700 "$GNUPGHOME" &&
-		(gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) &&
-		gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \
+		(gpgconf --kill gpg-agent || : ) &&
+		gpg --homedir "${GNUPGHOME}" --import \
 			"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
-		gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \
+		gpg --homedir "${GNUPGHOME}" --import-ownertrust \
 			"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
-		gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null 2>&1 \
+		gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null \
 			--sign -u committer@example.com
 		;;
 	esac
@@ -68,23 +68,23 @@  test_lazy_prereq GPGSM '
 	#	gpgsm --homedir /tmp/gpghome/ \
 	#		-o t/lib-gpg/gpgsm_cert.p12 \
 	#		--export-secret-key-p12 "committer@example.com"
-       echo | gpgsm --homedir "${GNUPGHOME}" 2>/dev/null \
-	       --passphrase-fd 0 --pinentry-mode loopback \
-	       --import "$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 &&
+	echo | gpgsm --homedir "${GNUPGHOME}" \
+		--passphrase-fd 0 --pinentry-mode loopback \
+		--import "$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 &&
 
-       gpgsm --homedir "${GNUPGHOME}" 2>/dev/null -K |
-       grep fingerprint: |
-       cut -d" " -f4 |
+	gpgsm --homedir "${GNUPGHOME}" -K |
+	grep fingerprint: |
+	cut -d" " -f4 |
 	tr -d "\\n" >"${GNUPGHOME}/trustlist.txt" &&
 
-       echo " S relax" >>"${GNUPGHOME}/trustlist.txt" &&
-       echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \
-	       -u committer@example.com -o /dev/null --sign - 2>&1
+	echo " S relax" >>"${GNUPGHOME}/trustlist.txt" &&
+	echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \
+	       -u committer@example.com -o /dev/null --sign -
 '
 
 test_lazy_prereq RFC1991 '
 	test_have_prereq GPG &&
-	echo | gpg --homedir "${GNUPGHOME}" -b --rfc1991 >/dev/null 2>&1
+	echo | gpg --homedir "${GNUPGHOME}" -b --rfc1991 >/dev/null
 '
 
 sanitize_pgp() {