[PATCH 0/2] Late edits to the rebase -i tests

[PATCH 0/2] Late edits to the rebase -i tests

[Johannes Schindelin]

I meant to send the first patch much earlier, but got side-tracked
before I could add the --gpg-sign test.

This is just another patch series in preparation for the rebase--helper
(to be precise, this is the seventh out of fourteen patch series that
have not yet been merged to master; Six are already in flight, so I will
stop here until the queue empties a little.)


Johannes Schindelin (2):
  t3404: fix another typo
  t3404: add a test for the --gpg-sign option

 t/t3404-rebase-interactive.sh | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Published-As: https://github.com/dscho/git/releases/tag/rebase-i-tests-v1
-- 
2.9.0.270.g810e421

base-commit: cf4c2cfe52be5bd973a4838f73a35d3959ce2f43

[PATCH 1/2] t3404: fix another typo

[Johannes Schindelin]

The past tense of "to run" is "run", not "ran".

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3404-rebase-interactive.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 66348f1..c7ea8ba 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -60,7 +60,7 @@ test_expect_success 'setup' '
 	test_commit P fileP
 '
 
-# "exec" commands are ran with the user shell by default, but this may
+# "exec" commands are run with the user shell by default, but this may
 # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
 # to create a file. Unsetting SHELL avoids such non-portable behavior
 # in tests. It must be exported for it to take effect where needed.
-- 
2.9.0.270.g810e421

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

[Johannes Schindelin]

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.

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="'"
+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
+'
+
 test_done
-- 
2.9.0.270.g810e421

Re: [PATCH 1/2] t3404: fix another typo

[Pranit Bauva]

Hey Johannes,

Very minor nits.

On Wed, Jun 29, 2016 at 8:01 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> The past tense of "to run" is "run", not "ran".

Past tense of "to run" is "ran" while past participle is "to run".

Past tense: He ran.
Past Participle: He has to run.

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  t/t3404-rebase-interactive.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 66348f1..c7ea8ba 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -60,7 +60,7 @@ test_expect_success 'setup' '
>         test_commit P fileP
>  '
>
> -# "exec" commands are ran with the user shell by default, but this may

This sentence seems to be in present tense if I am not wrong (though I
am not a native English speaker).

> +# "exec" commands are run with the user shell by default, but this may
>  # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
>  # to create a file. Unsetting SHELL avoids such non-portable behavior
>  # in tests. It must be exported for it to take effect where needed.

The change introduces fixed the grammo properly but the commit message
probably reports it incorrectly.

Would it be better if the commit message is without changing anything else:
     The present tense of "to run" is "run", not "ran".

Regards,
Pranit Bauva

Re: [PATCH 1/2] t3404: fix another typo

[Junio C Hamano]

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

> The past tense of "to run" is "run", not "ran".

Actually, past tense of the verb "to run" is "ran" ;-) The reason
why this patch is still correct is because this is writing in
passive voice, where you want "be + past participle", and the past
participle of the verb "to run" is "run".

>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  t/t3404-rebase-interactive.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 66348f1..c7ea8ba 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -60,7 +60,7 @@ test_expect_success 'setup' '
>  	test_commit P fileP
>  '
>  
> -# "exec" commands are ran with the user shell by default, but this may
> +# "exec" commands are run with the user shell by default, but this may
>  # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
>  # to create a file. Unsetting SHELL avoids such non-portable behavior
>  # in tests. It must be exported for it to take effect where needed.

Re: [PATCH 1/2] t3404: fix another typo

[Johannes Schindelin]

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

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > The past tense of "to run" is "run", not "ran".
> 
> Actually, past tense of the verb "to run" is "ran" ;-) The reason
> why this patch is still correct is because this is writing in
> passive voice, where you want "be + past participle", and the past
> participle of the verb "to run" is "run".

Embarrassing! ;-)

Well, I shall fix up the commit message thusly:

    t3404: fix another typo

    The passive form of "to run" is "run", not "ran".

    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Okay?
Dscho

Re: [PATCH 1/2] t3404: fix another typo

[Junio C Hamano]

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

> On Wed, 29 Jun 2016, Junio C Hamano wrote:
>
>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>> 
>> > The past tense of "to run" is "run", not "ran".
>> 
>> Actually, past tense of the verb "to run" is "ran" ;-) The reason
>> why this patch is still correct is because this is writing in
>> passive voice, where you want "be + past participle", and the past
>> participle of the verb "to run" is "run".
>
> Embarrassing! ;-)
>
> Well, I shall fix up the commit message thusly:
>
>     t3404: fix another typo
>
>     The passive form of "to run" is "run", not "ran".

If your convention of referring to a verb is to show its infinitive
form, i.e. "to run", then you would probably want to say

	The passive from of "to run" is "to be run", not "to be
	ran".

But I think we do not need any of this if we just retitled it to

	Subject: t3404: fix a grammo (commands are ran -> commands are run)

without any body.

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

[Junio C Hamano]

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

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

Re: [PATCH 1/2] t3404: fix another typo

[Johannes Schindelin]

Hi Junio,

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

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Wed, 29 Jun 2016, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> >> 
> >> > The past tense of "to run" is "run", not "ran".
> >> 
> >> Actually, past tense of the verb "to run" is "ran" ;-) The reason
> >> why this patch is still correct is because this is writing in
> >> passive voice, where you want "be + past participle", and the past
> >> participle of the verb "to run" is "run".
> >
> > Embarrassing! ;-)
> >
> > Well, I shall fix up the commit message thusly:
> >
> >     t3404: fix another typo
> >
> >     The passive form of "to run" is "run", not "ran".
> 
> If your convention of referring to a verb is to show its infinitive
> form, i.e. "to run", then you would probably want to say
> 
> 	The passive from of "to run" is "to be run", not "to be
> 	ran".
> 
> But I think we do not need any of this if we just retitled it to
> 
> 	Subject: t3404: fix a grammo (commands are ran -> commands are run)
> 
> without any body.

Done. Will be part of v2.

Ciao,
Dscho

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

[Johannes Schindelin]

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

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

[Junio C Hamano]

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

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

The other side of that coine is that even when GPG is available, we
do not see if it is invoked correctly at all.  That was what I found
disturbing.

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

[Johannes Schindelin]

Hi Junio,

On Fri, 1 Jul 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> 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.
> 
> The other side of that coine is that even when GPG is available, we
> do not see if it is invoked correctly at all.  That was what I found
> disturbing.

Okay, I see now.

However, is it not better to have even a limited test than none at all?
Granted, we still would not know whether rebased commits would be signed
properly. But if my trivial test case fails, we would still have a strong
indicator that something is broken, with a very convenient way to debug
it. And that is what a regression test suite is all about, isn't it?

Of course I agree that it would be very nice to have a test at a later
stage that does exercise GPG if it is available. But would it really be so
terrible to have a (simpler, not as complete) test that is exercised
*also* when GPG is *not* available?

Ciao,
Dscho

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

[Junio C Hamano]

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

> Of course I agree that it would be very nice to have a test at a later
> stage that does exercise GPG if it is available. But would it really be so
> terrible to have a (simpler, not as complete) test that is exercised
> *also* when GPG is *not* available?

What I would expect is "In the ideal world, we may want both, and in
an imperfect world in which we can have only one, we'd rather have
the 'even though we can run it only when GPG is available, we make
sure that we drive GPG correctly' one, dropping the other.", simply
because the end result matters more, not how the instruction to the
end user is phrased.

Sure, in even less perfect world, having a superficial test might be
better than nothing, but reminding ourselves to aim high (and make
sure we document the decision when we punt) is an important part of
the purpose of the review process, so...

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

[Johannes Schindelin]

Hi Junio,

On Wed, 6 Jul 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Of course I agree that it would be very nice to have a test at a later
> > stage that does exercise GPG if it is available. But would it really
> > be so terrible to have a (simpler, not as complete) test that is
> > exercised *also* when GPG is *not* available?
> 
> What I would expect is "In the ideal world, we may want both, and in an
> imperfect world in which we can have only one, we'd rather have the
> 'even though we can run it only when GPG is available, we make sure that
> we drive GPG correctly' one, dropping the other.", simply because the
> end result matters more, not how the instruction to the end user is
> phrased.
> 
> Sure, in even less perfect world, having a superficial test might be
> better than nothing, but reminding ourselves to aim high (and make sure
> we document the decision when we punt) is an important part of the
> purpose of the review process, so...

Okay, so here is the deal: on the development machine where this was
developed, I do not have gpg installed. So please take this test case just
to make sure that things work as intended for the moment.

Before sending the last rebase--helper patch series, I will make sure to
add a real test that requires gpg, and submit that, too.

Deal?

Ciao,
Dscho

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

[Junio C Hamano]

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

> Okay, so here is the deal: on the development machine where this was
> developed, I do not have gpg installed. So please take this test case just
> to make sure that things work as intended for the moment.
>
> Before sending the last rebase--helper patch series, I will make sure to
> add a real test that requires gpg, and submit that, too.
>
> Deal?

I do not particularly care if the latter one happens.

The only thing I care about is that the earlier round documents that
we know we probably should test the real driving of the GPG program,
but we deliberately do not do so in the series, and hint that such
an enhancement can happen later.

That might even entice others to help writing a test or two ;-)

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

[Johannes Schindelin]

Hi Junio,

On Wed, 6 Jul 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Okay, so here is the deal: on the development machine where this was
> > developed, I do not have gpg installed. So please take this test case just
> > to make sure that things work as intended for the moment.
> >
> > Before sending the last rebase--helper patch series, I will make sure to
> > add a real test that requires gpg, and submit that, too.
> >
> > Deal?
> 
> I do not particularly care if the latter one happens.
> 
> The only thing I care about is that the earlier round documents that
> we know we probably should test the real driving of the GPG program,
> but we deliberately do not do so in the series, and hint that such
> an enhancement can happen later.
> 
> That might even entice others to help writing a test or two ;-)

Okay. I tried my hand at editing the commit message, and threw two more
tests into the patch series for good measure. Will send out v2 once the
test suite passed (it's still running).

Thanks,
Dscho

[PATCH v2 0/3] Additional rebase -i tests

[Johannes Schindelin]

This is just another patch series in preparation for the rebase--helper.

Relative to v1 of this patch series,

- the grammo fix was backed out because it was picked up separately,

- two new tests were introduced, one demonstrating a bug, the other one
  ensuring that the rebase--helper does not introduce a regression (this
  test actually helped me debug and fix a regression in some previous
  revision of the rebase--helper)


Johannes Schindelin (3):
  t3404: add a test for the --gpg-sign option
  rebase -i: demonstrate a bug with --autosquash
  rebase -i: we allow extra spaces after fixup!/squash!

 t/t3404-rebase-interactive.sh |  8 ++++++++
 t/t3415-rebase-autosquash.sh  | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

Published-As: https://github.com/dscho/git/releases/tag/rebase-i-tests-v2
Interdiff vs v1:

 diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
 index 4c96075..aa393d2 100755
 --- a/t/t3404-rebase-interactive.sh
 +++ b/t/t3404-rebase-interactive.sh
 @@ -60,7 +60,7 @@ test_expect_success 'setup' '
  	test_commit P fileP
  '
  
 -# "exec" commands are run with the user shell by default, but this may
 +# "exec" commands are ran with the user shell by default, but this may
  # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
  # to create a file. Unsetting SHELL avoids such non-portable behavior
  # in tests. It must be exported for it to take effect where needed.
 @@ -1281,11 +1281,12 @@ test_expect_success 'editor saves as CR/LF' '
  	)
  '
  
 -EPIPHANY="'"
 +SQ="'"
  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="\"S I Gner\"" HEAD^ \
 +		>out 2>err &&
 +	grep "$SQ-S\"S I Gner\"$SQ" err
  '
  
  test_done
 diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
 index 8f53e54..48346f1 100755
 --- a/t/t3415-rebase-autosquash.sh
 +++ b/t/t3415-rebase-autosquash.sh
 @@ -271,4 +271,37 @@ test_expect_success 'autosquash with custom inst format' '
  	test 2 = $(git cat-file commit HEAD^ | grep squash | wc -l)
  '
  
 +set_backup_editor () {
 +	write_script backup-editor.sh <<-\EOF
 +	cp "$1" .git/backup-"$(basename "$1")"
 +	EOF
 +	test_set_editor "$PWD/backup-editor.sh"
 +}
 +
 +test_expect_failure 'autosquash with multiple empty patches' '
 +	test_tick &&
 +	git commit --allow-empty -m "empty" &&
 +	test_tick &&
 +	git commit --allow-empty -m "empty2" &&
 +	test_tick &&
 +	>fixup &&
 +	git add fixup &&
 +	git commit --fixup HEAD^^ &&
 +	(
 +		set_backup_editor &&
 +		GIT_USE_REBASE_HELPER=false \
 +		git rebase -i --force-rebase --autosquash HEAD~4 &&
 +		grep empty2 .git/backup-git-rebase-todo
 +	)
 +'
 +
 +test_expect_success 'extra spaces after fixup!' '
 +	base=$(git rev-parse HEAD) &&
 +	test_commit to-fixup &&
 +	git commit --allow-empty -m "fixup!  to-fixup" &&
 +	git rebase -i --autosquash --keep-empty HEAD~2 &&
 +	parent=$(git rev-parse HEAD^) &&
 +	test $base = $parent
 +'
 +
  test_done

-- 
2.9.0.278.g1caae67

base-commit: 5c589a73de4394ad125a4effac227b3aec856fa1

[PATCH v2 1/3] t3404: add a test for the --gpg-sign option

[Johannes Schindelin]

For the upcoming rebase--helper work (which will accelerate the
interactive rebase noticably), it is important to verify that the
--gpg-sign option is handled properly.

Please note that this patch does this on the cheap, by verifying that
the expected option is printed in the message of the 'edit' operation.

We really should test that the interactive rebase signs the commits
properly, iff GPG is available. This test is left for later.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3404-rebase-interactive.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 66348f1..aa393d2 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1281,4 +1281,12 @@ test_expect_success 'editor saves as CR/LF' '
 	)
 '
 
+SQ="'"
+test_expect_success 'rebase -i --gpg-sign=<key-id>' '
+	set_fake_editor &&
+	FAKE_LINES="edit 1" git rebase -i --gpg-sign="\"S I Gner\"" HEAD^ \
+		>out 2>err &&
+	grep "$SQ-S\"S I Gner\"$SQ" err
+'
+
 test_done
-- 
2.9.0.278.g1caae67

[PATCH v2 2/3] rebase -i: demonstrate a bug with --autosquash

[Johannes Schindelin]

When rearranging the edit script, we happily mistake the comment
character for a command, and the command for a SHA-1. As a consequence,
when we move fixup! and squash! commits, our logic to skip lines with
already handled SHA-1s mistakenly skips anything but the first
commented-out pick line, too.

The upcoming rebase--helper patches will address this bug, therefore we
do not need to make the current autosquash code even more complex than
it already is, just to fix this bug.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3415-rebase-autosquash.sh | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 8f53e54..9b71a49 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -271,4 +271,28 @@ test_expect_success 'autosquash with custom inst format' '
 	test 2 = $(git cat-file commit HEAD^ | grep squash | wc -l)
 '
 
+set_backup_editor () {
+	write_script backup-editor.sh <<-\EOF
+	cp "$1" .git/backup-"$(basename "$1")"
+	EOF
+	test_set_editor "$PWD/backup-editor.sh"
+}
+
+test_expect_failure 'autosquash with multiple empty patches' '
+	test_tick &&
+	git commit --allow-empty -m "empty" &&
+	test_tick &&
+	git commit --allow-empty -m "empty2" &&
+	test_tick &&
+	>fixup &&
+	git add fixup &&
+	git commit --fixup HEAD^^ &&
+	(
+		set_backup_editor &&
+		GIT_USE_REBASE_HELPER=false \
+		git rebase -i --force-rebase --autosquash HEAD~4 &&
+		grep empty2 .git/backup-git-rebase-todo
+	)
+'
+
 test_done
-- 
2.9.0.278.g1caae67

[PATCH v2 3/3] rebase -i: we allow extra spaces after fixup!/squash!

[Johannes Schindelin]

This new test case ensures that we handle commit messages that start
with fixup! or squash! followed by more than one space. While we do
not generate such messages when committing with --fixup/--squash, it
is perfectly legal for users to hand-craft their own fixup messages,
and we heed Postel's law by being lenient.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3415-rebase-autosquash.sh | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 9b71a49..48346f1 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -295,4 +295,13 @@ test_expect_failure 'autosquash with multiple empty patches' '
 	)
 '
 
+test_expect_success 'extra spaces after fixup!' '
+	base=$(git rev-parse HEAD) &&
+	test_commit to-fixup &&
+	git commit --allow-empty -m "fixup!  to-fixup" &&
+	git rebase -i --autosquash --keep-empty HEAD~2 &&
+	parent=$(git rev-parse HEAD^) &&
+	test $base = $parent
+'
+
 test_done
-- 
2.9.0.278.g1caae67