* [PATCH 0/2] Late edits to the rebase -i tests @ 2016-06-29 14:30 Johannes Schindelin 2016-06-29 14:31 ` [PATCH 1/2] t3404: fix another typo Johannes Schindelin ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Johannes Schindelin @ 2016-06-29 14:30 UTC (permalink / raw) To: git; +Cc: Junio C Hamano 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] t3404: fix another typo 2016-06-29 14:30 [PATCH 0/2] Late edits to the rebase -i tests Johannes Schindelin @ 2016-06-29 14:31 ` Johannes Schindelin 2016-06-29 15:11 ` Pranit Bauva 2016-06-29 15:26 ` Junio C Hamano 2016-06-29 14:31 ` [PATCH 2/2] t3404: add a test for the --gpg-sign option Johannes Schindelin 2016-07-07 15:52 ` [PATCH v2 0/3] Additional rebase -i tests Johannes Schindelin 2 siblings, 2 replies; 20+ messages in thread From: Johannes Schindelin @ 2016-06-29 14:31 UTC (permalink / raw) To: git; +Cc: Junio C Hamano 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] t3404: fix another typo 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 1 sibling, 0 replies; 20+ messages in thread From: Pranit Bauva @ 2016-06-29 15:11 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Git List, Junio C Hamano 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] t3404: fix another typo 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 1 sibling, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2016-06-29 15:26 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] t3404: fix another typo 2016-06-29 15:26 ` Junio C Hamano @ 2016-06-29 15:31 ` Johannes Schindelin 2016-06-29 18:06 ` Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Johannes Schindelin @ 2016-06-29 15:31 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Pranit Bauva 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] t3404: fix another typo 2016-06-29 15:31 ` Johannes Schindelin @ 2016-06-29 18:06 ` Junio C Hamano 2016-06-30 8:17 ` Johannes Schindelin 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2016-06-29 18:06 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Pranit Bauva 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] t3404: fix another typo 2016-06-29 18:06 ` Junio C Hamano @ 2016-06-30 8:17 ` Johannes Schindelin 0 siblings, 0 replies; 20+ messages in thread From: Johannes Schindelin @ 2016-06-30 8:17 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Pranit Bauva 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] t3404: add a test for the --gpg-sign option 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 14:31 ` Johannes Schindelin 2016-06-29 18:30 ` Junio C Hamano 2016-07-07 15:52 ` [PATCH v2 0/3] Additional rebase -i tests Johannes Schindelin 2 siblings, 1 reply; 20+ messages in thread From: Johannes Schindelin @ 2016-06-29 14:31 UTC (permalink / raw) To: git; +Cc: Junio C Hamano 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option 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 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2016-06-29 18:30 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option 2016-06-29 18:30 ` Junio C Hamano @ 2016-06-30 8:25 ` Johannes Schindelin 2016-07-01 16:06 ` Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Johannes Schindelin @ 2016-06-30 8:25 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option 2016-06-30 8:25 ` Johannes Schindelin @ 2016-07-01 16:06 ` Junio C Hamano 2016-07-02 7:38 ` Johannes Schindelin 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2016-07-01 16:06 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option 2016-07-01 16:06 ` Junio C Hamano @ 2016-07-02 7:38 ` Johannes Schindelin 2016-07-06 14:51 ` Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Johannes Schindelin @ 2016-07-02 7:38 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option 2016-07-02 7:38 ` Johannes Schindelin @ 2016-07-06 14:51 ` Junio C Hamano 2016-07-06 15:34 ` Johannes Schindelin 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2016-07-06 14:51 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git 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... ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option 2016-07-06 14:51 ` Junio C Hamano @ 2016-07-06 15:34 ` Johannes Schindelin 2016-07-06 19:02 ` Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Johannes Schindelin @ 2016-07-06 15:34 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option 2016-07-06 15:34 ` Johannes Schindelin @ 2016-07-06 19:02 ` Junio C Hamano 2016-07-07 15:23 ` Johannes Schindelin 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2016-07-06 19:02 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git 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 ;-) ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option 2016-07-06 19:02 ` Junio C Hamano @ 2016-07-07 15:23 ` Johannes Schindelin 0 siblings, 0 replies; 20+ messages in thread From: Johannes Schindelin @ 2016-07-07 15:23 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 0/3] Additional rebase -i tests 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 14:31 ` [PATCH 2/2] t3404: add a test for the --gpg-sign option Johannes Schindelin @ 2016-07-07 15:52 ` Johannes Schindelin 2016-07-07 15:52 ` [PATCH v2 1/3] t3404: add a test for the --gpg-sign option Johannes Schindelin ` (2 more replies) 2 siblings, 3 replies; 20+ messages in thread From: Johannes Schindelin @ 2016-07-07 15:52 UTC (permalink / raw) To: git; +Cc: Junio C Hamano 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/3] t3404: add a test for the --gpg-sign option 2016-07-07 15:52 ` [PATCH v2 0/3] Additional rebase -i tests Johannes Schindelin @ 2016-07-07 15:52 ` 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 2 siblings, 0 replies; 20+ messages in thread From: Johannes Schindelin @ 2016-07-07 15:52 UTC (permalink / raw) To: git; +Cc: Junio C Hamano 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/3] rebase -i: demonstrate a bug with --autosquash 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 ` Johannes Schindelin 2016-07-07 15:52 ` [PATCH v2 3/3] rebase -i: we allow extra spaces after fixup!/squash! Johannes Schindelin 2 siblings, 0 replies; 20+ messages in thread From: Johannes Schindelin @ 2016-07-07 15:52 UTC (permalink / raw) To: git; +Cc: Junio C Hamano 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/3] rebase -i: we allow extra spaces after fixup!/squash! 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 ` Johannes Schindelin 2 siblings, 0 replies; 20+ messages in thread From: Johannes Schindelin @ 2016-07-07 15:52 UTC (permalink / raw) To: git; +Cc: Junio C Hamano 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-07-07 15:53 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 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
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).