* [PATCH] Add test for git rebase --abort
@ 2008-02-29 22:08 Mike Hommey
2008-02-29 23:26 ` Junio C Hamano
0 siblings, 1 reply; 12+ messages in thread
From: Mike Hommey @ 2008-02-29 22:08 UTC (permalink / raw)
To: git, gitster
We expect git rebase --abort to come back to the original (pre-rebase)
head, independently from when it's run during a rebase.
Signed-off-by: Mike Hommey <mh@glandium.org>
---
I wrote this test because I spotted a problem with git rebase --abort, and
expanded the test to other cases that work, but that it is still better to
have a test for.
The failing test is the third. I don't have enough knowledge in git-rebase
to write an appropriate fix, but the problem seems to be in
move_to_original_branch, where testing head_name doesn't seem appropriate.
t/t3407-rebase-abort.sh | 59 +++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 59 insertions(+), 0 deletions(-)
create mode 100755 t/t3407-rebase-abort.sh
diff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh
new file mode 100755
index 0000000..abdecc9
--- /dev/null
+++ b/t/t3407-rebase-abort.sh
@@ -0,0 +1,59 @@
+#!/bin/sh
+
+test_description='git rebase --abort tests'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ echo a > a &&
+ git add a &&
+ git commit -m a &&
+ git branch to-rebase &&
+
+ echo b > a &&
+ git commit -a -m b &&
+ echo c > a &&
+ git commit -a -m c &&
+
+ git checkout to-rebase &&
+ echo d > a &&
+ git commit -a -m "merge should fail on this" &&
+ echo e > a &&
+ git commit -a -m "merge should fail on this, too" &&
+ git branch pre-rebase
+'
+
+test_expect_success 'rebase --abort' '
+ ! git rebase master &&
+ git rebase --abort &&
+ test $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)
+'
+
+# In case previous test failed
+git reset --hard pre-rebase >&3 2>&4
+rm -rf .dotest # Should be changed whenever rebase stop using .dotest
+
+test_expect_success 'rebase --abort after --skip' '
+ ! git rebase master &&
+ ! git rebase --skip &&
+ test $(git rev-parse HEAD) = $(git rev-parse master) &&
+ sh -x ../../git-rebase --abort &&
+ test $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)
+'
+
+# In case previous test failed
+git reset --hard pre-rebase >&3 2>&4
+rm -rf .dotest # Should be changed whenever rebase stop using .dotest
+
+test_expect_success 'rebase --abort after --continue' '
+ ! git rebase master &&
+ echo c > a &&
+ echo d >> a &&
+ git add a &&
+ ! git rebase --continue &&
+ test $(git rev-parse HEAD) != $(git rev-parse master) &&
+ git rebase --abort &&
+ test $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)
+'
+
+test_done
--
1.5.4.3.340.g97b97.dirty
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Add test for git rebase --abort
2008-02-29 22:08 [PATCH] Add test for git rebase --abort Mike Hommey
@ 2008-02-29 23:26 ` Junio C Hamano
2008-02-29 23:39 ` Johannes Schindelin
2008-03-01 7:36 ` Mike Hommey
0 siblings, 2 replies; 12+ messages in thread
From: Junio C Hamano @ 2008-02-29 23:26 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
Mike Hommey <mh@glandium.org> writes:
> The failing test is the third. I don't have enough knowledge in git-rebase
> to write an appropriate fix, but the problem seems to be in
> move_to_original_branch, where testing head_name doesn't seem appropriate.
Please mark such an "expected to succeed but fails due to
suspected bug" with test_expect_failure.
> +test_expect_success 'rebase --abort' '
> + ! git rebase master &&
When making sure "git frotz" refuses gracefully (instead of
segfault-and-burn), please say "test_must_fail git frotz".
> +# In case previous test failed
> +git reset --hard pre-rebase >&3 2>&4
> +rm -rf .dotest # Should be changed whenever rebase stop using .dotest
Have this kind of clean-up at the very beginning of the next
test. Test writers should not have to learn about file
descriptors 3 and 4.
Side note. As a test framework extension, we might want
to add 4th parameter to test_expect_{success,failure}
that specifies a clean-up to be made regardless of the
outcome of the test.
> +test_expect_success 'rebase --abort after --skip' '
> + ! git rebase master &&
> + ! git rebase --skip &&
> + test $(git rev-parse HEAD) = $(git rev-parse master) &&
> + sh -x ../../git-rebase --abort &&
> + test $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)
> +'
I take that "sh -x ../../" is not for inclusion in the official
release.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add test for git rebase --abort
2008-02-29 23:26 ` Junio C Hamano
@ 2008-02-29 23:39 ` Johannes Schindelin
2008-03-01 7:36 ` Mike Hommey
1 sibling, 0 replies; 12+ messages in thread
From: Johannes Schindelin @ 2008-02-29 23:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Mike Hommey, git
Hi,
On Fri, 29 Feb 2008, Junio C Hamano wrote:
> Side note. As a test framework extension, we might want
> to add 4th parameter to test_expect_{success,failure}
> that specifies a clean-up to be made regardless of the
> outcome of the test.
I saw something the other night, where the test _I_ introduced, for
git-daemon, failed (with -i -v), and the daemon was _not_ killed as I
expected (with trap "<bla>" 0).
Ciao,
Dscho
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add test for git rebase --abort
2008-02-29 23:26 ` Junio C Hamano
2008-02-29 23:39 ` Johannes Schindelin
@ 2008-03-01 7:36 ` Mike Hommey
2008-03-01 7:41 ` Junio C Hamano
2008-03-01 7:45 ` Mike Hommey
1 sibling, 2 replies; 12+ messages in thread
From: Mike Hommey @ 2008-03-01 7:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Fri, Feb 29, 2008 at 03:26:01PM -0800, Junio C Hamano wrote:
> Mike Hommey <mh@glandium.org> writes:
>
> > The failing test is the third. I don't have enough knowledge in git-rebase
> > to write an appropriate fix, but the problem seems to be in
> > move_to_original_branch, where testing head_name doesn't seem appropriate.
>
> Please mark such an "expected to succeed but fails due to
> suspected bug" with test_expect_failure.
I was kind of expecting the bug would be fixed before the test be
included ;)
> > +test_expect_success 'rebase --abort' '
> > + ! git rebase master &&
>
> When making sure "git frotz" refuses gracefully (instead of
> segfault-and-burn), please say "test_must_fail git frotz".
Ooooh, I just saw 74359821.
> > +# In case previous test failed
> > +git reset --hard pre-rebase >&3 2>&4
> > +rm -rf .dotest # Should be changed whenever rebase stop using .dotest
>
> Have this kind of clean-up at the very beginning of the next
> test. Test writers should not have to learn about file
> descriptors 3 and 4.
>
> Side note. As a test framework extension, we might want
> to add 4th parameter to test_expect_{success,failure}
> that specifies a clean-up to be made regardless of the
> outcome of the test.
>
> > +test_expect_success 'rebase --abort after --skip' '
> > + ! git rebase master &&
> > + ! git rebase --skip &&
> > + test $(git rev-parse HEAD) = $(git rev-parse master) &&
> > + sh -x ../../git-rebase --abort &&
> > + test $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)
> > +'
>
> I take that "sh -x ../../" is not for inclusion in the official
> release.
D'oh, I forgot to change that back.
Mike
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add test for git rebase --abort
2008-03-01 7:36 ` Mike Hommey
@ 2008-03-01 7:41 ` Junio C Hamano
2008-03-01 7:45 ` Mike Hommey
1 sibling, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2008-03-01 7:41 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
Mike Hommey <mh@glandium.org> writes:
> On Fri, Feb 29, 2008 at 03:26:01PM -0800, Junio C Hamano wrote:
> ...
>> Please mark such an "expected to succeed but fails due to
>> suspected bug" with test_expect_failure.
>
> I was kind of expecting the bug would be fixed before the test be
> included ;)
Your patch does not have to go through me to whoever fixes the issue.
People can pick it up from the list, apply them to their tree and start
working on it. The point is to be gentle to them.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add test for git rebase --abort
2008-03-01 7:36 ` Mike Hommey
2008-03-01 7:41 ` Junio C Hamano
@ 2008-03-01 7:45 ` Mike Hommey
2008-03-01 8:15 ` Mike Hommey
1 sibling, 1 reply; 12+ messages in thread
From: Mike Hommey @ 2008-03-01 7:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Sat, Mar 01, 2008 at 08:36:12AM +0100, Mike Hommey wrote:
> On Fri, Feb 29, 2008 at 03:26:01PM -0800, Junio C Hamano wrote:
> > Mike Hommey <mh@glandium.org> writes:
> >
> > > The failing test is the third. I don't have enough knowledge in git-rebase
> > > to write an appropriate fix, but the problem seems to be in
> > > move_to_original_branch, where testing head_name doesn't seem appropriate.
> >
> > Please mark such an "expected to succeed but fails due to
> > suspected bug" with test_expect_failure.
>
> I was kind of expecting the bug would be fixed before the test be
> included ;)
... and the test actually passes with v1.5.0. I'll bisect.
Mike
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add test for git rebase --abort
2008-03-01 7:45 ` Mike Hommey
@ 2008-03-01 8:15 ` Mike Hommey
2008-03-01 9:17 ` Junio C Hamano
0 siblings, 1 reply; 12+ messages in thread
From: Mike Hommey @ 2008-03-01 8:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Sat, Mar 01, 2008 at 08:45:11AM +0100, Mike Hommey wrote:
> On Sat, Mar 01, 2008 at 08:36:12AM +0100, Mike Hommey wrote:
> > On Fri, Feb 29, 2008 at 03:26:01PM -0800, Junio C Hamano wrote:
> > > Mike Hommey <mh@glandium.org> writes:
> > >
> > > > The failing test is the third. I don't have enough knowledge in git-rebase
> > > > to write an appropriate fix, but the problem seems to be in
> > > > move_to_original_branch, where testing head_name doesn't seem appropriate.
> > >
> > > Please mark such an "expected to succeed but fails due to
> > > suspected bug" with test_expect_failure.
> >
> > I was kind of expecting the bug would be fixed before the test be
> > included ;)
>
> ... and the test actually passes with v1.5.0. I'll bisect.
... and I'm even the one to blame
fb6e4e1f3f048898677f3cf177bfcaf60123bd5c is first bad commit
Mike
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add test for git rebase --abort
2008-03-01 8:15 ` Mike Hommey
@ 2008-03-01 9:17 ` Junio C Hamano
2008-03-01 10:32 ` [PATCH] Fix git reset --abort not restoring the right commit under some conditions Mike Hommey
0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2008-03-01 9:17 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
Mike Hommey <mh@glandium.org> writes:
> On Sat, Mar 01, 2008 at 08:45:11AM +0100, Mike Hommey wrote:
>> On Sat, Mar 01, 2008 at 08:36:12AM +0100, Mike Hommey wrote:
>> > On Fri, Feb 29, 2008 at 03:26:01PM -0800, Junio C Hamano wrote:
>> > > Mike Hommey <mh@glandium.org> writes:
>> > >
>> > > > The failing test is the third. I don't have enough knowledge in git-rebase
>> > > > to write an appropriate fix, but the problem seems to be in
>> > > > move_to_original_branch, where testing head_name doesn't seem appropriate.
>> > >
>> > > Please mark such an "expected to succeed but fails due to
>> > > suspected bug" with test_expect_failure.
>> >
>> > I was kind of expecting the bug would be fixed before the test be
>> > included ;)
>>
>> ... and the test actually passes with v1.5.0. I'll bisect.
>
> ... and I'm even the one to blame
> fb6e4e1f3f048898677f3cf177bfcaf60123bd5c is first bad commit
Heh, didn't you say you don't have enough knowledge in git-rebase? ;-)
I've applied the test with the following fixups.
t/t3407-rebase-abort.sh | 28 ++++++++++++++--------------
1 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh
index abdecc9..94bdd72 100755
--- a/t/t3407-rebase-abort.sh
+++ b/t/t3407-rebase-abort.sh
@@ -24,33 +24,33 @@ test_expect_success setup '
'
test_expect_success 'rebase --abort' '
- ! git rebase master &&
+ test_must_fail git rebase master &&
git rebase --abort &&
test $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)
'
-# In case previous test failed
-git reset --hard pre-rebase >&3 2>&4
-rm -rf .dotest # Should be changed whenever rebase stop using .dotest
+test_expect_failure 'rebase --abort after --skip' '
+ # Clean up the state from the previous one
+ git reset --hard pre-rebase
+ rm -rf .dotest
-test_expect_success 'rebase --abort after --skip' '
- ! git rebase master &&
- ! git rebase --skip &&
+ test_must_fail git rebase master &&
+ test_must_fail git rebase --skip &&
test $(git rev-parse HEAD) = $(git rev-parse master) &&
- sh -x ../../git-rebase --abort &&
+ git rebase --abort &&
test $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)
'
-# In case previous test failed
-git reset --hard pre-rebase >&3 2>&4
-rm -rf .dotest # Should be changed whenever rebase stop using .dotest
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] Fix git reset --abort not restoring the right commit under some conditions
2008-03-01 9:17 ` Junio C Hamano
@ 2008-03-01 10:32 ` Mike Hommey
2008-03-01 11:11 ` Mike Hommey
2008-03-02 2:27 ` Junio C Hamano
0 siblings, 2 replies; 12+ messages in thread
From: Mike Hommey @ 2008-03-01 10:32 UTC (permalink / raw)
To: git, gitster
Previously, --abort would end by git resetting to ORIG_HEAD, but some
commands, such as git reset --hard (which happened in git rebase --skip,
but could just as well be typed by the user), modify ORIG_HEAD.
Just use the orig-head we store in $dotest instead.
---
> > ... and I'm even the one to blame
> > fb6e4e1f3f048898677f3cf177bfcaf60123bd5c is first bad commit
>
> Heh, didn't you say you don't have enough knowledge in git-rebase? ;-)
I'm relieved, I'm not exactly to blame ;) I just exposed the bug that was
actually already here.
git-rebase.sh | 5 ++---
t/t3407-rebase-abort.sh | 2 +-
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index bdcea0e..6b9af96 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -208,16 +208,15 @@ do
if test -d "$dotest"
then
move_to_original_branch
- rm -r "$dotest"
elif test -d .dotest
then
dotest=.dotest
move_to_original_branch
- rm -r .dotest
else
die "No rebase in progress?"
fi
- git reset --hard ORIG_HEAD
+ git reset --hard $(cat $dotest/orig-head)
+ rm -r "$dotest"
exit
;;
--onto)
diff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh
index 94bdd72..3417138 100755
--- a/t/t3407-rebase-abort.sh
+++ b/t/t3407-rebase-abort.sh
@@ -29,7 +29,7 @@ test_expect_success 'rebase --abort' '
test $(git rev-parse to-rebase) = $(git rev-parse pre-rebase)
'
-test_expect_failure 'rebase --abort after --skip' '
+test_expect_success 'rebase --abort after --skip' '
# Clean up the state from the previous one
git reset --hard pre-rebase
rm -rf .dotest
--
1.5.4.3.343.gb141c.dirty
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix git reset --abort not restoring the right commit under some conditions
2008-03-01 10:32 ` [PATCH] Fix git reset --abort not restoring the right commit under some conditions Mike Hommey
@ 2008-03-01 11:11 ` Mike Hommey
2008-03-02 2:27 ` Junio C Hamano
1 sibling, 0 replies; 12+ messages in thread
From: Mike Hommey @ 2008-03-01 11:11 UTC (permalink / raw)
To: git, gitster
On Sat, Mar 01, 2008 at 11:32:14AM +0100, Mike Hommey wrote:
> Previously, --abort would end by git resetting to ORIG_HEAD, but some
> commands, such as git reset --hard (which happened in git rebase --skip,
> but could just as well be typed by the user), modify ORIG_HEAD.
>
> Just use the orig-head we store in $dotest instead.
Oops, forgot the -s.
Signed-off-by: Mike Hommey <mh@glandium.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix git reset --abort not restoring the right commit under some conditions
2008-03-01 10:32 ` [PATCH] Fix git reset --abort not restoring the right commit under some conditions Mike Hommey
2008-03-01 11:11 ` Mike Hommey
@ 2008-03-02 2:27 ` Junio C Hamano
2008-03-02 3:57 ` Junio C Hamano
1 sibling, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2008-03-02 2:27 UTC (permalink / raw)
To: Mike Hommey; +Cc: git, gitster
Mike Hommey <mh@glandium.org> writes:
> Previously, --abort would end by git resetting to ORIG_HEAD, but some
> commands, such as git reset --hard (which happened in git rebase --skip,
> but could just as well be typed by the user), modify ORIG_HEAD.
>
> Just use the orig-head we store in $dotest instead.
What happens if we are not using $dotest but .dotest?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix git reset --abort not restoring the right commit under some conditions
2008-03-02 2:27 ` Junio C Hamano
@ 2008-03-02 3:57 ` Junio C Hamano
0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2008-03-02 3:57 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> Mike Hommey <mh@glandium.org> writes:
>
>> Previously, --abort would end by git resetting to ORIG_HEAD, but some
>> commands, such as git reset --hard (which happened in git rebase --skip,
>> but could just as well be typed by the user), modify ORIG_HEAD.
>>
>> Just use the orig-head we store in $dotest instead.
>
> What happens if we are not using $dotest but .dotest?
Ah, there is an assignment to that variable in that case. Ok.
The script really needs an overhaul someday. It got unreadable over
time.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2008-03-02 3:58 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-29 22:08 [PATCH] Add test for git rebase --abort Mike Hommey
2008-02-29 23:26 ` Junio C Hamano
2008-02-29 23:39 ` Johannes Schindelin
2008-03-01 7:36 ` Mike Hommey
2008-03-01 7:41 ` Junio C Hamano
2008-03-01 7:45 ` Mike Hommey
2008-03-01 8:15 ` Mike Hommey
2008-03-01 9:17 ` Junio C Hamano
2008-03-01 10:32 ` [PATCH] Fix git reset --abort not restoring the right commit under some conditions Mike Hommey
2008-03-01 11:11 ` Mike Hommey
2008-03-02 2:27 ` Junio C Hamano
2008-03-02 3:57 ` Junio C Hamano
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).