Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 29, 2016, 18:30 UTC
Message-ID: <xmqq8txn4zm5.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <0df34c45db5b1500e55262c8948c9140e7ad6cb8.1467210629.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> To keep the time t3404 requires short (in this developer's Windows
> setup, this single test already takes a painful 8 minutes to pass),
> we avoid a full-blown GPG test and cop out by verifying the message
> displayed to the user upon an 'edit' command.

This is a tangent, but I wonder if we should be solving it by parallelizing the tests even more. If the script for example can be split into part1 and part2 that share the same set-up that is prepared by the very first test, we could split this into three files (common one that is dot-sourced by two actual test scripts that have part1 and part2).

Show 15 quoted lines

>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  t/t3404-rebase-interactive.sh | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index c7ea8ba..4c96075 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1281,4 +1281,11 @@ test_expect_success 'editor saves as CR/LF' '
>  	)
>  '
>  
> +EPIPHANY="'"

Why? If you really need a variable, SQ is probably the way this codebase typically spell it.

> +test_expect_success 'rebase -i --gpg-sign=<key-id>' '
> +	set_fake_editor &&
> +	FAKE_LINES="edit 1" git rebase -i --gpg-sign=\" HEAD^ >out 2>err &&
> +	grep "$EPIPHANY-S\"$EPIPHANY" err

I am not sure what is going on here. You are asking to sign using the keyid of a single double-quote, and expecting the message that says

	You can amend the commit now, with

		git commit --amend '-S"'

...

that has a substring '-S"' in it to ensure that the codepath to parse --gpg-sign= on the command line of "rebase", and to the message we see here are working correctly, without actually checking if GPG is invoked at all, or if it is invoked the key given by the option is correctly passed to the invocation?

If so, can't you do that without confusing users by using keyid "? IOW, wouldn't using --gpg-sign=me work equally well? For example:

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 4c96075..2dd3644 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1281,11 +1281,10 @@ test_expect_success 'editor saves as CR/LF' '
 	)
 '
 
-EPIPHANY="'"
 test_expect_success 'rebase -i --gpg-sign=<key-id>' '
 	set_fake_editor &&
-	FAKE_LINES="edit 1" git rebase -i --gpg-sign=\" HEAD^ >out 2>err &&
-	grep "$EPIPHANY-S\"$EPIPHANY" err
+	FAKE_LINES="edit 1" git rebase -i --gpg-sign=me HEAD^ >out 2>err &&
+	grep -e "commit --amend  '\''-Sme'\''" err
 '
 
 test_done