{"thread":{"id":"12387","subject":"[PATCH] Add test for git rebase --abort","startedAt":"2008-02-29T22:08:47Z","lastAt":"2008-03-02T03:57:34Z","messageCount":12,"participants":["Mike Hommey","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"70432","messageId":"1204322927-22407-1-git-send-email-mh@glandium.org","threadId":"12387","inReplyTo":null,"subject":"[PATCH] Add test for git rebase --abort","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-02-29T22:08:47Z","receivedAt":"2008-02-29T22:08:47Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"We expect git rebase --abort to come back to the original (pre-rebase)\nhead, independently from when it's run during a rebase.\n\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n\n I wrote this test because I spotted a problem with git rebase --abort, and\n expanded the test to other cases that work, but that it is still better to\n have a test for.\n\n The failing test is the third. I don't have enough knowledge in git-rebase\n to write an appropriate fix, but the problem seems to be in\n move_to_original_branch, where testing head_name doesn't seem appropriate.\n\n t/t3407-rebase-abort.sh |   59 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 59 insertions(+), 0 deletions(-)\n create mode 100755 t/t3407-rebase-abort.sh\n\ndiff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh\nnew file mode 100755\nindex 0000000..abdecc9\n--- /dev/null\n+++ b/t/t3407-rebase-abort.sh\n@@ -0,0 +1,59 @@\n+#!/bin/sh\n+\n+test_description='git rebase --abort tests'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\techo a > a &&\n+\tgit add a &&\n+\tgit commit -m a &&\n+\tgit branch to-rebase &&\n+\n+\techo b > a &&\n+\tgit commit -a -m b &&\n+\techo c > a &&\n+\tgit commit -a -m c &&\n+\n+\tgit checkout to-rebase &&\n+\techo d > a &&\n+\tgit commit -a -m \"merge should fail on this\" &&\n+\techo e > a &&\n+\tgit commit -a -m \"merge should fail on this, too\" &&\n+\tgit branch pre-rebase\n+'\n+\n+test_expect_success 'rebase --abort' '\n+\t! git rebase master &&\n+\tgit rebase --abort &&\n+\ttest $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)\n+'\n+\n+# In case previous test failed\n+git reset --hard pre-rebase >&3 2>&4\n+rm -rf .dotest # Should be changed whenever rebase stop using .dotest\n+\n+test_expect_success 'rebase --abort after --skip' '\n+\t! git rebase master &&\n+\t! git rebase --skip &&\n+\ttest $(git rev-parse HEAD) = $(git rev-parse master) &&\n+\tsh -x ../../git-rebase --abort &&\n+\ttest $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)\n+'\n+\n+# In case previous test failed\n+git reset --hard pre-rebase >&3 2>&4\n+rm -rf .dotest # Should be changed whenever rebase stop using .dotest\n+\n+test_expect_success 'rebase --abort after --continue' '\n+\t! git rebase master &&\n+\techo c > a &&\n+\techo d >> a &&\n+\tgit add a &&\n+\t! git rebase --continue &&\n+\ttest $(git rev-parse HEAD) != $(git rev-parse master) &&\n+\tgit rebase --abort &&\n+\ttest $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)\n+'\n+\n+test_done\n-- \n1.5.4.3.340.g97b97.dirty\n\n"},{"id":"70446","messageId":"7v63w7bb06.fsf@gitster.siamese.dyndns.org","threadId":"12387","inReplyTo":"1204322927-22407-1-git-send-email-mh@glandium.org","subject":"Re: [PATCH] Add test for git rebase --abort","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-29T23:26:01Z","receivedAt":"2008-02-29T23:26:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey <mh@glandium.org> writes:\n\n>  The failing test is the third. I don't have enough knowledge in git-rebase\n>  to write an appropriate fix, but the problem seems to be in\n>  move_to_original_branch, where testing head_name doesn't seem appropriate.\n\nPlease mark such an \"expected to succeed but fails due to\nsuspected bug\" with test_expect_failure.\n\n> +test_expect_success 'rebase --abort' '\n> +\t! git rebase master &&\n\nWhen making sure \"git frotz\" refuses gracefully (instead of\nsegfault-and-burn), please say \"test_must_fail git frotz\".\n\n> +# In case previous test failed\n> +git reset --hard pre-rebase >&3 2>&4\n> +rm -rf .dotest # Should be changed whenever rebase stop using .dotest\n\nHave this kind of clean-up at the very beginning of the next\ntest.  Test writers should not have to learn about file\ndescriptors 3 and 4.\n\n        Side note.  As a test framework extension, we might want\n        to add 4th parameter to test_expect_{success,failure}\n        that specifies a clean-up to be made regardless of the\n        outcome of the test.\n\n> +test_expect_success 'rebase --abort after --skip' '\n> +\t! git rebase master &&\n> +\t! git rebase --skip &&\n> +\ttest $(git rev-parse HEAD) = $(git rev-parse master) &&\n> +\tsh -x ../../git-rebase --abort &&\n> +\ttest $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)\n> +'\n\nI take that \"sh -x ../../\" is not for inclusion in the official\nrelease.\n"},{"id":"70448","messageId":"alpine.LSU.1.00.0802292338190.22527@racer.site","threadId":"12387","inReplyTo":"7v63w7bb06.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Add test for git rebase --abort","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-29T23:39:19Z","receivedAt":"2008-02-29T23:39:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 29 Feb 2008, Junio C Hamano wrote:\n\n>         Side note.  As a test framework extension, we might want\n>         to add 4th parameter to test_expect_{success,failure}\n>         that specifies a clean-up to be made regardless of the\n>         outcome of the test.\n\nI saw something the other night, where the test _I_ introduced, for \ngit-daemon, failed (with -i -v), and the daemon was _not_ killed as I \nexpected (with trap \"<bla>\" 0).\n\nCiao,\nDscho\n\n"},{"id":"70486","messageId":"20080301073612.GA26767@glandium.org","threadId":"12387","inReplyTo":"7v63w7bb06.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Add test for git rebase --abort","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-03-01T07:36:12Z","receivedAt":"2008-03-01T07:36:12Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Fri, Feb 29, 2008 at 03:26:01PM -0800, Junio C Hamano wrote:\n> Mike Hommey <mh@glandium.org> writes:\n> \n> >  The failing test is the third. I don't have enough knowledge in git-rebase\n> >  to write an appropriate fix, but the problem seems to be in\n> >  move_to_original_branch, where testing head_name doesn't seem appropriate.\n> \n> Please mark such an \"expected to succeed but fails due to\n> suspected bug\" with test_expect_failure.\n\nI was kind of expecting the bug would be fixed before the test be\nincluded ;)\n\n> > +test_expect_success 'rebase --abort' '\n> > +\t! git rebase master &&\n> \n> When making sure \"git frotz\" refuses gracefully (instead of\n> segfault-and-burn), please say \"test_must_fail git frotz\".\n\nOoooh, I just saw 74359821.\n\n> > +# In case previous test failed\n> > +git reset --hard pre-rebase >&3 2>&4\n> > +rm -rf .dotest # Should be changed whenever rebase stop using .dotest\n> \n> Have this kind of clean-up at the very beginning of the next\n> test.  Test writers should not have to learn about file\n> descriptors 3 and 4.\n> \n>         Side note.  As a test framework extension, we might want\n>         to add 4th parameter to test_expect_{success,failure}\n>         that specifies a clean-up to be made regardless of the\n>         outcome of the test.\n> \n> > +test_expect_success 'rebase --abort after --skip' '\n> > +\t! git rebase master &&\n> > +\t! git rebase --skip &&\n> > +\ttest $(git rev-parse HEAD) = $(git rev-parse master) &&\n> > +\tsh -x ../../git-rebase --abort &&\n> > +\ttest $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)\n> > +'\n> \n> I take that \"sh -x ../../\" is not for inclusion in the official\n> release.\n\nD'oh, I forgot to change that back.\n\nMike\n"},{"id":"70490","messageId":"7vlk526gdl.fsf@gitster.siamese.dyndns.org","threadId":"12387","inReplyTo":"20080301073612.GA26767@glandium.org","subject":"Re: [PATCH] Add test for git rebase --abort","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-01T07:41:10Z","receivedAt":"2008-03-01T07:41:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey <mh@glandium.org> writes:\n\n> On Fri, Feb 29, 2008 at 03:26:01PM -0800, Junio C Hamano wrote:\n> ...\n>> Please mark such an \"expected to succeed but fails due to\n>> suspected bug\" with test_expect_failure.\n>\n> I was kind of expecting the bug would be fixed before the test be\n> included ;)\n\nYour patch does not have to go through me to whoever fixes the issue.\nPeople can pick it up from the list, apply them to their tree and start\nworking on it.  The point is to be gentle to them.\n"},{"id":"70492","messageId":"20080301074511.GB26767@glandium.org","threadId":"12387","inReplyTo":"20080301073612.GA26767@glandium.org","subject":"Re: [PATCH] Add test for git rebase --abort","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-03-01T07:45:11Z","receivedAt":"2008-03-01T07:45:11Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sat, Mar 01, 2008 at 08:36:12AM +0100, Mike Hommey wrote:\n> On Fri, Feb 29, 2008 at 03:26:01PM -0800, Junio C Hamano wrote:\n> > Mike Hommey <mh@glandium.org> writes:\n> > \n> > >  The failing test is the third. I don't have enough knowledge in git-rebase\n> > >  to write an appropriate fix, but the problem seems to be in\n> > >  move_to_original_branch, where testing head_name doesn't seem appropriate.\n> > \n> > Please mark such an \"expected to succeed but fails due to\n> > suspected bug\" with test_expect_failure.\n> \n> I was kind of expecting the bug would be fixed before the test be\n> included ;)\n\n... and the test actually passes with v1.5.0. I'll bisect.\n\nMike\n"},{"id":"70494","messageId":"20080301081511.GA30613@glandium.org","threadId":"12387","inReplyTo":"20080301074511.GB26767@glandium.org","subject":"Re: [PATCH] Add test for git rebase --abort","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-03-01T08:15:11Z","receivedAt":"2008-03-01T08:15:11Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sat, Mar 01, 2008 at 08:45:11AM +0100, Mike Hommey wrote:\n> On Sat, Mar 01, 2008 at 08:36:12AM +0100, Mike Hommey wrote:\n> > On Fri, Feb 29, 2008 at 03:26:01PM -0800, Junio C Hamano wrote:\n> > > Mike Hommey <mh@glandium.org> writes:\n> > > \n> > > >  The failing test is the third. I don't have enough knowledge in git-rebase\n> > > >  to write an appropriate fix, but the problem seems to be in\n> > > >  move_to_original_branch, where testing head_name doesn't seem appropriate.\n> > > \n> > > Please mark such an \"expected to succeed but fails due to\n> > > suspected bug\" with test_expect_failure.\n> > \n> > I was kind of expecting the bug would be fixed before the test be\n> > included ;)\n> \n> ... and the test actually passes with v1.5.0. I'll bisect.\n\n... and I'm even the one to blame\nfb6e4e1f3f048898677f3cf177bfcaf60123bd5c is first bad commit\n\nMike\n"},{"id":"70501","messageId":"7vod9y4xcn.fsf@gitster.siamese.dyndns.org","threadId":"12387","inReplyTo":"20080301081511.GA30613@glandium.org","subject":"Re: [PATCH] Add test for git rebase --abort","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-01T09:17:28Z","receivedAt":"2008-03-01T09:17:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey <mh@glandium.org> writes:\n\n> On Sat, Mar 01, 2008 at 08:45:11AM +0100, Mike Hommey wrote:\n>> On Sat, Mar 01, 2008 at 08:36:12AM +0100, Mike Hommey wrote:\n>> > On Fri, Feb 29, 2008 at 03:26:01PM -0800, Junio C Hamano wrote:\n>> > > Mike Hommey <mh@glandium.org> writes:\n>> > > \n>> > > >  The failing test is the third. I don't have enough knowledge in git-rebase\n>> > > >  to write an appropriate fix, but the problem seems to be in\n>> > > >  move_to_original_branch, where testing head_name doesn't seem appropriate.\n>> > > \n>> > > Please mark such an \"expected to succeed but fails due to\n>> > > suspected bug\" with test_expect_failure.\n>> > \n>> > I was kind of expecting the bug would be fixed before the test be\n>> > included ;)\n>> \n>> ... and the test actually passes with v1.5.0. I'll bisect.\n>\n> ... and I'm even the one to blame\n> fb6e4e1f3f048898677f3cf177bfcaf60123bd5c is first bad commit\n\nHeh, didn't you say you don't have enough knowledge in git-rebase? ;-)\n\nI've applied the test with the following fixups.\n\n t/t3407-rebase-abort.sh |   28 ++++++++++++++--------------\n 1 files changed, 14 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh\nindex abdecc9..94bdd72 100755\n--- a/t/t3407-rebase-abort.sh\n+++ b/t/t3407-rebase-abort.sh\n@@ -24,33 +24,33 @@ test_expect_success setup '\n '\n \n test_expect_success 'rebase --abort' '\n-\t! git rebase master &&\n+\ttest_must_fail git rebase master &&\n \tgit rebase --abort &&\n \ttest $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)\n '\n \n-# In case previous test failed\n-git reset --hard pre-rebase >&3 2>&4\n-rm -rf .dotest # Should be changed whenever rebase stop using .dotest\n+test_expect_failure 'rebase --abort after --skip' '\n+\t# Clean up the state from the previous one\n+\tgit reset --hard pre-rebase\n+\trm -rf .dotest\n \n-test_expect_success 'rebase --abort after --skip' '\n-\t! git rebase master &&\n-\t! git rebase --skip &&\n+\ttest_must_fail git rebase master &&\n+\ttest_must_fail git rebase --skip &&\n \ttest $(git rev-parse HEAD) = $(git rev-parse master) &&\n-\tsh -x ../../git-rebase --abort &&\n+\tgit rebase --abort &&\n \ttest $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)\n '\n \n-# In case previous test failed\n-git reset --hard pre-rebase >&3 2>&4\n-rm -rf .dotest # Should be changed whenever rebase stop using .dotest\n"},{"id":"70507","messageId":"1204367534-28204-1-git-send-email-mh@glandium.org","threadId":"12387","inReplyTo":"7vod9y4xcn.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Fix git reset --abort not restoring the right commit under some conditions","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-03-01T10:32:14Z","receivedAt":"2008-03-01T10:32:14Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"Previously, --abort would end by git resetting to ORIG_HEAD, but some\ncommands, such as git reset --hard (which happened in git rebase --skip,\nbut could just as well be typed by the user), modify ORIG_HEAD.\n\nJust use the orig-head we store in $dotest instead.\n---\n\n > > ... and I'm even the one to blame\n > > fb6e4e1f3f048898677f3cf177bfcaf60123bd5c is first bad commit\n >\n > Heh, didn't you say you don't have enough knowledge in git-rebase? ;-)\n\n I'm relieved, I'm not exactly to blame ;) I just exposed the bug that was\n actually already here.\n\n git-rebase.sh           |    5 ++---\n t/t3407-rebase-abort.sh |    2 +-\n 2 files changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex bdcea0e..6b9af96 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -208,16 +208,15 @@ do\n \t\tif test -d \"$dotest\"\n \t\tthen\n \t\t\tmove_to_original_branch\n-\t\t\trm -r \"$dotest\"\n \t\telif test -d .dotest\n \t\tthen\n \t\t\tdotest=.dotest\n \t\t\tmove_to_original_branch\n-\t\t\trm -r .dotest\n \t\telse\n \t\t\tdie \"No rebase in progress?\"\n \t\tfi\n-\t\tgit reset --hard ORIG_HEAD\n+\t\tgit reset --hard $(cat $dotest/orig-head)\n+\t\trm -r \"$dotest\"\n \t\texit\n \t\t;;\n \t--onto)\ndiff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh\nindex 94bdd72..3417138 100755\n--- a/t/t3407-rebase-abort.sh\n+++ b/t/t3407-rebase-abort.sh\n@@ -29,7 +29,7 @@ test_expect_success 'rebase --abort' '\n \ttest $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)\n '\n \n-test_expect_failure 'rebase --abort after --skip' '\n+test_expect_success 'rebase --abort after --skip' '\n \t# Clean up the state from the previous one\n \tgit reset --hard pre-rebase\n \trm -rf .dotest\n-- \n1.5.4.3.343.gb141c.dirty\n\n"},{"id":"70511","messageId":"20080301111156.GA2933@glandium.org","threadId":"12387","inReplyTo":"1204367534-28204-1-git-send-email-mh@glandium.org","subject":"Re: [PATCH] Fix git reset --abort not restoring the right commit under some conditions","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-03-01T11:11:56Z","receivedAt":"2008-03-01T11:11:56Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sat, Mar 01, 2008 at 11:32:14AM +0100, Mike Hommey wrote:\n> Previously, --abort would end by git resetting to ORIG_HEAD, but some\n> commands, such as git reset --hard (which happened in git rebase --skip,\n> but could just as well be typed by the user), modify ORIG_HEAD.\n> \n> Just use the orig-head we store in $dotest instead.\n\nOops, forgot the -s.\nSigned-off-by: Mike Hommey <mh@glandium.org>\n"},{"id":"70571","messageId":"7vmyphx3l3.fsf@gitster.siamese.dyndns.org","threadId":"12387","inReplyTo":"1204367534-28204-1-git-send-email-mh@glandium.org","subject":"Re: [PATCH] Fix git reset --abort not restoring the right commit under some conditions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-02T02:27:36Z","receivedAt":"2008-03-02T02:27:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey <mh@glandium.org> writes:\n\n> Previously, --abort would end by git resetting to ORIG_HEAD, but some\n> commands, such as git reset --hard (which happened in git rebase --skip,\n> but could just as well be typed by the user), modify ORIG_HEAD.\n>\n> Just use the orig-head we store in $dotest instead.\n\nWhat happens if we are not using $dotest but .dotest?\n"},{"id":"70575","messageId":"7vmyphvkup.fsf@gitster.siamese.dyndns.org","threadId":"12387","inReplyTo":"7vmyphx3l3.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix git reset --abort not restoring the right commit under some conditions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-02T03:57:34Z","receivedAt":"2008-03-02T03:57:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Mike Hommey <mh@glandium.org> writes:\n>\n>> Previously, --abort would end by git resetting to ORIG_HEAD, but some\n>> commands, such as git reset --hard (which happened in git rebase --skip,\n>> but could just as well be typed by the user), modify ORIG_HEAD.\n>>\n>> Just use the orig-head we store in $dotest instead.\n>\n> What happens if we are not using $dotest but .dotest?\n\nAh, there is an assignment to that variable in that case.  Ok.\n\nThe script really needs an overhaul someday.  It got unreadable over\ntime.\n"}]}