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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 30, 2016, 08:25 UTC
Message-ID: <alpine.DEB.2.20.1606301019040.12947@virtualbox>
In-Reply-To: <xmqq8txn4zm5.fsf@gitster.mtv.corp.google.com>

Hi Junio,

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

Show 13 quoted lines

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

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.

Show 12 quoted lines

> > 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"?

Show 12 quoted lines

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

Show 7 quoted lines

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