git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option
Date: Thu, 30 Jun 2016 10:25:30 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.20.1606301019040.12947@virtualbox> (raw)
In-Reply-To: <xmqq8txn4zm5.fsf@gitster.mtv.corp.google.com>

Hi Junio,

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

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

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

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

  reply	other threads:[~2016-06-30  8:25 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-29 14:30 [PATCH 0/2] Late edits to the rebase -i tests Johannes Schindelin
2016-06-29 14:31 ` [PATCH 1/2] t3404: fix another typo Johannes Schindelin
2016-06-29 15:11   ` Pranit Bauva
2016-06-29 15:26   ` Junio C Hamano
2016-06-29 15:31     ` Johannes Schindelin
2016-06-29 18:06       ` Junio C Hamano
2016-06-30  8:17         ` Johannes Schindelin
2016-06-29 14:31 ` [PATCH 2/2] t3404: add a test for the --gpg-sign option Johannes Schindelin
2016-06-29 18:30   ` Junio C Hamano
2016-06-30  8:25     ` Johannes Schindelin [this message]
2016-07-01 16:06       ` Junio C Hamano
2016-07-02  7:38         ` Johannes Schindelin
2016-07-06 14:51           ` Junio C Hamano
2016-07-06 15:34             ` Johannes Schindelin
2016-07-06 19:02               ` Junio C Hamano
2016-07-07 15:23                 ` Johannes Schindelin
2016-07-07 15:52 ` [PATCH v2 0/3] Additional rebase -i tests Johannes Schindelin
2016-07-07 15:52   ` [PATCH v2 1/3] t3404: add a test for the --gpg-sign option Johannes Schindelin
2016-07-07 15:52   ` [PATCH v2 2/3] rebase -i: demonstrate a bug with --autosquash Johannes Schindelin
2016-07-07 15:52   ` [PATCH v2 3/3] rebase -i: we allow extra spaces after fixup!/squash! Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.20.1606301019040.12947@virtualbox \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).