{"thread":{"id":"42746","subject":"[PATCH 0/2] Late edits to the rebase -i tests","startedAt":"2016-06-29T14:31:17Z","lastAt":"2016-07-07T15:53:23Z","messageCount":20,"participants":["Johannes Schindelin","Junio C Hamano","Pranit Bauva"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"290488","messageId":"cover.1467210629.git.johannes.schindelin@gmx.de","threadId":"42746","inReplyTo":null,"subject":"[PATCH 0/2] Late edits to the rebase -i tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-29T14:30:58Z","receivedAt":"2016-06-29T14:31:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"I meant to send the first patch much earlier, but got side-tracked\nbefore I could add the --gpg-sign test.\n\nThis is just another patch series in preparation for the rebase--helper\n(to be precise, this is the seventh out of fourteen patch series that\nhave not yet been merged to master; Six are already in flight, so I will\nstop here until the queue empties a little.)\n\n\nJohannes Schindelin (2):\n t3404: fix another typo\n t3404: add a test for the --gpg-sign option\n\n t/t3404-rebase-interactive.sh | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\nPublished-As: https://github.com/dscho/git/releases/tag/rebase-i-tests-v1\n-- \n2.9.0.270.g810e421\n\nbase-commit: cf4c2cfe52be5bd973a4838f73a35d3959ce2f43\n"},{"id":"290489","messageId":"2ad336745e1b2a610b20b7cfba169bd3e042c178.1467210629.git.johannes.schindelin@gmx.de","threadId":"42746","inReplyTo":"cover.1467210629.git.johannes.schindelin@gmx.de","subject":"[PATCH 1/2] t3404: fix another typo","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-29T14:31:03Z","receivedAt":"2016-06-29T14:31:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The past tense of \"to run\" is \"run\", not \"ran\".\n\nSigned-off-by: Johannes Schindelin \n---\n t/t3404-rebase-interactive.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 66348f1..c7ea8ba 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -60,7 +60,7 @@ test_expect_success 'setup' '\n \ttest_commit P fileP\n '\n \n-# \"exec\" commands are ran with the user shell by default, but this may\n+# \"exec\" commands are run with the user shell by default, but this may\n # be non-POSIX. For example, if SHELL=zsh then \">file\" doesn't work\n # to create a file. Unsetting SHELL avoids such non-portable behavior\n # in tests. It must be exported for it to take effect where needed.\n-- \n2.9.0.270.g810e421\n\n\n"},{"id":"290490","messageId":"0df34c45db5b1500e55262c8948c9140e7ad6cb8.1467210629.git.johannes.schindelin@gmx.de","threadId":"42746","inReplyTo":"cover.1467210629.git.johannes.schindelin@gmx.de","subject":"[PATCH 2/2] t3404: add a test for the --gpg-sign option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-29T14:31:07Z","receivedAt":"2016-06-29T14:31:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"To keep the time t3404 requires short (in this developer's Windows\nsetup, this single test already takes a painful 8 minutes to pass),\nwe avoid a full-blown GPG test and cop out by verifying the message\ndisplayed to the user upon an 'edit' command.\n\nSigned-off-by: Johannes Schindelin \n---\n t/t3404-rebase-interactive.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex c7ea8ba..4c96075 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1281,4 +1281,11 @@ test_expect_success 'editor saves as CR/LF' '\n \t)\n '\n \n+EPIPHANY=\"'\"\n+test_expect_success 'rebase -i --gpg-sign=' '\n+\tset_fake_editor &&\n+\tFAKE_LINES=\"edit 1\" git rebase -i --gpg-sign=\\\" HEAD^ >out 2>err &&\n+\tgrep \"$EPIPHANY-S\\\"$EPIPHANY\" err\n+'\n+\n test_done\n-- \n2.9.0.270.g810e421\n"},{"id":"290504","messageId":"xmqqlh1o584u.fsf@gitster.mtv.corp.google.com","threadId":"42746","inReplyTo":"2ad336745e1b2a610b20b7cfba169bd3e042c178.1467210629.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 1/2] t3404: fix another typo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-29T15:26:09Z","receivedAt":"2016-06-29T15:26:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> The past tense of \"to run\" is \"run\", not \"ran\".\n\nActually, past tense of the verb \"to run\" is \"ran\" ;-) The reason\nwhy this patch is still correct is because this is writing in\npassive voice, where you want \"be + past participle\", and the past\nparticiple of the verb \"to run\" is \"run\".\n\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> t/t3404-rebase-interactive.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 66348f1..c7ea8ba 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -60,7 +60,7 @@ test_expect_success 'setup' '\n> \ttest_commit P fileP\n> '\n> \n> -# \"exec\" commands are ran with the user shell by default, but this may\n> +# \"exec\" commands are run with the user shell by default, but this may\n> # be non-POSIX. For example, if SHELL=zsh then \">file\" doesn't work\n> # to create a file. Unsetting SHELL avoids such non-portable behavior\n> # in tests. It must be exported for it to take effect where needed.\n"},{"id":"290505","messageId":"alpine.DEB.2.20.1606291729200.12947@virtualbox","threadId":"42746","inReplyTo":"xmqqlh1o584u.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] t3404: fix another typo","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-29T15:31:17Z","receivedAt":"2016-06-29T15:33:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\n\nOn Wed, 29 Jun 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > The past tense of \"to run\" is \"run\", not \"ran\".\n> \n> Actually, past tense of the verb \"to run\" is \"ran\" ;-) The reason\n> why this patch is still correct is because this is writing in\n> passive voice, where you want \"be + past participle\", and the past\n> participle of the verb \"to run\" is \"run\".\n\nEmbarrassing! ;-)\n\nWell, I shall fix up the commit message thusly:\n\n t3404: fix another typo\n\n The passive form of \"to run\" is \"run\", not \"ran\".\n\n Signed-off-by: Johannes Schindelin \n\nOkay?\nDscho\n"},{"id":"290507","messageId":"CAFZEwPMw-Coq3vV+8o6MCDYCg9qN3aFBjHy9652mh_AFAPsJQQ@mail.gmail.com","threadId":"42746","inReplyTo":"2ad336745e1b2a610b20b7cfba169bd3e042c178.1467210629.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 1/2] t3404: fix another typo","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2016-06-29T15:11:05Z","receivedAt":"2016-06-29T16:15:24Z","isPatch":true,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"Hey Johannes,\n\nVery minor nits.\n\nOn Wed, Jun 29, 2016 at 8:01 PM, Johannes Schindelin\n wrote:\n> The past tense of \"to run\" is \"run\", not \"ran\".\n\nPast tense of \"to run\" is \"ran\" while past participle is \"to run\".\n\nPast tense: He ran.\nPast Participle: He has to run.\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> t/t3404-rebase-interactive.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 66348f1..c7ea8ba 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -60,7 +60,7 @@ test_expect_success 'setup' '\n> test_commit P fileP\n> '\n>\n> -# \"exec\" commands are ran with the user shell by default, but this may\n\nThis sentence seems to be in present tense if I am not wrong (though I\nam not a native English speaker).\n\n> +# \"exec\" commands are run with the user shell by default, but this may\n> # be non-POSIX. For example, if SHELL=zsh then \">file\" doesn't work\n> # to create a file. Unsetting SHELL avoids such non-portable behavior\n> # in tests. It must be exported for it to take effect where needed.\n\nThe change introduces fixed the grammo properly but the commit message\nprobably reports it incorrectly.\n\nWould it be better if the commit message is without changing anything else:\n The present tense of \"to run\" is \"run\", not \"ran\".\n\nRegards,\nPranit Bauva\n"},{"id":"290516","messageId":"xmqqh9cb50p3.fsf@gitster.mtv.corp.google.com","threadId":"42746","inReplyTo":"alpine.DEB.2.20.1606291729200.12947@virtualbox","subject":"Re: [PATCH 1/2] t3404: fix another typo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-29T18:06:48Z","receivedAt":"2016-06-29T18:07:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Wed, 29 Jun 2016, Junio C Hamano wrote:\n>\n>> Johannes Schindelin writes:\n>> \n>> > The past tense of \"to run\" is \"run\", not \"ran\".\n>> \n>> Actually, past tense of the verb \"to run\" is \"ran\" ;-) The reason\n>> why this patch is still correct is because this is writing in\n>> passive voice, where you want \"be + past participle\", and the past\n>> participle of the verb \"to run\" is \"run\".\n>\n> Embarrassing! ;-)\n>\n> Well, I shall fix up the commit message thusly:\n>\n> t3404: fix another typo\n>\n> The passive form of \"to run\" is \"run\", not \"ran\".\n\nIf your convention of referring to a verb is to show its infinitive\nform, i.e. \"to run\", then you would probably want to say\n\n\tThe passive from of \"to run\" is \"to be run\", not \"to be\n\tran\".\n\nBut I think we do not need any of this if we just retitled it to\n\n\tSubject: t3404: fix a grammo (commands are ran -> commands are run)\n\nwithout any body.\n\n\n\n"},{"id":"290518","messageId":"xmqq8txn4zm5.fsf@gitster.mtv.corp.google.com","threadId":"42746","inReplyTo":"0df34c45db5b1500e55262c8948c9140e7ad6cb8.1467210629.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-29T18:30:10Z","receivedAt":"2016-06-29T18:30:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> To keep the time t3404 requires short (in this developer's Windows\n> setup, this single test already takes a painful 8 minutes to pass),\n> we avoid a full-blown GPG test and cop out by verifying the message\n> displayed to the user upon an 'edit' command.\n\nThis is a tangent, but I wonder if we should be solving it by\nparallelizing the tests even more. If the script for example\ncan be split into part1 and part2 that share the same set-up\nthat is prepared by the very first test, we could split this\ninto three files (common one that is dot-sourced by two actual\ntest scripts that have part1 and part2).\n\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> t/t3404-rebase-interactive.sh | 7 +++++++\n> 1 file changed, 7 insertions(+)\n>\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index c7ea8ba..4c96075 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -1281,4 +1281,11 @@ test_expect_success 'editor saves as CR/LF' '\n> \t)\n> '\n> \n> +EPIPHANY=\"'\"\n\nWhy? If you really need a variable, SQ is probably the way this\ncodebase typically spell it.\n\n> +test_expect_success 'rebase -i --gpg-sign=' '\n> +\tset_fake_editor &&\n> +\tFAKE_LINES=\"edit 1\" git rebase -i --gpg-sign=\\\" HEAD^ >out 2>err &&\n> +\tgrep \"$EPIPHANY-S\\\"$EPIPHANY\" err\n\nI am not sure what is going on here. You are asking to sign using\nthe keyid of a single double-quote, and expecting the message that\nsays\n\n\tYou can amend the commit now, with\n\n\t\tgit commit --amend '-S\"'\n\n\t...\n\nthat has a substring '-S\"' in it to ensure that the codepath to\nparse --gpg-sign= on the command line of \"rebase\", and to the\nmessage we see here are working correctly, without actually checking\nif GPG is invoked at all, or if it is invoked the key given by the\noption is correctly passed to the invocation?\n\nIf so, can't you do that without confusing users by using keyid \"?\nIOW, wouldn't using --gpg-sign=me work equally well? For example:\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 4c96075..2dd3644 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1281,11 +1281,10 @@ test_expect_success 'editor saves as CR/LF' '\n \t)\n '\n \n-EPIPHANY=\"'\"\n test_expect_success 'rebase -i --gpg-sign=' '\n \tset_fake_editor &&\n-\tFAKE_LINES=\"edit 1\" git rebase -i --gpg-sign=\\\" HEAD^ >out 2>err &&\n-\tgrep \"$EPIPHANY-S\\\"$EPIPHANY\" err\n+\tFAKE_LINES=\"edit 1\" git rebase -i --gpg-sign=me HEAD^ >out 2>err &&\n+\tgrep -e \"commit --amend '\\''-Sme'\\''\" err\n '\n \n test_done\n"},{"id":"290568","messageId":"alpine.DEB.2.20.1606301017240.12947@virtualbox","threadId":"42746","inReplyTo":"xmqqh9cb50p3.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] t3404: fix another typo","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-30T08:17:48Z","receivedAt":"2016-06-30T08:19:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 29 Jun 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > On Wed, 29 Jun 2016, Junio C Hamano wrote:\n> >\n> >> Johannes Schindelin writes:\n> >> \n> >> > The past tense of \"to run\" is \"run\", not \"ran\".\n> >> \n> >> Actually, past tense of the verb \"to run\" is \"ran\" ;-) The reason\n> >> why this patch is still correct is because this is writing in\n> >> passive voice, where you want \"be + past participle\", and the past\n> >> participle of the verb \"to run\" is \"run\".\n> >\n> > Embarrassing! ;-)\n> >\n> > Well, I shall fix up the commit message thusly:\n> >\n> > t3404: fix another typo\n> >\n> > The passive form of \"to run\" is \"run\", not \"ran\".\n> \n> If your convention of referring to a verb is to show its infinitive\n> form, i.e. \"to run\", then you would probably want to say\n> \n> \tThe passive from of \"to run\" is \"to be run\", not \"to be\n> \tran\".\n> \n> But I think we do not need any of this if we just retitled it to\n> \n> \tSubject: t3404: fix a grammo (commands are ran -> commands are run)\n> \n> without any body.\n\nDone. Will be part of v2.\n\nCiao,\nDscho\n"},{"id":"290569","messageId":"alpine.DEB.2.20.1606301019040.12947@virtualbox","threadId":"42746","inReplyTo":"xmqq8txn4zm5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-30T08:25:30Z","receivedAt":"2016-06-30T08:25:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 29 Jun 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > To keep the time t3404 requires short (in this developer's Windows\n> > setup, this single test already takes a painful 8 minutes to pass),\n> > we avoid a full-blown GPG test and cop out by verifying the message\n> > displayed to the user upon an 'edit' command.\n> \n> This is a tangent, but I wonder if we should be solving it by\n> parallelizing the tests even more. If the script for example\n> can be split into part1 and part2 that share the same set-up\n> that is prepared by the very first test, we could split this\n> into three files (common one that is dot-sourced by two actual\n> test scripts that have part1 and part2).\n\nSure, that would work around the problem in the short run.\n\nIn the long run, the only real solution would be to stop relying on shell\nscripting so much, because the biggest performance hit on Windows stems\nfrom the fact that our test suite is a big honking shell script.\n\n> > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> > index c7ea8ba..4c96075 100755\n> > --- a/t/t3404-rebase-interactive.sh\n> > +++ b/t/t3404-rebase-interactive.sh\n> > @@ -1281,4 +1281,11 @@ test_expect_success 'editor saves as CR/LF' '\n> > \t)\n> > '\n> > \n> > +EPIPHANY=\"'\"\n> \n> Why? If you really need a variable, SQ is probably the way this\n> codebase typically spell it.\n\nHave you ever watched the movie \"Hook\"?\n\n> > +test_expect_success 'rebase -i --gpg-sign=' '\n> > +\tset_fake_editor &&\n> > +\tFAKE_LINES=\"edit 1\" git rebase -i --gpg-sign=\\\" HEAD^ >out 2>err &&\n> > +\tgrep \"$EPIPHANY-S\\\"$EPIPHANY\" err\n> \n> I am not sure what is going on here. You are asking to sign using\n> the keyid of a single double-quote, and expecting the message that\n> says\n> \n> \tYou can amend the commit now, with\n> \n> \t\tgit commit --amend '-S\"'\n\nPrecisely. That way, I not only verified that the key ID was correctly\npassed around, but also that it is quoted properly.\n\n> \t...\n> \n> that has a substring '-S\"' in it to ensure that the codepath to\n> parse --gpg-sign= on the command line of \"rebase\", and to the\n> message we see here are working correctly, without actually checking\n> if GPG is invoked at all, or if it is invoked the key given by the\n> option is correctly passed to the invocation?\n\nExactly. I want to test --gpg-sign even when there is no gpg executable\navailable.\n\n> If so, can't you do that without confusing users by using keyid \"?\n> IOW, wouldn't using --gpg-sign=me work equally well?\n\nNot really, because it is much easier to get quoting of \"me\" right than of\n\"\\\"\".\n\nI guess I could change the test to pass --gpg-sign=\"\\\"S I Gner\\\"\".\n\nCiao,\nDscho\n"},{"id":"290678","messageId":"xmqqpoqxz6kp.fsf@gitster.mtv.corp.google.com","threadId":"42746","inReplyTo":"alpine.DEB.2.20.1606301019040.12947@virtualbox","subject":"Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-01T16:06:14Z","receivedAt":"2016-07-01T16:08:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> that has a substring '-S\"' in it to ensure that the codepath to\n>> parse --gpg-sign= on the command line of \"rebase\", and to the\n>> message we see here are working correctly, without actually checking\n>> if GPG is invoked at all, or if it is invoked the key given by the\n>> option is correctly passed to the invocation?\n>\n> Exactly. I want to test --gpg-sign even when there is no gpg executable\n> available.\n\nThe other side of that coine is that even when GPG is available, we\ndo not see if it is invoked correctly at all. That was what I found\ndisturbing.\n"},{"id":"290739","messageId":"alpine.DEB.2.20.1607020933080.12947@virtualbox","threadId":"42746","inReplyTo":"xmqqpoqxz6kp.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-02T07:38:46Z","receivedAt":"2016-07-02T07:39:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 1 Jul 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> >> that has a substring '-S\"' in it to ensure that the codepath to\n> >> parse --gpg-sign= on the command line of \"rebase\", and to the\n> >> message we see here are working correctly, without actually checking\n> >> if GPG is invoked at all, or if it is invoked the key given by the\n> >> option is correctly passed to the invocation?\n> >\n> > Exactly. I want to test --gpg-sign even when there is no gpg executable\n> > available.\n> \n> The other side of that coine is that even when GPG is available, we\n> do not see if it is invoked correctly at all. That was what I found\n> disturbing.\n\nOkay, I see now.\n\nHowever, is it not better to have even a limited test than none at all?\nGranted, we still would not know whether rebased commits would be signed\nproperly. But if my trivial test case fails, we would still have a strong\nindicator that something is broken, with a very convenient way to debug\nit. And that is what a regression test suite is all about, isn't it?\n\nOf course I agree that it would be very nice to have a test at a later\nstage that does exercise GPG if it is available. But would it really be so\nterrible to have a (simpler, not as complete) test that is exercised\n*also* when GPG is *not* available?\n\nCiao,\nDscho\n"},{"id":"290876","messageId":"xmqqeg76vmzb.fsf@gitster.mtv.corp.google.com","threadId":"42746","inReplyTo":"alpine.DEB.2.20.1607020933080.12947@virtualbox","subject":"Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-06T14:51:20Z","receivedAt":"2016-07-06T14:51:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Of course I agree that it would be very nice to have a test at a later\n> stage that does exercise GPG if it is available. But would it really be so\n> terrible to have a (simpler, not as complete) test that is exercised\n> *also* when GPG is *not* available?\n\nWhat I would expect is \"In the ideal world, we may want both, and in\nan imperfect world in which we can have only one, we'd rather have\nthe 'even though we can run it only when GPG is available, we make\nsure that we drive GPG correctly' one, dropping the other.\", simply\nbecause the end result matters more, not how the instruction to the\nend user is phrased.\n\nSure, in even less perfect world, having a superficial test might be\nbetter than nothing, but reminding ourselves to aim high (and make\nsure we document the decision when we punt) is an important part of\nthe purpose of the review process, so...\n\n\n\n"},{"id":"290884","messageId":"alpine.DEB.2.20.1607061733200.6426@virtualbox","threadId":"42746","inReplyTo":"xmqqeg76vmzb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-06T15:34:54Z","receivedAt":"2016-07-06T15:35:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 6 Jul 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > Of course I agree that it would be very nice to have a test at a later\n> > stage that does exercise GPG if it is available. But would it really\n> > be so terrible to have a (simpler, not as complete) test that is\n> > exercised *also* when GPG is *not* available?\n> \n> What I would expect is \"In the ideal world, we may want both, and in an\n> imperfect world in which we can have only one, we'd rather have the\n> 'even though we can run it only when GPG is available, we make sure that\n> we drive GPG correctly' one, dropping the other.\", simply because the\n> end result matters more, not how the instruction to the end user is\n> phrased.\n> \n> Sure, in even less perfect world, having a superficial test might be\n> better than nothing, but reminding ourselves to aim high (and make sure\n> we document the decision when we punt) is an important part of the\n> purpose of the review process, so...\n\nOkay, so here is the deal: on the development machine where this was\ndeveloped, I do not have gpg installed. So please take this test case just\nto make sure that things work as intended for the moment.\n\nBefore sending the last rebase--helper patch series, I will make sure to\nadd a real test that requires gpg, and submit that, too.\n\nDeal?\n\nCiao,\nDscho\n"},{"id":"290904","messageId":"xmqqmvlusi7v.fsf@gitster.mtv.corp.google.com","threadId":"42746","inReplyTo":"alpine.DEB.2.20.1607061733200.6426@virtualbox","subject":"Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-06T19:02:28Z","receivedAt":"2016-07-06T19:02:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Okay, so here is the deal: on the development machine where this was\n> developed, I do not have gpg installed. So please take this test case just\n> to make sure that things work as intended for the moment.\n>\n> Before sending the last rebase--helper patch series, I will make sure to\n> add a real test that requires gpg, and submit that, too.\n>\n> Deal?\n\nI do not particularly care if the latter one happens.\n\nThe only thing I care about is that the earlier round documents that\nwe know we probably should test the real driving of the GPG program,\nbut we deliberately do not do so in the series, and hint that such\nan enhancement can happen later.\n\nThat might even entice others to help writing a test or two ;-)\n"},{"id":"290955","messageId":"alpine.DEB.2.20.1607071721590.6426@virtualbox","threadId":"42746","inReplyTo":"xmqqmvlusi7v.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] t3404: add a test for the --gpg-sign option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-07T15:23:00Z","receivedAt":"2016-07-07T15:23:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 6 Jul 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > Okay, so here is the deal: on the development machine where this was\n> > developed, I do not have gpg installed. So please take this test case just\n> > to make sure that things work as intended for the moment.\n> >\n> > Before sending the last rebase--helper patch series, I will make sure to\n> > add a real test that requires gpg, and submit that, too.\n> >\n> > Deal?\n> \n> I do not particularly care if the latter one happens.\n> \n> The only thing I care about is that the earlier round documents that\n> we know we probably should test the real driving of the GPG program,\n> but we deliberately do not do so in the series, and hint that such\n> an enhancement can happen later.\n> \n> That might even entice others to help writing a test or two ;-)\n\nOkay. I tried my hand at editing the commit message, and threw two more\ntests into the patch series for good measure. Will send out v2 once the\ntest suite passed (it's still running).\n\nThanks,\nDscho\n"},{"id":"290957","messageId":"cover.1467906747.git.johannes.schindelin@gmx.de","threadId":"42746","inReplyTo":"cover.1467210629.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 0/3] Additional rebase -i tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-07T15:52:46Z","receivedAt":"2016-07-07T15:53:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This is just another patch series in preparation for the rebase--helper.\n\nRelative to v1 of this patch series,\n\n- the grammo fix was backed out because it was picked up separately,\n\n- two new tests were introduced, one demonstrating a bug, the other one\n ensuring that the rebase--helper does not introduce a regression (this\n test actually helped me debug and fix a regression in some previous\n revision of the rebase--helper)\n\n\nJohannes Schindelin (3):\n t3404: add a test for the --gpg-sign option\n rebase -i: demonstrate a bug with --autosquash\n rebase -i: we allow extra spaces after fixup!/squash!\n\n t/t3404-rebase-interactive.sh | 8 ++++++++\n t/t3415-rebase-autosquash.sh | 33 +++++++++++++++++++++++++++++++++\n 2 files changed, 41 insertions(+)\n\nPublished-As: https://github.com/dscho/git/releases/tag/rebase-i-tests-v2\nInterdiff vs v1:\n\n diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n index 4c96075..aa393d2 100755\n --- a/t/t3404-rebase-interactive.sh\n +++ b/t/t3404-rebase-interactive.sh\n @@ -60,7 +60,7 @@ test_expect_success 'setup' '\n \ttest_commit P fileP\n '\n \n -# \"exec\" commands are run with the user shell by default, but this may\n +# \"exec\" commands are ran with the user shell by default, but this may\n # be non-POSIX. For example, if SHELL=zsh then \">file\" doesn't work\n # to create a file. Unsetting SHELL avoids such non-portable behavior\n # in tests. It must be exported for it to take effect where needed.\n @@ -1281,11 +1281,12 @@ test_expect_success 'editor saves as CR/LF' '\n \t)\n '\n \n -EPIPHANY=\"'\"\n +SQ=\"'\"\n test_expect_success 'rebase -i --gpg-sign=' '\n \tset_fake_editor &&\n -\tFAKE_LINES=\"edit 1\" git rebase -i --gpg-sign=\\\" HEAD^ >out 2>err &&\n -\tgrep \"$EPIPHANY-S\\\"$EPIPHANY\" err\n +\tFAKE_LINES=\"edit 1\" git rebase -i --gpg-sign=\"\\\"S I Gner\\\"\" HEAD^ \\\n +\t\t>out 2>err &&\n +\tgrep \"$SQ-S\\\"S I Gner\\\"$SQ\" err\n '\n \n test_done\n diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh\n index 8f53e54..48346f1 100755\n --- a/t/t3415-rebase-autosquash.sh\n +++ b/t/t3415-rebase-autosquash.sh\n @@ -271,4 +271,37 @@ test_expect_success 'autosquash with custom inst format' '\n \ttest 2 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n '\n \n +set_backup_editor () {\n +\twrite_script backup-editor.sh <<-\\EOF\n +\tcp \"$1\" .git/backup-\"$(basename \"$1\")\"\n +\tEOF\n +\ttest_set_editor \"$PWD/backup-editor.sh\"\n +}\n +\n +test_expect_failure 'autosquash with multiple empty patches' '\n +\ttest_tick &&\n +\tgit commit --allow-empty -m \"empty\" &&\n +\ttest_tick &&\n +\tgit commit --allow-empty -m \"empty2\" &&\n +\ttest_tick &&\n +\t>fixup &&\n +\tgit add fixup &&\n +\tgit commit --fixup HEAD^^ &&\n +\t(\n +\t\tset_backup_editor &&\n +\t\tGIT_USE_REBASE_HELPER=false \\\n +\t\tgit rebase -i --force-rebase --autosquash HEAD~4 &&\n +\t\tgrep empty2 .git/backup-git-rebase-todo\n +\t)\n +'\n +\n +test_expect_success 'extra spaces after fixup!' '\n +\tbase=$(git rev-parse HEAD) &&\n +\ttest_commit to-fixup &&\n +\tgit commit --allow-empty -m \"fixup! to-fixup\" &&\n +\tgit rebase -i --autosquash --keep-empty HEAD~2 &&\n +\tparent=$(git rev-parse HEAD^) &&\n +\ttest $base = $parent\n +'\n +\n test_done\n\n-- \n2.9.0.278.g1caae67\n\nbase-commit: 5c589a73de4394ad125a4effac227b3aec856fa1\n"},{"id":"290958","messageId":"46a5e7960232dc89a1db7728983c8f2e6c939c69.1467906747.git.johannes.schindelin@gmx.de","threadId":"42746","inReplyTo":"cover.1467906747.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 2/3] rebase -i: demonstrate a bug with --autosquash","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-07T15:52:54Z","receivedAt":"2016-07-07T15:53:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When rearranging the edit script, we happily mistake the comment\ncharacter for a command, and the command for a SHA-1. As a consequence,\nwhen we move fixup! and squash! commits, our logic to skip lines with\nalready handled SHA-1s mistakenly skips anything but the first\ncommented-out pick line, too.\n\nThe upcoming rebase--helper patches will address this bug, therefore we\ndo not need to make the current autosquash code even more complex than\nit already is, just to fix this bug.\n\nSigned-off-by: Johannes Schindelin \n---\n t/t3415-rebase-autosquash.sh | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh\nindex 8f53e54..9b71a49 100755\n--- a/t/t3415-rebase-autosquash.sh\n+++ b/t/t3415-rebase-autosquash.sh\n@@ -271,4 +271,28 @@ test_expect_success 'autosquash with custom inst format' '\n \ttest 2 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n '\n \n+set_backup_editor () {\n+\twrite_script backup-editor.sh <<-\\EOF\n+\tcp \"$1\" .git/backup-\"$(basename \"$1\")\"\n+\tEOF\n+\ttest_set_editor \"$PWD/backup-editor.sh\"\n+}\n+\n+test_expect_failure 'autosquash with multiple empty patches' '\n+\ttest_tick &&\n+\tgit commit --allow-empty -m \"empty\" &&\n+\ttest_tick &&\n+\tgit commit --allow-empty -m \"empty2\" &&\n+\ttest_tick &&\n+\t>fixup &&\n+\tgit add fixup &&\n+\tgit commit --fixup HEAD^^ &&\n+\t(\n+\t\tset_backup_editor &&\n+\t\tGIT_USE_REBASE_HELPER=false \\\n+\t\tgit rebase -i --force-rebase --autosquash HEAD~4 &&\n+\t\tgrep empty2 .git/backup-git-rebase-todo\n+\t)\n+'\n+\n test_done\n-- \n2.9.0.278.g1caae67\n\n\n"},{"id":"290959","messageId":"4fe1c1b7598f3c5ddc0cf8f9c0ef89ac193a22e2.1467906747.git.johannes.schindelin@gmx.de","threadId":"42746","inReplyTo":"cover.1467906747.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 1/3] t3404: add a test for the --gpg-sign option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-07T15:52:50Z","receivedAt":"2016-07-07T15:53:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"For the upcoming rebase--helper work (which will accelerate the\ninteractive rebase noticably), it is important to verify that the\n--gpg-sign option is handled properly.\n\nPlease note that this patch does this on the cheap, by verifying that\nthe expected option is printed in the message of the 'edit' operation.\n\nWe really should test that the interactive rebase signs the commits\nproperly, iff GPG is available. This test is left for later.\n\nSigned-off-by: Johannes Schindelin \n---\n t/t3404-rebase-interactive.sh | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 66348f1..aa393d2 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1281,4 +1281,12 @@ test_expect_success 'editor saves as CR/LF' '\n \t)\n '\n \n+SQ=\"'\"\n+test_expect_success 'rebase -i --gpg-sign=' '\n+\tset_fake_editor &&\n+\tFAKE_LINES=\"edit 1\" git rebase -i --gpg-sign=\"\\\"S I Gner\\\"\" HEAD^ \\\n+\t\t>out 2>err &&\n+\tgrep \"$SQ-S\\\"S I Gner\\\"$SQ\" err\n+'\n+\n test_done\n-- \n2.9.0.278.g1caae67\n\n\n"},{"id":"290960","messageId":"7a3f6dde53593160e8200a7bbec902d8fdfcf6bd.1467906747.git.johannes.schindelin@gmx.de","threadId":"42746","inReplyTo":"cover.1467906747.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 3/3] rebase -i: we allow extra spaces after fixup!/squash!","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-07T15:52:57Z","receivedAt":"2016-07-07T15:53:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This new test case ensures that we handle commit messages that start\nwith fixup! or squash! followed by more than one space. While we do\nnot generate such messages when committing with --fixup/--squash, it\nis perfectly legal for users to hand-craft their own fixup messages,\nand we heed Postel's law by being lenient.\n\nSigned-off-by: Johannes Schindelin \n---\n t/t3415-rebase-autosquash.sh | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh\nindex 9b71a49..48346f1 100755\n--- a/t/t3415-rebase-autosquash.sh\n+++ b/t/t3415-rebase-autosquash.sh\n@@ -295,4 +295,13 @@ test_expect_failure 'autosquash with multiple empty patches' '\n \t)\n '\n \n+test_expect_success 'extra spaces after fixup!' '\n+\tbase=$(git rev-parse HEAD) &&\n+\ttest_commit to-fixup &&\n+\tgit commit --allow-empty -m \"fixup! to-fixup\" &&\n+\tgit rebase -i --autosquash --keep-empty HEAD~2 &&\n+\tparent=$(git rev-parse HEAD^) &&\n+\ttest $base = $parent\n+'\n+\n test_done\n-- \n2.9.0.278.g1caae67\n"}]}