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

Johannes Schindelin [off-list ref] 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).

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

Hi Junio,

On Wed, 29 Jun 2016, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

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).

Sure, that would work around the problem in the short run.

In the long run, the only real solution would be to stop relying on shell
scripting so much, because the biggest performance hit on Windows stems
from the fact that our test suite is a big honking shell script.

quoted

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.

Have you ever watched the movie "Hook"?

quoted

+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"'

Precisely. That way, I not only verified that the key ID was correctly
passed around, but also that it is quoted properly.

	...

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?

Exactly. I want to test --gpg-sign even when there is no gpg executable
available.

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

Not really, because it is much easier to get quoting of "me" right than of
"\"".

I guess I could change the test to pass --gpg-sign="\"S I Gner\"".

Ciao,
Dscho