threads / rfc / 31582

RFC patchadd t3420-rebase-topology

Subject: [RFC PATCH] add t3420-rebase-topology

## tl;dr

75 messages between Sep 18, 2012 and Jun 19, 2013. Diffs are folded; open one to read it.

replies: 74people: 6as markdown or json

Martin von Zweigbergk· Sep 18, 2012, 06:31 UTC · lore

Add more test cases to check that the topology after a rebase is as expected. Conflicts are not considered, but patch-equivalence is. ---

Tests pass and fail as indicated by the suffix (_success/_failure). Your input especially appreciated on whether you agree with the intent of the test cases. For example, do you agree that 'rebase --onto does not re-apply patches in onto' is desirable? And if you do, then do you also agree that 'rebase --root --onto ignores patch in onto' is desirable? How about 'rebase --root is not a no-op'? One might think that --force would be necessary, but on the other hand, if that was the case, the only point (AFAICT) of "git rebase --root <branch>" without --force would be to linearize history, so I instead made the test case confirm that --root without --onto effectively behaves as if --force was also passed.

Feedback on the structure/setup and style is of course also appreciated.

 t/t3420-rebase-topology.sh | 348 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 348 insertions(+)
 create mode 100755 t/t3420-rebase-topology.sh
Show changes to t/t3420-rebase-topology.sh +348 −0
diff --git a/t/t3420-rebase-topology.sh b/t/t3420-rebase-topology.sh
new file mode 100755
index 0000000..024a2b4
--- /dev/null
+++ b/t/t3420-rebase-topology.sh
@@ -0,0 +1,348 @@
+#!/bin/sh
+
+test_description='effect of rebase on topology'
+. ./test-lib.sh
+
+
+#       q---C---r
+#      /
+# a---b---c---d!--e---p
+#      \
+#       f---g!--h
+#        \
+#         j-------E---k
+#          \       \
+#           n---H---w
+#
+# x---y---B
+#
+#
+# ! = empty
+# uppercase = cherry-picked
+# p = reverted e
+#
+# TODO:
+# prune graph to what's needed
+
+empty () {
+	git commit --allow-empty -m $1 &&
+	git tag $1
+}
+
+cherry_pick () {
+	git cherry-pick -n $1 &&
+	git commit -m $2 &&
+	git tag $2
+}
+
+revert () {
+	git revert -n $1 &&
+	git commit -m $2 &&
+	git tag $2
+}
+
+
+test_expect_success 'setup' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	empty d &&
+	test_commit e &&
+	revert e p &&
+	git checkout b &&
+	test_commit f &&
+	empty g &&
+	test_commit h &&
+	git checkout f &&
+	test_commit j &&
+	cherry_pick e E &&
+	test_commit k &&
+	git checkout j &&
+	test_commit n &&
+	cherry_pick h H &&
+	git merge -m w E &&
+	git tag w &&
+	git checkout b &&
+	test_commit q &&
+	cherry_pick c C &&
+	test_commit r &&
+	git checkout --orphan disjoint &&
+	git rm -rf . &&
+	test_commit x &&
+	test_commit y &&
+	cherry_pick b B
+'
+
+reset () {
+	git rebase --abort
+	git reset --hard
+}
+
+test_range () {
+	test "$(git log --reverse --topo-order --format=%s "$1" | xargs)" = "$2"
+}
+
+test_revisions () {
+	expected="$1"
+	shift
+	test "$(git log --format=%s --no-walk=unsorted "$@" | xargs)" = "$expected"
+}
+
+same_revision () {
+	test "$(git rev-parse $1)" = "$(git rev-parse $2)"
+}
+
+# the following 5 (?) tests copy t3400 tests, but check the history rather than status code and/or stdout
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' c j &&
+	same_revision HEAD~2 c &&
+	test_range c.. "f j"
+'
+}
+test_expect_success 'simple rebase' "$(run)"
+test_expect_success 'simple rebase -m' "$(run -m)"
+test_expect_success 'simple rebase -i' "$(run -i)"
+test_expect_success 'simple rebase -p' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' b j &&
+	same_revision HEAD j
+'
+}
+test_expect_success 'rebase is no-op if upstream is an ancestor' "$(run)"
+test_expect_success 'rebase -m is no-op if upstream is an ancestor' "$(run -m)"
+test_expect_success 'rebase -i is no-op if upstream is an ancestor' "$(run -i)"
+test_expect_success 'rebase -p is no-op if upstream is an ancestor' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --force b j &&
+	! same_revision HEAD j &&
+	test_range b.. "f j"
+'
+}
+test_expect_success 'rebase --force' "$(run)"
+test_expect_success 'rebase -m --force' "$(run -m)"
+test_expect_success 'rebase -i --force' "$(run -i)"
+test_expect_failure 'rebase -p --force' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' j b &&
+	same_revision HEAD j
+'
+}
+test_expect_success 'rebase fast-forwards if an ancestor of upstream' "$(run)"
+test_expect_success 'rebase -m fast-forwards if an ancestor of upstream' "$(run -m)"
+test_expect_success 'rebase -i fast-forwards if an ancestor of upstream' "$(run -i)"
+test_expect_success 'rebase -p fast-forwards if an ancestor of upstream' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' p k &&
+	test_range p.. "f j k"
+'
+}
+test_expect_success 'rebase ignores patch in upstream' "$(run)"
+test_expect_failure 'rebase -m ignores patch in upstream' "$(run -m)"
+test_expect_success 'rebase -i ignores patch in upstream' "$(run -i)"
+test_expect_success 'rebase -p ignores patch in upstream' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' c h &&
+	test_range c.. "f h"
+'
+}
+test_expect_success 'rebase ignores empty commit' "$(run)"
+test_expect_success 'rebase -m ignores empty commit' "$(run -m)"
+test_expect_success 'rebase -i ignores empty commit' "$(run -i)"
+test_expect_success 'rebase -p ignores empty commit' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --keep-empty c h &&
+	test_range c.. "f g h"
+'
+}
+test_expect_success 'rebase --keep-empty' "$(run)"
+test_expect_failure 'rebase -m --keep-empty' "$(run -m)"
+test_expect_success 'rebase -i --keep-empty' "$(run -i)"
+test_expect_failure 'rebase -p --keep-empty' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --keep-empty p h &&
+	test_range p.. "f g h"
+'
+}
+test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' "$(run)"
+test_expect_failure 'rebase -m --keep-empty keeps empty even if already in upstream' "$(run -m)"
+test_expect_failure 'rebase -i --keep-empty keeps empty even if already in upstream' "$(run -i)"
+test_expect_failure 'rebase -p --keep-empty keeps empty even if already in upstream' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' E w &&
+	test_range E.. "n H"
+'
+}
+test_expect_success 'rebase after merge' "$(run)"
+test_expect_success 'rebase -m after merge' "$(run -m)"
+test_expect_success 'rebase -i after merge' "$(run -i)"
+
+test_expect_success 'rebase -p is no-op in history with merges' '
+	reset &&
+	git rebase -p j w &&
+	same_revision HEAD w
+'
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' j w &&
+	test_range j.. "E n H" || test_range j.. "n H E"
+'
+}
+test_expect_success 'rebase of history with merges is linearized' "$(run)"
+test_expect_success 'rebase -m of history with merges is linearized' "$(run -m)"
+test_expect_success 'rebase -i of history with merges is linearized' "$(run -i)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --onto p h k &&
+	test_range p.. "j k"
+'
+}
+test_expect_failure 'rebase --onto does not re-apply patches in onto' "$(run)"
+test_expect_failure 'rebase -m --onto does not re-apply patches in onto' "$(run -m)"
+test_expect_failure 'rebase -i --onto does not re-apply patches in onto' "$(run -i)"
+test_expect_failure 'rebase -p --onto does not re-apply patches in onto' "$(run -p)"
+
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --onto f d r &&
+	test_range f.. "q C r"
+'
+}
+test_expect_failure 'rebase --onto does not lose patches in upstream' "$(run)"
+test_expect_success 'rebase -m --onto does not lose patches in upstream' "$(run -m)"
+test_expect_failure 'rebase -i --onto does not lose patches in upstream' "$(run -i)"
+test_expect_failure 'rebase -p --onto does not lose patches in upstream' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --root c &&
+	! same_revision HEAD c &&
+	test_range c "a b c"
+'
+}
+test_expect_success 'rebase --root is not a no-op' "$(run)"
+test_expect_success 'rebase -m --root is not a no-op' "$(run -m)"
+test_expect_success 'rebase -i --root is not a no-op' "$(run -i)"
+test_expect_success 'rebase -p --root is not a no-op' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --root --onto e y &&
+	test_range e.. "x y"
+'
+}
+test_expect_success 'rebase --root --onto' "$(run)"
+test_expect_failure 'rebase -m --root --onto' "$(run -m)"
+test_expect_success 'rebase -i --root --onto' "$(run -i)"
+test_expect_success 'rebase -p --root --onto' "$(run -p)"
+
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --root --onto e B &&
+	test_range e.. "x y"
+'
+}
+test_expect_success 'rebase --root --onto ignores patch in onto' "$(run)"
+test_expect_failure 'rebase -m --root --onto ignores patch in onto' "$(run -m)"
+test_expect_success 'rebase -i --root --onto ignores patch in onto' "$(run -i)"
+test_expect_success 'rebase -p --root --onto ignores patch in onto' "$(run -p)"
+
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' e y &&
+	test_range e.. "x y"
+'
+}
+test_expect_success 'rebase without --root works on disjoint history' "$(run)"
+test_expect_failure 'rebase -m without --root works on disjoint history' "$(run -m)"
+test_expect_success 'rebase -i without --root works on disjoint history' "$(run -i)"
+test_expect_failure 'rebase -p without --root works on disjoint history' "$(run -p)"
+
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --root --onto p k &&
+	test_range p.. "f j k"
+'
+}
+test_expect_success 'rebase --root --onto with merge-base ignores --root' "$(run)"
+test_expect_failure 'rebase -m --root --onto with merge-base ignores --root' "$(run -m)"
+test_expect_success 'rebase -i --root --onto with merge-base ignores --root' "$(run -i)"
+test_expect_success 'rebase -p --root --onto with merge-base ignores --root' "$(run -p)"
+
+test_expect_success 'rebase -p re-creates merge from upstream' '
+	reset &&
+	git rebase -p k w &&
+	same_revision HEAD^ H &&
+	same_revision HEAD^2 k
+'
+
+test_expect_success 'rebase -p re-creates internal merge' '
+	reset &&
+	git rebase -p c w &&
+	test_revisions "f j n E H w" HEAD~4 HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+'
+
+test_expect_success 'rebase -p rebuilds history around dropped commit matching upstream' '
+	reset &&
+	git rebase -p h w &&
+	test_revisions "j E n w" HEAD~2 HEAD^2 HEAD^ HEAD
+'
+
+test_expect_success 'rebase -p drops merge commit when one entire side is dropped' '
+	reset &&
+	git rebase -p p w &&
+	test_range p.. "f j n H"
+'
+
+test_expect_failure 'rebase -p --onto drops commit in <onto>' '
+	reset &&
+	git rebase -p --onto p f w &&
+	test_range p.. "j n H"
+'
+
+test_expect_success 'rebase -p with two paths to $from' '
+	reset &&
+	git rebase -p --onto c j w &&
+	test_revisions "c n E H w" HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+'
+
+test_done
-- 
1.7.11.1.104.ge7b44f1
Junio C Hamano· Sep 18, 2012, 07:51 UTC · re: Martin von Zweigbergk · lore

Re: [RFC PATCH] add t3420-rebase-topology

Martin von Zweigbergk <martinvonz@gmail.com> writes:
> do you agree
> that 'rebase --onto does not re-apply patches in onto' is desirable?

This depends on how you look at --onto. Recall the most typical and the original use case of rebase:

                               A'--C' your rebased work
                              /
  ---o---o---o---F---B'--o---T master
     ^            \
     v1.7.12       A---B---C your work
You could view "git rebase master" as a short-hand for
        $ git rebase --onto $(git merge-base master HEAD) master
i.e.
        $ git rebase --onto F master

If you view "onto" that way, you would certainly do not want to see B replayed on top of T, which already contains another replay of B in its history.

The intended use case for "--onto", however, is primarily to replay a history to older base, i.e.

       A'--B'--C' your rebased work
      /
  ---o---o---o---F---B'--o---T master
     ^            \
     v1.7.12       A---B---C your work
        $ git rebase --onto v1.7.12 master
which is a moral equivalent of
        $ git checkout v1.7.12
        $ git cherry-pick A B C ;# or git cherry-pick master..HEAD

You could argue that you can compute the patch equivalence between the commits in "onto..master" and commits in "master..HEAD" and filter out the equivalent commits to reach the expected results in either cases, and at the logical level, it certainly will produce the result I showed in the above illustrations.

But there are two flaws in such an argument.

The "replay to an updated base" case (i.e. without "--onto") expects to see duplicated commits in the updated history. That is the only reason you would want to rebase---otherwise you can keep the old fork point and keep polishing your work and merge when it is truly ready. More importantly, the old master (the fork point F) and updated master (the current tip T) are expected to have some ancestry relationship between them. For these reasons, running the equivalent of "git cherry" to filter commits with patch equivalence makes perfect sense.

On the other hand, when the user replays to an older base, she has some idea what constitutes "a series" that she is replaying (i.e. "$(git merge-base master HEAD)..HEAD"). It smells to go against the user's world model if the command silently filtered commits by patch equivalence. The user wanted to replay A, B and C in that case, and it is better to stop the process and make her inspect the history when the older base happened to have a patch-equivalent commit that would cause a conflict. That would give her a chance to assess the situation, instead of silently dropping some of these three commits that she initially thought constitutes the series and ending up with a series with only two commits.

Besides, the whole point of a separate "onto" is to allow the user to specify a commit that does not have a straightforward ancestry relationship with the bottom of the series (i.e. either "master" or "F"), and computation of patch equivalence is expected to be much higher. Given that it is unlikely to find any match, it feels doubly wrong to always run "git cherry" equivalent in that case.

So I dunno, as I can argue both ways.

My gut feeling is that "git rebase master" and "git rebase --onto F master" should not be identical in this regard and an explicit "onto" should omit the patch equivalence filtering, but I can be persuaded otherwise with good arguments.

> How about 'rebase --root is not a no-op'?
  ---o---o---o---F---B'--o---T master
     ^            \
     v1.7.12       A---B---C your work

If "git rebase F" when you are at C in the above illustration (reproduced only the relevant parts) is a no-op (and I think it should be), "git rebase --root" in the illustration below ought to be as well, no?

                 F---B'--o---T master
                  \
                   A---B---C your work
Martin von Zweigbergk· Sep 21, 2012, 17:06 UTC · re: Junio C Hamano · lore

Re: [RFC PATCH] add t3420-rebase-topology

On Tue, Sep 18, 2012 at 12:51 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 17 quoted lines
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> do you agree
>> that 'rebase --onto does not re-apply patches in onto' is desirable?
>
> This depends on how you look at --onto.  Recall the most typical and
> the original use case of rebase:
>
>                                A'--C' your rebased work
>                               /
>   ---o---o---o---F---B'--o---T master
>      ^            \
>      v1.7.12       A---B---C your work
>
> You could view "git rebase master" as a short-hand for
>
>         $ git rebase --onto $(git merge-base master HEAD) master

Exactly. I frequently consider it a short-hand for that. It might be worth pointing out that 'git pull --rebase', which might be one of the most frequent uses of rebase, internally often does

  git rebase --onto upstream upstream@{...} branch

where upstream@{...} is the most recent upstream that is an ancestor of "branch". For example, if your work is based on origin/pu and you send the bottom-most patch ("B" in the figure below) to the maintainer and and it gets applied to pu. Running "git pull --rebase" would then lead to "git rebase --onto T A". You would want this to drop B.

                               C' your rebased work
                              /
  ---o---o---o---F---B'--o---T origin/pu
                  \
                   A origin/pu@{1}
                    \
                     B---C your work
Show 5 quoted lines
> The intended use case for "--onto", however, is primarily to replay
> a history to older base, i.e. [...] a moral equivalent of
>
>         $ git checkout v1.7.12
>         $ git cherry-pick A B C ;# or git cherry-pick master..HEAD

Yes, this is the alternative way of looking it at and exactly why I, too, was not sure how it should behave.

> You could argue that you can compute the patch equivalence between
> the commits in "onto..master" and commits in "master..HEAD" and
> filter out the equivalent commits

I'm not sure if you meant "master..onto" rather than "onto..master". Rebase (well, all flavors of rebase but "-m") currently drops patches from "master..HEAD" that are also in "HEAD..master". This is what the "rebase --onto does not lose patches in upstream" test is about. It is also one of the main problems that I try to fix in my long-stalled rebase-range series. I think we should drop patches in "master..HEAD" that are also in "HEAD..onto" (which is almost the same as "master..onto").

> The "replay to an updated base" case (i.e. without "--onto")
Or _with_ --onto as in the above example from "git pull --rebase".
Show 5 quoted lines
> On the other hand, when the user replays to an older base, she has
> some idea what constitutes "a series" that she is replaying (i.e.
> "$(git merge-base master HEAD)..HEAD").  It smells to go against the
> user's world model if the command silently filtered commits by patch
> equivalence.

If it's truly about rebasing onto an older base, there can't possibly be any patches in "HEAD..onto", so assuming you agree with my reasoning above that those are the patches we should drop, rebasing onto older history would be safe.

Show 6 quoted lines
> Besides, the whole point of a separate "onto" is to allow the user
> to specify a commit that does not have a straightforward ancestry
> relationship with the bottom of the series (i.e. either "master" or
> "F"), and computation of patch equivalence is expected to be much
> higher.  Given that it is unlikely to find any match, it feels
> doubly wrong to always run "git cherry" equivalent in that case.

Yes, this was my only concern (apart from it possibly being conceptually wrong to do, depending on what the user meant by issuing the command).

Show 14 quoted lines
>> How about 'rebase --root is not a no-op'?
>
>   ---o---o---o---F---B'--o---T master
>      ^            \
>      v1.7.12       A---B---C your work
>
> If "git rebase F" when you are at C in the above illustration
> (reproduced only the relevant parts) is a no-op (and I think it
> should be), "git rebase --root" in the illustration below ought to
> be as well, no?
>
>                  F---B'--o---T master
>                   \
>                    A---B---C your work

Yeah, that's what I thought as well at first. I think my test case even started out as "rebase --root _is_ a no-op".

When thinking about how to handle roots in general, I often imagine a single virtual root commit (parent of all "initial" commits), and that reasoning also implies that "git rebase --root" should be a no-op. Then I saw that the test case failed (or perhaps I remembered how it is implemented with the clever fake root/initial commit) and started thinking about why anyone would use "git rebase --root" if it was a no-op. I could only think of using it to linearize history, but that doesn't seem like a very likely use case. So it seems like weighing purity/correctness against usefulness to me. I'm not sure which way to go.

Thanks for quick and detailed feedback on an RFC patch.
Martin
Johannes Sixt· Sep 18, 2012, 07:53 UTC · re: Martin von Zweigbergk · lore

Re: [RFC PATCH] add t3420-rebase-topology

Am 9/18/2012 8:31, schrieb Martin von Zweigbergk:
Show 56 quoted lines
> Add more test cases to check that the topology after a rebase is as
> expected. Conflicts are not considered, but patch-equivalence is.
> ---
> 
> Tests pass and fail as indicated by the suffix
> (_success/_failure). Your input especially appreciated on whether you
> agree with the intent of the test cases. For example, do you agree
> that 'rebase --onto does not re-apply patches in onto' is desirable?
> And if you do, then do you also agree that 'rebase --root --onto
> ignores patch in onto' is desirable? How about 'rebase --root is not a
> no-op'? One might think that --force would be necessary, but on the
> other hand, if that was the case, the only point (AFAICT) of "git
> rebase --root <branch>" without --force would be to linearize history,
> so I instead made the test case confirm that --root without --onto
> effectively behaves as if --force was also passed.
> 
> Feedback on the structure/setup and style is of course also
> appreciated.
> 
>  t/t3420-rebase-topology.sh | 348 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 348 insertions(+)
>  create mode 100755 t/t3420-rebase-topology.sh
> 
> diff --git a/t/t3420-rebase-topology.sh b/t/t3420-rebase-topology.sh
> new file mode 100755
> index 0000000..024a2b4
> --- /dev/null
> +++ b/t/t3420-rebase-topology.sh
> @@ -0,0 +1,348 @@
> +#!/bin/sh
> +
> +test_description='effect of rebase on topology'
> +. ./test-lib.sh
> +
> +
> +#       q---C---r
> +#      /
> +# a---b---c---d!--e---p
> +#      \
> +#       f---g!--h
> +#        \
> +#         j-------E---k
> +#          \       \
> +#           n---H---w
> +#
> +# x---y---B
> +#
> +#
> +# ! = empty
> +# uppercase = cherry-picked
> +# p = reverted e
> +#
> +# TODO:
> +# prune graph to what's needed
> +
> +empty () {
Is it "is_empty" or "make_empty"/"empty_commit"?
> +	git commit --allow-empty -m $1 &&
> +	git tag $1
> +}
Obviously the latter.
Show 49 quoted lines
> +
> +cherry_pick () {
> +	git cherry-pick -n $1 &&
> +	git commit -m $2 &&
> +	git tag $2
> +}
> +
> +revert () {
> +	git revert -n $1 &&
> +	git commit -m $2 &&
> +	git tag $2
> +}
> +
> +
> +test_expect_success 'setup' '
> +	test_commit a &&
> +	test_commit b &&
> +	test_commit c &&
> +	empty d &&
> +	test_commit e &&
> +	revert e p &&
> +	git checkout b &&
> +	test_commit f &&
> +	empty g &&
> +	test_commit h &&
> +	git checkout f &&
> +	test_commit j &&
> +	cherry_pick e E &&
> +	test_commit k &&
> +	git checkout j &&
> +	test_commit n &&
> +	cherry_pick h H &&
> +	git merge -m w E &&
> +	git tag w &&
> +	git checkout b &&
> +	test_commit q &&
> +	cherry_pick c C &&
> +	test_commit r &&
> +	git checkout --orphan disjoint &&
> +	git rm -rf . &&
> +	test_commit x &&
> +	test_commit y &&
> +	cherry_pick b B
> +'
> +
> +reset () {
> +	git rebase --abort
> +	git reset --hard
> +}

The 'rebase --abort' can fail, but there is no && chain. Good. Using this function instead of the individual commands in the tests does not break the && chain there. Good.

These following three functions should be and can be consistent with the order of their arguments: expected actual.

> +test_range () {
> +	test "$(git log --reverse --topo-order --format=%s "$1" | xargs)" = "$2"
	expected=$1
	set -- $(git log --reverse --topo-order --format=%s "$2")
	test "expected" = "$*"
Show 6 quoted lines
> +}
> +
> +test_revisions () {
> +	expected="$1"
> +	shift
> +	test "$(git log --format=%s --no-walk=unsorted "$@" | xargs)" = "$expected"
	set -- $(git log --format=%s --no-walk=unsorted "$@")
	test "expected" = "$*"
Show 5 quoted lines
> +}
> +
> +same_revision () {
> +	test "$(git rev-parse $1)" = "$(git rev-parse $2)"
> +}
'test_same_revision'?
Show 14 quoted lines
> +
> +# the following 5 (?) tests copy t3400 tests, but check the history rather than status code and/or stdout
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' c j &&
> +	same_revision HEAD~2 c &&
> +	test_range c.. "f j"
> +'
> +}
> +test_expect_success 'simple rebase' "$(run)"
> +test_expect_success 'simple rebase -m' "$(run -m)"
> +test_expect_success 'simple rebase -i' "$(run -i)"
> +test_expect_success 'simple rebase -p' "$(run -p)"

Since here and in the following tests the test cases and test descriptions vary in the same way, wouldn't it make sense to factor the description out as well?

test_run_rebase () {
	test_expect_success "simple rebase $*" "
		reset &&
		git rebase $* c j &&
		same_revision HEAD~2 c &&
		test_range c.. 'f j'
	"
}

test_run_rebase '' test_run_rebase -m test_run_rebase -i test_run_rebase -p

(Watch your quoting, though.)
Oh, I see: some tests expect failure. How about:
test_run_rebase () {
	result=$1
	shift
	test_expect_$result "simple rebase $*" "..."
}
test_run_rebase success ''
etc.
Show 12 quoted lines
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' b j &&
> +	same_revision HEAD j
> +'
> +}
> +test_expect_success 'rebase is no-op if upstream is an ancestor' "$(run)"
> +test_expect_success 'rebase -m is no-op if upstream is an ancestor' "$(run -m)"
> +test_expect_success 'rebase -i is no-op if upstream is an ancestor' "$(run -i)"
> +test_expect_success 'rebase -p is no-op if upstream is an ancestor' "$(run -p)"
OK.
Show 13 quoted lines
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --force b j &&
> +	! same_revision HEAD j &&
> +	test_range b.. "f j"
> +'
> +}
> +test_expect_success 'rebase --force' "$(run)"
> +test_expect_success 'rebase -m --force' "$(run -m)"
> +test_expect_success 'rebase -i --force' "$(run -i)"
> +test_expect_failure 'rebase -p --force' "$(run -p)"

Do you mean "rebase --force rewrites even if upstream is an ancestor"? That would make sense.

Show 12 quoted lines
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' j b &&
> +	same_revision HEAD j
> +'
> +}
> +test_expect_success 'rebase fast-forwards if an ancestor of upstream' "$(run)"
> +test_expect_success 'rebase -m fast-forwards if an ancestor of upstream' "$(run -m)"
> +test_expect_success 'rebase -i fast-forwards if an ancestor of upstream' "$(run -i)"
> +test_expect_success 'rebase -p fast-forwards if an ancestor of upstream' "$(run -p)"
OK.
Show 12 quoted lines
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' p k &&
> +	test_range p.. "f j k"
> +'
> +}
> +test_expect_success 'rebase ignores patch in upstream' "$(run)"
> +test_expect_failure 'rebase -m ignores patch in upstream' "$(run -m)"
> +test_expect_success 'rebase -i ignores patch in upstream' "$(run -i)"
> +test_expect_success 'rebase -p ignores patch in upstream' "$(run -p)"
"drops" instead of "ignores"? OK.
Show 12 quoted lines
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' c h &&
> +	test_range c.. "f h"
> +'
> +}
> +test_expect_success 'rebase ignores empty commit' "$(run)"
> +test_expect_success 'rebase -m ignores empty commit' "$(run -m)"
> +test_expect_success 'rebase -i ignores empty commit' "$(run -i)"
> +test_expect_success 'rebase -p ignores empty commit' "$(run -p)"
"drops" instead of "ignores"? OK.
Show 12 quoted lines
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --keep-empty c h &&
> +	test_range c.. "f g h"
> +'
> +}
> +test_expect_success 'rebase --keep-empty' "$(run)"
> +test_expect_failure 'rebase -m --keep-empty' "$(run -m)"
> +test_expect_success 'rebase -i --keep-empty' "$(run -i)"
> +test_expect_failure 'rebase -p --keep-empty' "$(run -p)"
OK.
Show 12 quoted lines
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --keep-empty p h &&
> +	test_range p.. "f g h"
> +'
> +}
> +test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' "$(run)"
> +test_expect_failure 'rebase -m --keep-empty keeps empty even if already in upstream' "$(run -m)"
> +test_expect_failure 'rebase -i --keep-empty keeps empty even if already in upstream' "$(run -i)"
> +test_expect_failure 'rebase -p --keep-empty keeps empty even if already in upstream' "$(run -p)"

"is in upstream" is decided by the patch text. If an empty commit is already in upstream, this adds another one with the same or a different commit message and authorship information. Dubious, but since it is opt-in, it should be OK.

Show 11 quoted lines
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' E w &&
> +	test_range E.. "n H"
> +'
> +}
> +test_expect_success 'rebase after merge' "$(run)"
> +test_expect_success 'rebase -m after merge' "$(run -m)"
> +test_expect_success 'rebase -i after merge' "$(run -i)"
OK.
Show 13 quoted lines
> +
> +test_expect_success 'rebase -p is no-op in history with merges' '
> +	reset &&
> +	git rebase -p j w &&
> +	same_revision HEAD w
> +'
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' j w &&
> +	test_range j.. "E n H" || test_range j.. "n H E"
> +'

Chaining tests with || is dangerous: you do not know whether the first failed because the condition is not satisfied or because of some other failure.

Why is this needed in the first place? Shouldn't the history be deterministic, provided that the commit timestamps are all distinct?

> +}
> +test_expect_success 'rebase of history with merges is linearized' "$(run)"
> +test_expect_success 'rebase -m of history with merges is linearized' "$(run -m)"
> +test_expect_success 'rebase -i of history with merges is linearized' "$(run -i)"
OK.
Show 12 quoted lines
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --onto p h k &&
> +	test_range p.. "j k"
> +'
> +}
> +test_expect_failure 'rebase --onto does not re-apply patches in onto' "$(run)"
> +test_expect_failure 'rebase -m --onto does not re-apply patches in onto' "$(run -m)"
> +test_expect_failure 'rebase -i --onto does not re-apply patches in onto' "$(run -i)"
> +test_expect_failure 'rebase -p --onto does not re-apply patches in onto' "$(run -p)"
Makes sense.
Show 13 quoted lines
> +
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --onto f d r &&
> +	test_range f.. "q C r"
> +'
> +}
> +test_expect_failure 'rebase --onto does not lose patches in upstream' "$(run)"
> +test_expect_success 'rebase -m --onto does not lose patches in upstream' "$(run -m)"
> +test_expect_failure 'rebase -i --onto does not lose patches in upstream' "$(run -i)"
> +test_expect_failure 'rebase -p --onto does not lose patches in upstream' "$(run -p)"
Makes sense.
Show 13 quoted lines
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --root c &&
> +	! same_revision HEAD c &&
> +	test_range c "a b c"
> +'
> +}
> +test_expect_success 'rebase --root is not a no-op' "$(run)"
> +test_expect_success 'rebase -m --root is not a no-op' "$(run -m)"
> +test_expect_success 'rebase -i --root is not a no-op' "$(run -i)"
> +test_expect_success 'rebase -p --root is not a no-op' "$(run -p)"
Why? Is it more like "--root implies --force"?
Show 12 quoted lines
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --root --onto e y &&
> +	test_range e.. "x y"
> +'
> +}
> +test_expect_success 'rebase --root --onto' "$(run)"
> +test_expect_failure 'rebase -m --root --onto' "$(run -m)"
> +test_expect_success 'rebase -i --root --onto' "$(run -i)"
> +test_expect_success 'rebase -p --root --onto' "$(run -p)"

Where does this rebase start? Ah, --root stands in for the "upstream" argument, hence, y is the tip to rebase. Right? Then it makes sense.

Show 13 quoted lines
> +
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --root --onto e B &&
> +	test_range e.. "x y"
> +'
> +}
> +test_expect_success 'rebase --root --onto ignores patch in onto' "$(run)"
> +test_expect_failure 'rebase -m --root --onto ignores patch in onto' "$(run -m)"
> +test_expect_success 'rebase -i --root --onto ignores patch in onto' "$(run -i)"
> +test_expect_success 'rebase -p --root --onto ignores patch in onto' "$(run -p)"
Makes sense.
Show 13 quoted lines
> +
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' e y &&
> +	test_range e.. "x y"
> +'
> +}
> +test_expect_success 'rebase without --root works on disjoint history' "$(run)"
> +test_expect_failure 'rebase -m without --root works on disjoint history' "$(run -m)"
> +test_expect_success 'rebase -i without --root works on disjoint history' "$(run -i)"
> +test_expect_failure 'rebase -p without --root works on disjoint history' "$(run -p)"
OK.
Show 13 quoted lines
> +
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --root --onto p k &&
> +	test_range p.. "f j k"
> +'
> +}
> +test_expect_success 'rebase --root --onto with merge-base ignores --root' "$(run)"
> +test_expect_failure 'rebase -m --root --onto with merge-base ignores --root' "$(run -m)"
> +test_expect_success 'rebase -i --root --onto with merge-base ignores --root' "$(run -i)"
> +test_expect_success 'rebase -p --root --onto with merge-base ignores --root' "$(run -p)"
Makes sense.
Show 7 quoted lines
> +
> +test_expect_success 'rebase -p re-creates merge from upstream' '
> +	reset &&
> +	git rebase -p k w &&
> +	same_revision HEAD^ H &&
> +	same_revision HEAD^2 k
> +'
IMO, this tests the wrong thing. You have this history:
 ---j-------E---k
     \       \
      n---H---w

where E is the second parent of w. What does it mean to rebase w onto k? IMO, it is a meaningless operation, and the outcome is irrelevant.

It would make sense to test that this history results after the upstream at H moved forward:

 ---j-------E---k
     \       \
      n---H   \
           \   \
            z---w'

That is, w began a topic by mergeing the sidebranch E; then upstream advanced to z, and now you rebase the topic to the new upstream.

Show 5 quoted lines
> +
> +test_expect_success 'rebase -p re-creates internal merge' '
> +	reset &&
> +	git rebase -p c w &&
> +	test_revisions "f j n E H w" HEAD~4 HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD

You must also test for c; otherwise the test would succeed if rebase did nothing at all.

This comment applies to all other tests as well, even the "regular" rebase tests above. (But I noticed only when I read this test.)

Show 25 quoted lines
> +'
> +
> +test_expect_success 'rebase -p rebuilds history around dropped commit matching upstream' '
> +	reset &&
> +	git rebase -p h w &&
> +	test_revisions "j E n w" HEAD~2 HEAD^2 HEAD^ HEAD
> +'
> +
> +test_expect_success 'rebase -p drops merge commit when one entire side is dropped' '
> +	reset &&
> +	git rebase -p p w &&
> +	test_range p.. "f j n H"
> +'
> +
> +test_expect_failure 'rebase -p --onto drops commit in <onto>' '
> +	reset &&
> +	git rebase -p --onto p f w &&
> +	test_range p.. "j n H"
> +'
> +
> +test_expect_success 'rebase -p with two paths to $from' '
> +	reset &&
> +	git rebase -p --onto c j w &&
> +	test_revisions "c n E H w" HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
> +'
I didn't look at these remaining -p tests because I ran out of time.

After this plethora of tests, can we get rid of some or many from other test scripts? (t34* tests are the ones that take the longest on Windows to run.)

A nice summary of the rebase behavior. I like it.
-- Hannes
Martin von Zweigbergk· Sep 26, 2012, 17:07 UTC · re: Johannes Sixt · lore

Re: [RFC PATCH] add t3420-rebase-topology

[+Chris Webb regarding "git rebase --root"]
First of all, thanks for a meticulous review!
On Tue, Sep 18, 2012 at 12:53 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
Show 5 quoted lines
> Am 9/18/2012 8:31, schrieb Martin von Zweigbergk:
>
> Since here and in the following tests the test cases and test descriptions
> vary in the same way, wouldn't it make sense to factor the description out
> as well?

Definitely. I just couldn't think of a good way of doing it, so thanks for great and concrete suggestions!

> (Watch your quoting, though.)

Switched to putting the test body in double quotes as you suggested in your examples and used single quotes for strings within the test body.

Show 16 quoted lines
>> +run () {
>> +echo '
>> +     reset &&
>> +     git rebase '"$@"' --keep-empty p h &&
>> +     test_range p.. "f g h"
>> +'
>> +}
>> +test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' "$(run)"
>> +test_expect_failure 'rebase -m --keep-empty keeps empty even if already in upstream' "$(run -m)"
>> +test_expect_failure 'rebase -i --keep-empty keeps empty even if already in upstream' "$(run -i)"
>> +test_expect_failure 'rebase -p --keep-empty keeps empty even if already in upstream' "$(run -p)"
>
> "is in upstream" is decided by the patch text. If an empty commit is
> already in upstream, this adds another one with the same or a different
> commit message and authorship information. Dubious, but since it is
> opt-in, it should be OK.

Yes, it is a little dubious. See http://thread.gmane.org/gmane.comp.version-control.git/203097/focus=203159 and Junio's answer, which I think makes sense.

Show 10 quoted lines
>> +run () {
>> +echo '
>> +     reset &&
>> +     git rebase '"$@"' j w &&
>> +     test_range j.. "E n H" || test_range j.. "n H E"
>> +'
>
> Chaining tests with || is dangerous: you do not know whether the first
> failed because the condition is not satisfied or because of some other
> failure.
Good point. Thanks.
> Why is this needed in the first place? Shouldn't the history be
> deterministic, provided that the commit timestamps are all distinct?

It may be deterministic, but it's not specified, I think, so I didn't want to depend on the order. Thinking more about it, though, I think it's good to protect the current behavior from patches that change the order of the parents. Although it may not be incorrect to change the order, it would at least protect against accidental changes.

It turns out that "rebase -i" goes through the commits in --topo-order, while the others use default order, I think. Which flavor should pass the test case and which should fail (and be fixed)? I would personally prefer to say that "rebase -i" is correct in using --topo-order and that the others should be fixed. Again, it's not specified, but I would hate to have them behave differently.

Show 14 quoted lines
>> +run () {
>> +echo '
>> +     reset &&
>> +     git rebase '"$@"' --root c &&
>> +     ! same_revision HEAD c &&
>> +     test_range c "a b c"
>> +'
>> +}
>> +test_expect_success 'rebase --root is not a no-op' "$(run)"
>> +test_expect_success 'rebase -m --root is not a no-op' "$(run -m)"
>> +test_expect_success 'rebase -i --root is not a no-op' "$(run -i)"
>> +test_expect_success 'rebase -p --root is not a no-op' "$(run -p)"
>
> Why? Is it more like "--root implies --force"?

It doesn't currently exactly imply --force, but the effect is the same. Also see my reply to Junio's email in this thread.

Maybe Chris has some thoughts on this?
Show 14 quoted lines
>> +run () {
>> +echo '
>> +     reset &&
>> +     git rebase '"$@"' --root --onto e y &&
>> +     test_range e.. "x y"
>> +'
>> +}
>> +test_expect_success 'rebase --root --onto' "$(run)"
>> +test_expect_failure 'rebase -m --root --onto' "$(run -m)"
>> +test_expect_success 'rebase -i --root --onto' "$(run -i)"
>> +test_expect_success 'rebase -p --root --onto' "$(run -p)"
>
> Where does this rebase start? Ah, --root stands in for the "upstream"
> argument, hence, y is the tip to rebase. Right? Then it makes sense.

Thanks for pointing that out. I changed the order to "git rebase --onto e --root y". I hope that makes it clearer.

Show 27 quoted lines
>> +test_expect_success 'rebase -p re-creates merge from upstream' '
>> +     reset &&
>> +     git rebase -p k w &&
>> +     same_revision HEAD^ H &&
>> +     same_revision HEAD^2 k
>> +'
>
> IMO, this tests the wrong thing. You have this history:
>
>  ---j-------E---k
>      \       \
>       n---H---w
>
> where E is the second parent of w. What does it mean to rebase w onto k?
> IMO, it is a meaningless operation, and the outcome is irrelevant.
>
> It would make sense to test that this history results after the upstream
> at H moved forward:
>
>  ---j-------E---k
>      \       \
>       n---H   \
>            \   \
>             z---w'
>
> That is, w began a topic by mergeing the sidebranch E; then upstream
> advanced to z, and now you rebase the topic to the new upstream.
Fair enough. Changed accordingly.
Show 10 quoted lines
>> +test_expect_success 'rebase -p re-creates internal merge' '
>> +     reset &&
>> +     git rebase -p c w &&
>> +     test_revisions "f j n E H w" HEAD~4 HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
>
> You must also test for c; otherwise the test would succeed if rebase did
> nothing at all.
>
> This comment applies to all other tests as well, even the "regular" rebase
> tests above. (But I noticed only when I read this test.)

I did this only in one or two places thinking that that would be enough to make sure that rebase is not normally a no-op. But I think you are right that we should check it most of the time. It turns out that doing this caught a case where the rebase did do something and the right patches were in "c.." (or whatever it was; I forgot which test case), but the new base was not "c".

> After this plethora of tests, can we get rid of some or many from other
> test scripts? (t34* tests are the ones that take the longest on Windows to
> run.)

I was afraid that this file would be the slowest of all and it might very well be :-(. But, yes, it does replace a few test cases. I will send out an updated version of the patch later. That version should delete a few existing test cases as well.

I am having trouble finding enough time to get the patch into shape, but I didn't want to put off this reply for any longer.

Martin
Chris Webb· Sep 27, 2012, 12:20 UTC · re: Martin von Zweigbergk · lore

Re: [RFC PATCH] add t3420-rebase-topology

Martin von Zweigbergk <martinvonz@gmail.com> writes:
Show 8 quoted lines
> On Tue, Sep 18, 2012 at 12:53 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>
> > Why? Is it more like "--root implies --force"?
> 
> It doesn't currently exactly imply --force, but the effect is the
> same. Also see my reply to Junio's email in this thread.
> 
> Maybe Chris has some thoughts on this?
Hi Martin and Johannes. Sorry for the slow follow-up here.

You're right that rebase --root without --onto always creates a brand new root as a result of the implementation using a sentinel commit. Clearly this is what's wanted with --interactive, but rebase --root with neither --onto nor --interactive is a slightly odd combination for which I struggle to imagine a natural use. Perhaps you're right that for consistency it should be a no-op unless --force-rebase is given?

If we did this, this combination would be a no-op unconditionally as by definition we're always descended from the root of our current commit. However, given the not-very-useful behaviour, I suspect that rebase --root is much more likely to be a mistyped version of rebase -i --root than rebase --root --force-rebase. (Unless I'm missing a reasonable use for this? History linearisation perhaps?)

Best wishes,
Chris.
Martin von Zweigbergk· Sep 28, 2012, 18:03 UTC · re: Chris Webb · lore

Re: [RFC PATCH] add t3420-rebase-topology

On Thu, Sep 27, 2012 at 5:20 AM, Chris Webb <chris@arachsys.com> wrote:
> You're right that rebase --root without --onto always creates a brand new
> root as a result of the implementation using a sentinel commit. Clearly this
> is what's wanted with --interactive

That's not as clear as one might first think. "git rebase -i" actually honors --force; if one marks a commit for "edit", but then --continue without making any changes, the next commit(s) will be fast-forwarded. See the first few lines of pick_one (or pick_one_preserving_merges).

Show 7 quoted lines
> but rebase --root with neither --onto
> nor --interactive is a slightly odd combination for which I struggle to
> imagine a natural use. Perhaps you're right that for consistency it should
> be a no-op unless --force-rebase is given?
>
> If we did this, this combination would be a no-op unconditionally as by
> definition we're always descended from the root of our current commit.

For consistency, it seems like "git rebase -p --root" should always be a no-op, while "git rebase [-i/-m] --root" should be no-op if the history has no merges. Also, since "git rebase -i" tries to fast-forward through existing commits, it seems like "git rebase -i --root" should ideally not create a sentinel commit, but instead stop at the first commit marked for editing.

If, OTOH, --force-rebase is given, we should rewrite history from the first commit, which in the case of --root would mean creating a sentinel commit.

So, in short, I have a feeling that the sentinel commit should be created if and only if both --root and --force-rebase (but not --onto) are given.

> However, given the not-very-useful behaviour, I suspect that rebase --root
> is much more likely to be a mistyped version of rebase -i --root than rebase
> --root --force-rebase. (Unless I'm missing a reasonable use for this?
> History linearisation perhaps?)

Yes, the "not-very-useful"-ness of this is the clear argument against making them no-ops. But I have to say I was slightly surprised when I tried "git rebase --root" for the first time and it created completely new history for me. As you say, "git rebase --root" is probably often a mistyped "git rebase -i --root", and if that is the case, it seems nicer (in addition to being more consistent) if we don't do anything rather than rewriting the history. The history rewriting might even go unnoticed and come as an unpleasant surprise later.

When working on a new project, "git rebase -i --root" might even be a convenient replacement for "git rebase -i <initial commit>" even when one does not want to rewrite the initial commit itself, and in such a case, the user would clearly not want a sentinel commit either.

So I'm getting more and more convinced that the sentinel commit should only be created if --force-rebase was given. Let me know if I'm missing something.

Martin
Chris Webb· Sep 29, 2012, 08:08 UTC · re: Martin von Zweigbergk · lore

Re: [RFC PATCH] add t3420-rebase-topology

Martin von Zweigbergk <martinvonz@gmail.com> writes:
Show 14 quoted lines
> For consistency, it seems like "git rebase -p --root" should always be
> a no-op, while "git rebase [-i/-m] --root" should be no-op if the
> history has no merges. Also, since "git rebase -i" tries to
> fast-forward through existing commits, it seems like "git rebase -i
> --root" should ideally not create a sentinel commit, but instead stop
> at the first commit marked for editing.
> 
> If, OTOH, --force-rebase is given, we should rewrite history from the
> first commit, which in the case of --root would mean creating a
> sentinel commit.
> 
> So, in short, I have a feeling that the sentinel commit should be
> created if and only if both --root and --force-rebase (but not --onto)
> are given.
[...]
> So I'm getting more and more convinced that the sentinel commit should
> only be created if --force-rebase was given. Let me know if I'm
> missing something.

No, that sounds fairly convincing to me. Personally, the only behaviour I want to be able to get at without --force-rebase is for rebase -i --root to allow the root commit to be dropped, edited or reworded, and commits to reordered to make a different one the root, in the same way as normal interactive rebase does for later commits.

The least intrusive implementation for rebase --root was the unconditional sentinel, but as you say, you don't need it (and it's a bit surprising) if the root commit on the instruction sheet is the same as the original root: in the edit/reword case, you could just checkout the existing root and then drop out in the normal way. You only really need the sentinel to deal with the possibility of a conflict if the root is replaced with a different commit.

I think I agree with you it would be better only to create the sentinel on demand when it's required or if --force is given.

Cheers,
Chris.
Martin von Zweigbergk· May 29, 2013, 06:39 UTC · re: Martin von Zweigbergk · lore

[PATCH v2 0/7] Rebase topology test

After way too long, here is finally a new version of the tests I sent at: http://thread.gmane.org/gmane.comp.version-control.git/205796.

I have split the test up into two files. They stil take quite some time to run.

Martin von Zweigbergk (7):
  add simple tests of consistency across rebase types
  add tests for rebasing with patch-equivalence present
  add tests for rebasing of empty commits
  add tests for rebasing root
  add tests for rebasing merged history
  t3406: modernize style
  tests: move test for rebase messages from t3400 to t3406
 t/lib-rebase.sh                   |  32 ++++
 t/t3400-rebase.sh                 |  53 +-----
 t/t3401-rebase-partial.sh         |  69 --------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3406-rebase-message.sh         |  50 +++---
 t/t3409-rebase-preserve-merges.sh |  53 ------
 t/t3420-rebase-topology-linear.sh | 350 ++++++++++++++++++++++++++++++++++++++
 t/t3425-rebase-topology-merges.sh | 250 +++++++++++++++++++++++++++
 8 files changed, 664 insertions(+), 203 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3420-rebase-topology-linear.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh
-- 
1.8.2.674.gd17d3d2
Martin von Zweigbergk· May 29, 2013, 06:39 UTC · re: Martin von Zweigbergk · lore

[PATCH v2 1/7] add simple tests of consistency across rebase types

Helped-by: Johannes Sixt <j6t@kdbg.org>
---
 t/lib-rebase.sh                   | 15 ++++++++
 t/t3420-rebase-topology-linear.sh | 78 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)
 create mode 100755 t/t3420-rebase-topology-linear.sh
Show changes to 2 files +93 −0

t/lib-rebase.sh, t/t3420-rebase-topology-linear.sh

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6ccf797..62b3887 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -65,3 +65,18 @@ EOF
 	test_set_editor "$(pwd)/fake-editor.sh"
 	chmod a+x fake-editor.sh
 }
+
+# checks that the revisions in "$2" represent a linear range with the
+# subjects in "$1"
+test_linear_range () {
+	! { git log --format=%p "$2" | sane_grep " " ;} &&
+	expected=$1
+	set -- $(git log --reverse --format=%s "$2")
+	test "$expected" = "$*"
+}
+
+reset_rebase () {
+	git rebase --abort # may fail; ignore exit code
+	git reset --hard &&
+	git clean -f
+}
diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
new file mode 100755
index 0000000..c4b32db
--- /dev/null
+++ b/t/t3420-rebase-topology-linear.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='basic rebase topology tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+# a---b---c
+#      \
+#       d---e
+test_expect_success 'setup' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "simple rebase $*" "
+		reset_rebase &&
+		git rebase $* c e &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd e' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* is no-op if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* b e &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f rewrites even if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* -f b e &&
+		! test_cmp_rev e HEAD &&
+		test_cmp_rev b HEAD~2 &&
+		test_linear_range 'd e' b..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* fast-forwards if an ancestor of upstream" "
+		reset_rebase &&
+		git rebase $* e b &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_done
-- 
1.8.2.674.gd17d3d2
Martin von Zweigbergk· Jun 3, 2013, 17:16 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v2 1/7] add simple tests of consistency across rebase types

On Tue, May 28, 2013 at 11:39 PM, Martin von Zweigbergk <martinvonz@gmail.com> wrote:

>  create mode 100755 t/t3420-rebase-topology-linear.sh

Just FYI, there's another test case with the same number (t3420-rebase-autostash) in pu. I don't know how you normally handle such cases.

Junio C Hamano· Jun 3, 2013, 18:05 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v2 1/7] add simple tests of consistency across rebase types

Martin von Zweigbergk <martinvonz@gmail.com> writes:
Show 7 quoted lines
> On Tue, May 28, 2013 at 11:39 PM, Martin von Zweigbergk
> <martinvonz@gmail.com> wrote:
>>  create mode 100755 t/t3420-rebase-topology-linear.sh
>
> Just FYI, there's another test case with the same number
> (t3420-rebase-autostash) in pu. I don't know how you normally handle
> such cases.

Thanks for a heads-up. Usually, the series that appears later on the list yields and finds a unique number.

Martin von Zweigbergk· Jun 3, 2013, 18:12 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 1/7] add simple tests of consistency across rebase types

On Mon, Jun 3, 2013 at 11:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 12 quoted lines
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> On Tue, May 28, 2013 at 11:39 PM, Martin von Zweigbergk
>> <martinvonz@gmail.com> wrote:
>>>  create mode 100755 t/t3420-rebase-topology-linear.sh
>>
>> Just FYI, there's another test case with the same number
>> (t3420-rebase-autostash) in pu. I don't know how you normally handle
>> such cases.
>
> Thanks for a heads-up.  Usually, the series that appears later on
> the list yields and finds a unique number.

In this case, that's my series. Want a resend or do you want to do it yourself? I'm fine either way, whatever is easiest for you.

Martin von Zweigbergk· May 29, 2013, 06:39 UTC · re: Martin von Zweigbergk · lore

[PATCH v2 2/7] add tests for rebasing with patch-equivalence present

---
 t/lib-rebase.sh                   | 17 ++++++++
 t/t3420-rebase-topology-linear.sh | 85 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)
Show changes to 2 files +102 −0

t/lib-rebase.sh, t/t3420-rebase-topology-linear.sh

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 62b3887..16eeb1c 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -80,3 +80,20 @@ reset_rebase () {
 	git reset --hard &&
 	git clean -f
 }
+
+cherry_pick () {
+	git cherry-pick -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+revert () {
+	git revert -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+make_empty () {
+	git commit --allow-empty -m "$1" &&
+	git tag "$1"
+}
diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
index c4b32db..1152491 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -75,4 +75,89 @@ test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#
+# uppercase = cherry-picked
+# h = reverted g
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for range selection tests' '
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops patches in upstream" "
+		reset_rebase &&
+		git rebase $* h i &&
+		test_cmp_rev h HEAD~2 &&
+		test_linear_range 'd i' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* can drop last patch if in upstream" "
+		reset_rebase &&
+		git rebase $* h G &&
+		test_cmp_rev h HEAD^ &&
+		test_linear_range 'd' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto does not lose patches in upstream" "
+		reset_rebase &&
+		git rebase $* --onto f h i &&
+		test_cmp_rev f HEAD~3 &&
+		test_linear_range 'd G i' f..
+	"
+}
+test_run_rebase failure ''
+test_run_rebase success -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto drops patches in onto" "
+		reset_rebase &&
+		git rebase $* --onto h f i &&
+		test_cmp_rev h HEAD~2 &&
+		test_linear_range 'd i' h..
+	"
+}
+test_run_rebase failure ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
 test_done
-- 
1.8.2.674.gd17d3d2
Johannes Sixt· May 29, 2013, 07:09 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
Show 5 quoted lines
> +#       f
> +#      /
> +# a---b---c---g---h
> +#      \
> +#       d---G---i
...
Show 8 quoted lines
> +test_run_rebase () {
> +	result=$1
> +	shift
> +	test_expect_$result "rebase $* --onto drops patches in onto" "
> +		reset_rebase &&
> +		git rebase $* --onto h f i &&
> +		test_cmp_rev h HEAD~2 &&
> +		test_linear_range 'd i' h..

Isn't this expectation wrong? The upstream of the rebased branch is f, and it does not contain G. Hence, G should be replayed. Since h is the reversal of g, the state at h is the same as at c, and applying G should succeed (it is the same change as g). Therefore, I think the correct expectation is:

		test_linear_range 'd G i' h..
Show 6 quoted lines
> +	"
> +}
> +test_run_rebase failure ''
> +test_run_rebase failure -m
> +test_run_rebase failure -i
> +test_run_rebase failure -p
-- Hannes
Martin von Zweigbergk· May 30, 2013, 05:30 UTC · re: Johannes Sixt · lore

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
Show 23 quoted lines
> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>> +#       f
>> +#      /
>> +# a---b---c---g---h
>> +#      \
>> +#       d---G---i
> ...
>> +test_run_rebase () {
>> +     result=$1
>> +     shift
>> +     test_expect_$result "rebase $* --onto drops patches in onto" "
>> +             reset_rebase &&
>> +             git rebase $* --onto h f i &&
>> +             test_cmp_rev h HEAD~2 &&
>> +             test_linear_range 'd i' h..
>
> Isn't this expectation wrong? The upstream of the rebased branch is f, and
> it does not contain G. Hence, G should be replayed. Since h is the
> reversal of g, the state at h is the same as at c, and applying G should
> succeed (it is the same change as g). Therefore, I think the correct
> expectation is:
>
>                 test_linear_range 'd G i' h..

Good question! It is really not obvious what the right answer is. Some arguments in favor of dropping 'G':

1. Let's say origin/master points to 'b' when you start the 'd G i'
branch. You then send the 'G' patch to Junio who applies it as 'g'
(cherry-picking direction is reversed compared to figure, but same
effect). You then "git pull --rebase" when master on origin points to
'h'. Because of the cleverness in 'git pull --rebase', it issues 'git
rebase --onto h b i'. In this case it's clearly useful to have the
patch dropped.
2. In the test a little before the above one, we instead do 'git
rebase --onto f h i' and make sure that the 'G' is _not_ lost. In that
case it doesn't matter what's in $branch..$upstream. Do we agree that
$branch..$upstream should never matter (instead, $upstream is only
used to find merge base with $branch)? Do we also agree that 'git
rebase a b' should be identical to 'git rebase --onto a a b'? Because
'git rebase h i' should clearly drop 'G', then so should 'git rebase
--onto h h i'. Then, if we agreed that $branch..$upstream doesn't
matter, 'git rebase --onto h f i' should behave the same, no?

The set of commits to rebase that I was thinking of using was "$upstream..$branch, unless equivalent with patch in $branch..$onto". But I'm not very confident about my conclusions above :-)

Martin
Felipe Contreras· May 30, 2013, 05:41 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

On Thu, May 30, 2013 at 12:30 AM, Martin von Zweigbergk <martinvonz@gmail.com> wrote:

Show 27 quoted lines
> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>>> +#       f
>>> +#      /
>>> +# a---b---c---g---h
>>> +#      \
>>> +#       d---G---i
>> ...
>>> +test_run_rebase () {
>>> +     result=$1
>>> +     shift
>>> +     test_expect_$result "rebase $* --onto drops patches in onto" "
>>> +             reset_rebase &&
>>> +             git rebase $* --onto h f i &&
>>> +             test_cmp_rev h HEAD~2 &&
>>> +             test_linear_range 'd i' h..
>>
>> Isn't this expectation wrong? The upstream of the rebased branch is f, and
>> it does not contain G. Hence, G should be replayed. Since h is the
>> reversal of g, the state at h is the same as at c, and applying G should
>> succeed (it is the same change as g). Therefore, I think the correct
>> expectation is:
>>
>>                 test_linear_range 'd G i' h..
>
> Good question! It is really not obvious what the right answer is. Some
> arguments in favor of dropping 'G':

I think the answer is obvious; G should not be dropped. Maybe it made sense to drop g in upstream, but d fixes an issue, and it makes sense to apply G on upstream.

Git should not make any assumptions, if the user wants G to be dropped, he can easily do that by himself.

I don't think it's easy to implement that code-wise, but I think the ideal behavior is clear.

-- 
Felipe Contreras
Martin von Zweigbergk· May 30, 2013, 06:14 UTC · re: Felipe Contreras · lore

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

On Wed, May 29, 2013 at 10:41 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 33 quoted lines
> On Thu, May 30, 2013 at 12:30 AM, Martin von Zweigbergk
> <martinvonz@gmail.com> wrote:
>> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>>>> +#       f
>>>> +#      /
>>>> +# a---b---c---g---h
>>>> +#      \
>>>> +#       d---G---i
>>> ...
>>>> +test_run_rebase () {
>>>> +     result=$1
>>>> +     shift
>>>> +     test_expect_$result "rebase $* --onto drops patches in onto" "
>>>> +             reset_rebase &&
>>>> +             git rebase $* --onto h f i &&
>>>> +             test_cmp_rev h HEAD~2 &&
>>>> +             test_linear_range 'd i' h..
>>>
>>> Isn't this expectation wrong? The upstream of the rebased branch is f, and
>>> it does not contain G. Hence, G should be replayed. Since h is the
>>> reversal of g, the state at h is the same as at c, and applying G should
>>> succeed (it is the same change as g). Therefore, I think the correct
>>> expectation is:
>>>
>>>                 test_linear_range 'd G i' h..
>>
>> Good question! It is really not obvious what the right answer is. Some
>> arguments in favor of dropping 'G':
>
> I think the answer is obvious; G should not be dropped. Maybe it made
> sense to drop g in upstream, but d fixes an issue, and it makes sense
> to apply G on upstream.

Well, maybe I was wrong in thinking that dropping 'G' in 'git rebase --onto f h i' is bad. It seems to complicate things a lot, so maybe we should just decide that it's fine to do that (to drop 'G' in that case). Since that's mostly how it has worked forever and no one seems to have reported a problem with it, I'm probably just being paranoid. Thoughts?

Felipe Contreras· May 30, 2013, 06:40 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

On Thu, May 30, 2013 at 1:14 AM, Martin von Zweigbergk <martinvonz@gmail.com> wrote:

Show 42 quoted lines
> On Wed, May 29, 2013 at 10:41 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Thu, May 30, 2013 at 12:30 AM, Martin von Zweigbergk
>> <martinvonz@gmail.com> wrote:
>>> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>>> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>>>>> +#       f
>>>>> +#      /
>>>>> +# a---b---c---g---h
>>>>> +#      \
>>>>> +#       d---G---i
>>>> ...
>>>>> +test_run_rebase () {
>>>>> +     result=$1
>>>>> +     shift
>>>>> +     test_expect_$result "rebase $* --onto drops patches in onto" "
>>>>> +             reset_rebase &&
>>>>> +             git rebase $* --onto h f i &&
>>>>> +             test_cmp_rev h HEAD~2 &&
>>>>> +             test_linear_range 'd i' h..
>>>>
>>>> Isn't this expectation wrong? The upstream of the rebased branch is f, and
>>>> it does not contain G. Hence, G should be replayed. Since h is the
>>>> reversal of g, the state at h is the same as at c, and applying G should
>>>> succeed (it is the same change as g). Therefore, I think the correct
>>>> expectation is:
>>>>
>>>>                 test_linear_range 'd G i' h..
>>>
>>> Good question! It is really not obvious what the right answer is. Some
>>> arguments in favor of dropping 'G':
>>
>> I think the answer is obvious; G should not be dropped. Maybe it made
>> sense to drop g in upstream, but d fixes an issue, and it makes sense
>> to apply G on upstream.
>
> Well, maybe I was wrong in thinking that dropping 'G' in 'git rebase
> --onto f h i' is bad. It seems to complicate things a lot, so maybe we
> should just decide that it's fine to do that (to drop 'G' in that
> case). Since that's mostly how it has worked forever and no one seems
> to have reported a problem with it, I'm probably just being paranoid.
> Thoughts?
Huh? I said the opposite; G should *not* be dropped.
-- 
Felipe Contreras
Martin von Zweigbergk· May 30, 2013, 06:46 UTC · re: Felipe Contreras · lore

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

On Wed, May 29, 2013 at 11:40 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 46 quoted lines
> On Thu, May 30, 2013 at 1:14 AM, Martin von Zweigbergk
> <martinvonz@gmail.com> wrote:
>> On Wed, May 29, 2013 at 10:41 PM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> On Thu, May 30, 2013 at 12:30 AM, Martin von Zweigbergk
>>> <martinvonz@gmail.com> wrote:
>>>> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>>>> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>>>>>> +#       f
>>>>>> +#      /
>>>>>> +# a---b---c---g---h
>>>>>> +#      \
>>>>>> +#       d---G---i
>>>>> ...
>>>>>> +test_run_rebase () {
>>>>>> +     result=$1
>>>>>> +     shift
>>>>>> +     test_expect_$result "rebase $* --onto drops patches in onto" "
>>>>>> +             reset_rebase &&
>>>>>> +             git rebase $* --onto h f i &&
>>>>>> +             test_cmp_rev h HEAD~2 &&
>>>>>> +             test_linear_range 'd i' h..
>>>>>
>>>>> Isn't this expectation wrong? The upstream of the rebased branch is f, and
>>>>> it does not contain G. Hence, G should be replayed. Since h is the
>>>>> reversal of g, the state at h is the same as at c, and applying G should
>>>>> succeed (it is the same change as g). Therefore, I think the correct
>>>>> expectation is:
>>>>>
>>>>>                 test_linear_range 'd G i' h..
>>>>
>>>> Good question! It is really not obvious what the right answer is. Some
>>>> arguments in favor of dropping 'G':
>>>
>>> I think the answer is obvious; G should not be dropped. Maybe it made
>>> sense to drop g in upstream, but d fixes an issue, and it makes sense
>>> to apply G on upstream.
>>
>> Well, maybe I was wrong in thinking that dropping 'G' in 'git rebase
>> --onto f h i' is bad. It seems to complicate things a lot, so maybe we
>> should just decide that it's fine to do that (to drop 'G' in that
>> case). Since that's mostly how it has worked forever and no one seems
>> to have reported a problem with it, I'm probably just being paranoid.
>> Thoughts?
>
> Huh? I said the opposite; G should *not* be dropped.

I suspect you missed that I said 'git rebase --onto f h i', not 'git rebase --onto h f i'. Sorry, I should have pointed that out.

Johannes Sixt· May 30, 2013, 12:54 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

Am 30.05.2013 07:30, schrieb Martin von Zweigbergk:
Show 34 quoted lines
> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>>> +#       f
>>> +#      /
>>> +# a---b---c---g---h
>>> +#      \
>>> +#       d---G---i
>> ...
>>> +test_run_rebase () {
>>> +     result=$1
>>> +     shift
>>> +     test_expect_$result "rebase $* --onto drops patches in onto" "
>>> +             reset_rebase &&
>>> +             git rebase $* --onto h f i &&
>>> +             test_cmp_rev h HEAD~2 &&
>>> +             test_linear_range 'd i' h..
>>
>> Isn't this expectation wrong? The upstream of the rebased branch is f, and
>> it does not contain G. Hence, G should be replayed. Since h is the
>> reversal of g, the state at h is the same as at c, and applying G should
>> succeed (it is the same change as g). Therefore, I think the correct
>> expectation is:
>>
>>                 test_linear_range 'd G i' h..
> 
> Good question! It is really not obvious what the right answer is. Some
> arguments in favor of dropping 'G':
> 
> 1. Let's say origin/master points to 'b' when you start the 'd G i'
> branch. You then send the 'G' patch to Junio who applies it as 'g'
> (cherry-picking direction is reversed compared to figure, but same
> effect). You then "git pull --rebase" when master on origin points to
> 'h'. Because of the cleverness in 'git pull --rebase', it issues 'git
> rebase --onto h b i'.

The reason for this git pull cleverness is to be prepared for rewritten history:

   b'--c'--g'--h'
  /
 a---b
      \
       d---G---i
to avoid that b is rebased.
Show 8 quoted lines
> In this case it's clearly useful to have the
> patch dropped.
> 
> 2. In the test a little before the above one, we instead do 'git
> rebase --onto f h i' and make sure that the 'G' is _not_ lost. In that
> case it doesn't matter what's in $branch..$upstream. Do we agree that
> $branch..$upstream should never matter (instead, $upstream is only
> used to find merge base with $branch)?

No, we do not agree. $branch..$upstream should be the set of patches that should be omitted. $branch..$onto should not matter. $onto is only used to specify the destination of the rebased commits.

> Do we also agree that 'git
> rebase a b' should be identical to 'git rebase --onto a a b'?
Absolutely!
> Because
> 'git rebase h i' should clearly drop 'G', then so should 'git rebase
> --onto h h i'.
Yes!
> Then, if we agreed that $branch..$upstream doesn't
> matter, 'git rebase --onto h f i' should behave the same, no?

Correct in the mathematically logical sense. ;) But we do not agree that $branch..$upstream doesn't matter.

> The set of commits to rebase that I was thinking of using was
> "$upstream..$branch, unless equivalent with patch in $branch..$onto".
> But I'm not very confident about my conclusions above :-)

At least the man page says that ..$upstream counts and $onto tells just the new base.

The way how git pull calls rebase should be revisited, I think.
-- Hannes
Martin von Zweigbergk· May 30, 2013, 15:01 UTC · re: Johannes Sixt · lore

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

On Thu, May 30, 2013 at 5:54 AM, Johannes Sixt <j6t@kdbg.org> wrote:
Show 46 quoted lines
> Am 30.05.2013 07:30, schrieb Martin von Zweigbergk:
>> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>>>> +#       f
>>>> +#      /
>>>> +# a---b---c---g---h
>>>> +#      \
>>>> +#       d---G---i
>>> ...
>>>> +test_run_rebase () {
>>>> +     result=$1
>>>> +     shift
>>>> +     test_expect_$result "rebase $* --onto drops patches in onto" "
>>>> +             reset_rebase &&
>>>> +             git rebase $* --onto h f i &&
>>>> +             test_cmp_rev h HEAD~2 &&
>>>> +             test_linear_range 'd i' h..
>>>
>>> Isn't this expectation wrong? The upstream of the rebased branch is f, and
>>> it does not contain G. Hence, G should be replayed. Since h is the
>>> reversal of g, the state at h is the same as at c, and applying G should
>>> succeed (it is the same change as g). Therefore, I think the correct
>>> expectation is:
>>>
>>>                 test_linear_range 'd G i' h..
>>
>> Good question! It is really not obvious what the right answer is. Some
>> arguments in favor of dropping 'G':
>>
>> 1. Let's say origin/master points to 'b' when you start the 'd G i'
>> branch. You then send the 'G' patch to Junio who applies it as 'g'
>> (cherry-picking direction is reversed compared to figure, but same
>> effect). You then "git pull --rebase" when master on origin points to
>> 'h'. Because of the cleverness in 'git pull --rebase', it issues 'git
>> rebase --onto h b i'.
>
> The reason for this git pull cleverness is to be prepared for rewritten
> history:
>
>    b'--c'--g'--h'
>   /
>  a---b
>       \
>        d---G---i
>
> to avoid that b is rebased.

Right. It doesn't currently drop 'G' and maybe it shouldn't, so let's leave it as is for now, I would say.

Show 9 quoted lines
>> 2. In the test a little before the above one, we instead do 'git
>> rebase --onto f h i' and make sure that the 'G' is _not_ lost. In that
>> case it doesn't matter what's in $branch..$upstream. Do we agree that
>> $branch..$upstream should never matter (instead, $upstream is only
>> used to find merge base with $branch)?
>
> No, we do not agree. $branch..$upstream should be the set of patches
> that should be omitted. $branch..$onto should not matter. $onto is only
> used to specify the destination of the rebased commits.

Ok. As I said to Felipe, I'm not sure why I was so convinced that it's bad to lose the patch in 'git rebase --onto f h i'. It can result in lost work, but it seems rare enough that no one has reported it, AFAIK.

I'll change those tests in a re-roll, and perhaps I'll drop a few of them. Let me know if you (anyone) disagree.

Martin
PS. Thanks for a meticulous review!
Martin von Zweigbergk· May 29, 2013, 06:39 UTC · re: Martin von Zweigbergk · lore

[PATCH v2 3/7] add tests for rebasing of empty commits

---
 t/t3401-rebase-partial.sh         | 24 ----------------
 t/t3420-rebase-topology-linear.sh | 58 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 24 deletions(-)
Show changes to 2 files +58 −24

t/t3401-rebase-partial.sh, t/t3420-rebase-topology-linear.sh

diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index 58f4823..7ba1797 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -42,28 +42,4 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr
 	test_path_is_missing .git/rebase-merge
 '
 
-test_expect_success 'rebase ignores empty commit' '
-	git reset --hard A &&
-	git commit --allow-empty -m empty &&
-	test_commit D &&
-	git rebase C &&
-	test "$(git log --format=%s C..)" = "D"
-'
-
-test_expect_success 'rebase --keep-empty' '
-	git reset --hard D &&
-	git rebase --keep-empty C &&
-	test "$(git log --format=%s C..)" = "D
-empty"
-'
-
-test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' '
-	git reset --hard A &&
-	git commit --allow-empty -m also-empty &&
-	git rebase --keep-empty D &&
-	test "$(git log --format=%s A..)" = "also-empty
-D
-empty"
-'
-
 test_done
diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
index 1152491..40fe264 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -160,4 +160,62 @@ test_run_rebase failure -m
 test_run_rebase failure -i
 test_run_rebase failure -p
 
+# a---b---c---j!
+#      \
+#       d---k!--l
+#
+# ! = empty
+test_expect_success 'setup of linear history for empty commit tests' '
+	git checkout c &&
+	make_empty j &&
+	git checkout d &&
+	make_empty k &&
+	test_commit l
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops empty commit" "
+		reset_rebase &&
+		git rebase $* c l &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty" "
+		reset_rebase &&
+		git rebase $* --keep-empty c l &&
+		test_cmp_rev c HEAD~3 &&
+		test_linear_range 'd k l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty keeps empty even if already in upstream" "
+		reset_rebase &&
+		git rebase $* --keep-empty j l &&
+		test_cmp_rev j HEAD~3 &&
+		test_linear_range 'd k l' j..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
 test_done
-- 
1.8.2.674.gd17d3d2
Martin von Zweigbergk· May 29, 2013, 06:39 UTC · re: Martin von Zweigbergk · lore

[PATCH v2 4/7] add tests for rebasing root

---
 t/t3420-rebase-topology-linear.sh | 129 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)
Show changes to t/t3420-rebase-topology-linear.sh +129 −0
diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
index 40fe264..2429aa8 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -218,4 +218,133 @@ test_run_rebase failure -m
 test_run_rebase failure -i
 test_run_rebase failure -p
 
+#       m
+#      /
+# a---b---c---g
+#
+# x---y---B
+#
+# uppercase = cherry-picked
+# m = reverted b
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for test involving root' '
+	git checkout b &&
+	revert m b &&
+	git checkout --orphan disjoint &&
+	git rm -rf . &&
+	test_commit x &&
+	test_commit y &&
+	cherry_pick B b
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root" "
+		reset_rebase &&
+		git rebase $* --onto c --root y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history" "
+		reset_rebase &&
+		git rebase $* c y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root drops patch in onto" "
+		reset_rebase &&
+		git rebase $* --onto m --root B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root with merge-base does not go to root" "
+		reset_rebase &&
+		git rebase $* --onto m --root g &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'c g' m..
+	"
+}
+
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history drops patch in onto" "
+		reset_rebase &&
+		git rebase $* m B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --root on linear history is a no-op" "
+		reset_rebase &&
+		git rebase $* --root c &&
+		test_cmp_rev c HEAD
+	"
+}
+test_run_rebase failure ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f --root on linear history causes re-write" "
+		reset_rebase &&
+		git rebase $* -f --root c &&
+		! test_cmp_rev a HEAD~2 &&
+		test_linear_range 'a b c' HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
 test_done
-- 
1.8.2.674.gd17d3d2
Johannes Sixt· May 29, 2013, 07:31 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v2 4/7] add tests for rebasing root

Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
Show 8 quoted lines
> +test_run_rebase () {
> +	result=$1
> +	shift
> +	test_expect_$result "rebase $* --onto --root with merge-base does not go to root" "
> +		reset_rebase &&
> +		git rebase $* --onto m --root g &&
> +		test_cmp_rev m HEAD~2 &&
> +		test_linear_range 'c g' m..

Here you check the outcome. There is no explicit check whether the rebase attempted to replay a and b. But that check is implicit: If a or b were attempted to replay, the rebase would have been interrupted with "no new changes". Right?

Show 7 quoted lines
> +	"
> +}
> +
> +test_run_rebase success ''
> +test_run_rebase success -m
> +test_run_rebase success -i
> +test_run_rebase failure -p

Just curious: Does the last one fail because the result is not correct or because it does go to the root?

-- Hannes
Martin von Zweigbergk· May 30, 2013, 05:51 UTC · re: Johannes Sixt · lore

Re: [PATCH v2 4/7] add tests for rebasing root

On Wed, May 29, 2013 at 12:31 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
Show 14 quoted lines
> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>> +test_run_rebase () {
>> +     result=$1
>> +     shift
>> +     test_expect_$result "rebase $* --onto --root with merge-base does not go to root" "
>> +             reset_rebase &&
>> +             git rebase $* --onto m --root g &&
>> +             test_cmp_rev m HEAD~2 &&
>> +             test_linear_range 'c g' m..
>
> Here you check the outcome. There is no explicit check whether the rebase
> attempted to replay a and b. But that check is implicit: If a or b were
> attempted to replay, the rebase would have been interrupted with "no new
> changes". Right?

Because 'm' is a reverted 'b', I think if it had gone to the root, we would have seen 'b m c g' (I _think_ 'a' would be silently skipped at least in am mode).

>> +test_run_rebase failure -p
>
> Just curious: Does the last one fail because the result is not correct or
> because it does go to the root?

Because the result is not correct; it first checks out 'm', but something goes wrong (maybe because 'm' gets written to /rewritten/root?) and it somehow fast-forwards to 'c' (from 'b'?).

Martin von Zweigbergk· May 29, 2013, 06:39 UTC · re: Martin von Zweigbergk · lore

[PATCH v2 5/7] add tests for rebasing merged history

---
 t/t3400-rebase.sh                 |  31 +----
 t/t3401-rebase-partial.sh         |  45 -------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3409-rebase-preserve-merges.sh |  53 --------
 t/t3425-rebase-topology-merges.sh | 250 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 252 insertions(+), 137 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh
Show changes to 5 files +252 −137

t/t3400-rebase.sh, t/t3401-rebase-partial.sh, t/t3404-rebase-interactive.sh, t/t3409-rebase-preserve-merges.sh, t/t3425-rebase-topology-merges.sh

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..b436ef4 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' '
 	echo Side >>C &&
 	git add C &&
 	git commit -m "Add C" &&
-	git checkout -b nonlinear my-topic-branch &&
-	echo Edit >>B &&
-	git add B &&
-	git commit -m "Modify B" &&
-	git merge side &&
-	git checkout -b upstream-merged-nonlinear &&
-	git merge master &&
 	git checkout -f my-topic-branch &&
 	git tag topic
 '
@@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' '
 	git rebase master
 '
 
-test_expect_success 'rebase after merge master' '
-	git checkout --detach refs/tags/topic &&
-	git branch -D topic &&
-	git reset --hard topic &&
-	git merge master &&
-	git rebase master &&
-	! (git show | grep "^Merge:")
-'
-
-test_expect_success 'rebase of history with merges is linearized' '
-	git checkout nonlinear &&
-	test 4 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
-test_expect_success 'rebase of history with merges after upstream merge is linearized' '
-	git checkout upstream-merged-nonlinear &&
-	test 5 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
 test_expect_success 'rebase a single mode change' '
 	git checkout master &&
+	git branch -D topic &&
 	echo 1 >X &&
 	git add X &&
 	test_tick &&
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
deleted file mode 100755
index 7ba1797..0000000
--- a/t/t3401-rebase-partial.sh
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland
-#
-
-test_description='git rebase should detect patches integrated upstream
-
-This test cherry-picks one local change of two into master branch, and
-checks that git rebase succeeds with only the second patch in the
-local branch.
-'
-. ./test-lib.sh
-
-test_expect_success 'prepare repository with topic branch' '
-	test_commit A &&
-	git checkout -b my-topic-branch &&
-	test_commit B &&
-	test_commit C &&
-	git checkout -f master &&
-	test_commit A2 A.t
-'
-
-test_expect_success 'pick top patch from topic branch into master' '
-	git cherry-pick C &&
-	git checkout -f my-topic-branch
-'
-
-test_debug '
-	git cherry master &&
-	git format-patch -k --stdout --full-index master >/dev/null &&
-	gitk --all & sleep 1
-'
-
-test_expect_success 'rebase topic branch against new master and check git am did not get halted' '
-	git rebase master &&
-	test_path_is_missing .git/rebase-apply
-'
-
-test_expect_success 'rebase --merge topic branch that was partially merged upstream' '
-	git reset --hard C &&
-	git rebase --merge master &&
-	test_path_is_missing .git/rebase-merge
-'
-
-test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index a58406d..ffcaf02 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' '
 	test $one = $(git rev-parse HEAD~2)
 '
 
-test_expect_success 'ignore patch if in upstream' '
-	HEAD=$(git rev-parse HEAD) &&
-	git checkout -b has-cherry-picked HEAD^ &&
+test_expect_success '--continue tries to commit, even for "edit"' '
 	echo unrelated > file7 &&
 	git add file7 &&
 	test_tick &&
 	git commit -m "unrelated change" &&
-	git cherry-pick $HEAD &&
-	EXPECT_COUNT=1 git rebase -i $HEAD &&
-	test $HEAD = $(git rev-parse HEAD^)
-'
-
-test_expect_success '--continue tries to commit, even for "edit"' '
 	parent=$(git rev-parse HEAD^) &&
 	test_tick &&
 	FAKE_LINES="edit 1" git rebase -i HEAD^ &&
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 6de4e22..2e0c364 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -11,14 +11,6 @@ Run "git rebase -p" and check that merges are properly carried along
 GIT_AUTHOR_EMAIL=bogus_email_address
 export GIT_AUTHOR_EMAIL
 
-# Clone 1 (trivial merge):
-#
-# A1--A2  <-- origin/master
-#  \   \
-#   B1--M  <-- topic
-#    \
-#     B2  <-- origin/topic
-#
 # Clone 2 (conflicting merge):
 #
 # A1--A2--B3   <-- origin/master
@@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL
 #     \--A3    <-- topic2
 #      \
 #       B2     <-- origin/topic
-#
-# Clone 4 (merge using second parent as base):
-#
-# A1--A2--B3   <-- origin/master
-#  \
-#   B1--A3--M  <-- topic
-#    \     /
-#     \--A4    <-- topic2
-#      \
-#       B2     <-- origin/topic
 
 test_expect_success 'setup for merge-preserving rebase' \
 	'echo First > A &&
@@ -58,20 +40,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git checkout -f master &&
 	echo Third >> A &&
 	git commit -a -m "Modify A2" &&
-
-	git clone ./. clone1 &&
-	(cd clone1 &&
-	git checkout -b topic origin/topic &&
-	git merge origin/master
-	) &&
-
-	git clone ./. clone4 &&
-	(
-		cd clone4 &&
-		git checkout -b topic origin/topic &&
-		git merge origin/master
-	) &&
-
 	echo Fifth > B &&
 	git add B &&
 	git commit -m "Add different B" &&
@@ -101,16 +69,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git commit -a -m "Modify B2"
 '
 
-test_expect_success 'rebase -p fakes interactive rebase' '
-	(
-	cd clone1 &&
-	git fetch &&
-	git rebase -p origin/topic &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_expect_success '--continue works after a conflict' '
 	(
 	cd clone2 &&
@@ -138,15 +96,4 @@ test_expect_success 'rebase -p preserves no-ff merges' '
 	)
 '
 
-test_expect_success 'rebase -p works when base inside second parent' '
-	(
-	cd clone4 &&
-	git fetch &&
-	git rebase -p HEAD^2 &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify B" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_done
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
new file mode 100755
index 0000000..0f56407
--- /dev/null
+++ b/t/t3425-rebase-topology-merges.sh
@@ -0,0 +1,250 @@
+#!/bin/sh
+
+test_description='rebase topology tests with merges'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+test_revision_subjects () {
+	expected="$1"
+	shift
+	set -- $(git log --format=%s --no-walk=unsorted "$@")
+	test "$expected" = "$*"
+}
+
+# a---b-----------c
+#      \           \
+#       d-------e   \
+#        \       \   \
+#         n---o---w---v
+#              \
+#               z
+#
+# uppercase = cherry-picked
+test_expect_success 'setup of non-linear-history' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+
+	git checkout d &&
+	test_commit n &&
+	test_commit o &&
+	test_merge w e &&
+	test_merge v c &&
+	git checkout o &&
+	test_commit z
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* after merge from upstream" "
+		reset_rebase &&
+		git rebase $* e w &&
+		test_cmp_rev e HEAD~2 &&
+		test_linear_range 'n o' e..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized in place" "
+		reset_rebase &&
+		git rebase $* d w &&
+		test_cmp_rev d HEAD~3 &&
+		test_linear_range "\'"$expected"\'" d..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'e n o' ''
+test_run_rebase success 'e n o' -m
+test_run_rebase success 'n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized upstream" "
+		reset_rebase &&
+		git rebase $* c w &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history with merges after upstream merge is linearized" "
+		reset_rebase &&
+		git rebase $* c v &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_expect_success "rebase -p is no-op in non-linear history" "
+	reset_rebase &&
+	git rebase -p d w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p is no-op when base inside second parent" "
+	reset_rebase &&
+	git rebase -p e w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_failure "rebase -p --root on non-linear history is a no-op" "
+	reset_rebase &&
+	git rebase -p --root w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p re-creates merge from side branch" "
+	reset_rebase &&
+	git rebase -p z w &&
+	test_cmp_rev z HEAD^ &&
+	test_cmp_rev w^2 HEAD^2
+"
+
+test_expect_success "rebase -p re-creates internal merge" "
+	reset_rebase &&
+	git rebase -p c w &&
+	test_revision_subjects 'c d n e o w' HEAD~4 HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p can re-create two branches on onto" "
+	reset_rebase &&
+	git rebase -p --onto c d w &&
+	test_revision_subjects 'c n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#        \       \
+#         e-------u
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for patch-equivalence tests' '
+	git checkout e &&
+	test_merge u i
+'
+
+test_expect_success "rebase -p re-creates history around dropped commit matching upstream" "
+	reset_rebase &&
+	git rebase -p h u &&
+	test_cmp_rev h HEAD~3 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_failure "rebase -p --onto in merged history does not lose patches in upstream" "
+	reset_rebase &&
+	git rebase -p --onto f h u &&
+	test_cmp_rev f HEAD~3 &&
+	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history drops patches in onto" "
+	reset_rebase &&
+	git rebase -p --onto h f u &&
+	test_cmp_rev h HEAD~3 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+# a---b---c---g---h
+#      \
+#       d---G---s
+#        \   \ /
+#         \   X
+#          \ / \
+#           e---t
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for dropping whole side' '
+	git checkout G &&
+	test_merge s e &&
+	git checkout e &&
+	test_merge t G
+'
+
+test_expect_failure "rebase -p drops merge commit when entire first-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h s &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+test_expect_success "rebase -p drops merge commit when entire second-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h t &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+# a---b---c
+#      \
+#       d---e
+#        \   \
+#         n---r
+#          \
+#           o
+#
+# r = tree-same with n
+# uppercace = cherry-picked
+test_expect_success 'setup of non-linear-history for empty commits' '
+	git checkout n &&
+	git merge --no-commit e &&
+	git reset n . &&
+	git commit -m r &&
+	git reset --hard &&
+	git clean -f &&
+	git tag r
+'
+
+test_expect_success "rebase -p re-creates empty internal merge commit" "
+	reset_rebase &&
+	git rebase -p c r &&
+	test_revision_subjects 'c d e n r' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p re-creates empty merge commit" "
+	reset_rebase &&
+	git rebase -p o r &&
+	test_revision_subjects 'e o r' HEAD^2 HEAD^ HEAD
+"
+
+test_done
-- 
1.8.2.674.gd17d3d2
Johannes Sixt· May 29, 2013, 07:57 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v2 5/7] add tests for rebasing merged history

Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
Show 7 quoted lines
> +# a---b-----------c
> +#      \           \
> +#       d-------e   \
> +#        \       \   \
> +#         n---o---w---v
> +#              \
> +#               z
> +#TODO: make all flavors of rebase use --topo-order
> +test_run_rebase success 'e n o' ''
> +test_run_rebase success 'e n o' -m
> +test_run_rebase success 'n o e' -i

As test_commit offers predictable timestamps, I think you can work around this discrepancy by generating commits n and o before e. (That is not a solution--just a workaround that depends on the current implementation--because the order in which parents of a merge are listed is unspecified.)

> +test_expect_success "rebase -p re-creates internal merge" "
> +	reset_rebase &&
> +	git rebase -p c w &&
> +	test_revision_subjects 'c d n e o w' HEAD~4 HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
Shouldn't this better be
	test_cmp_rev c HEAD~4 &&
	test_revision_subjects 'd n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
to ensure that c is not a rewritten commit?
Show 7 quoted lines
> +"
> +
> +test_expect_success "rebase -p can re-create two branches on onto" "
> +	reset_rebase &&
> +	git rebase -p --onto c d w &&
> +	test_revision_subjects 'c n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
> +"
Same here.
Time is fleeting. I have to stop here.
-- Hannes
Martin von Zweigbergk· May 31, 2013, 05:42 UTC · re: Johannes Sixt · lore

Re: [PATCH v2 5/7] add tests for rebasing merged history

On Wed, May 29, 2013 at 12:57 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
Show 19 quoted lines
> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>> +# a---b-----------c
>> +#      \           \
>> +#       d-------e   \
>> +#        \       \   \
>> +#         n---o---w---v
>> +#              \
>> +#               z
>
>> +#TODO: make all flavors of rebase use --topo-order
>> +test_run_rebase success 'e n o' ''
>> +test_run_rebase success 'e n o' -m
>> +test_run_rebase success 'n o e' -i
>
> As test_commit offers predictable timestamps, I think you can work around
> this discrepancy by generating commits n and o before e. (That is not a
> solution--just a workaround that depends on the current
> implementation--because the order in which parents of a merge are listed
> is unspecified.)

I actually liked it as documentation of the current inconsistency and with an explicit TODO.

I have addressed the remainder of your comments in this and the next message. Thanks again.

Johannes Sixt· May 29, 2013, 10:33 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v2 5/7] add tests for rebasing merged history

Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
Show 10 quoted lines
> +#       f
> +#      /
> +# a---b---c---g---h
> +#      \
> +#       d---G---i
> +#        \       \
> +#         e-------u
> +#
> +# uppercase = cherry-picked
> +# h = reverted g
Show 6 quoted lines
> +test_expect_failure "rebase -p --onto in merged history does not lose patches in upstream" "
> +	reset_rebase &&
> +	git rebase -p --onto f h u &&
> +	test_cmp_rev f HEAD~3 &&
> +	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
> +"

My expectations are different: When a patch is in upstream, then it is not to be rebased, even --onto somewhere else than upstream.

But take this with a grain of salt, as I never encounter(ed) this use-case in practice.

Show 6 quoted lines
> +test_expect_success "rebase -p --onto in merged history drops patches in onto" "
> +	reset_rebase &&
> +	git rebase -p --onto h f u &&
> +	test_cmp_rev h HEAD~3 &&
> +	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
> +"
And this is just the opposite case, where I think the patch should be kept.
Show 10 quoted lines
> +# a---b---c
> +#      \
> +#       d---e
> +#        \   \
> +#         n---r
> +#          \
> +#           o
> +#
> +# r = tree-same with n
> +# uppercace = cherry-picked
I do not see any upper-cased letters in this graph. ;)
> +test_expect_success "rebase -p re-creates empty internal merge commit" "
> +	reset_rebase &&
> +	git rebase -p c r &&
> +	test_revision_subjects 'c d e n r' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
Again, check c with test_cmp_rev.
-- Hannes
Martin von Zweigbergk· May 29, 2013, 06:39 UTC · re: Martin von Zweigbergk · lore

[PATCH v2 6/7] t3406: modernize style

Update the following:
 - Quote 'setup'
 - Remove blank lines within test case body
 - Use test_commit instead of custom quick_one
 - Create branch "topic" from tag created by test_commit
---
 t/t3406-rebase-message.sh | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)
Show changes to t/t3406-rebase-message.sh +9 −21
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index e6a9a0d..fe8c27f 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -4,27 +4,17 @@ test_description='messages from rebase operation'
 
 . ./test-lib.sh
 
-quick_one () {
-	echo "$1" >"file$1" &&
-	git add "file$1" &&
-	test_tick &&
-	git commit -m "$1"
-}
+test_expect_success 'setup' '
+	test_commit O fileO &&
+	test_commit X fileX &&
+	test_commit A fileA &&
+	test_commit B fileB &&
+	test_commit Y fileY &&
 
-test_expect_success setup '
-	quick_one O &&
-	git branch topic &&
-	quick_one X &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Y &&
-
-	git checkout topic &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Z &&
+	git checkout -b topic O &&
+	git cherry-pick A B &&
+	test_commit Z fileZ &&
 	git tag start
-
 '
 
 cat >expect <<\EOF
@@ -34,12 +24,10 @@ Committed: 0003 Z
 EOF
 
 test_expect_success 'rebase -m' '
-
 	git rebase -m master >report &&
 	sed -n -e "/^Already applied: /p" \
 		-e "/^Committed: /p" report >actual &&
 	test_cmp expect actual
-
 '
 
 test_expect_success 'rebase --stat' '
-- 
1.8.2.674.gd17d3d2
Martin von Zweigbergk· May 29, 2013, 06:39 UTC · re: Martin von Zweigbergk · lore

[PATCH v2 7/7] tests: move test for rebase messages from t3400 to t3406

t3406 is supposed to test "messages from rebase operation", so let's
move tests in t3400 that fit that description into 3406. Most of the
functionality they tested, except for the messages, has now been
subsumed by t3420.
---
 t/t3400-rebase.sh         | 22 ----------------------
 t/t3406-rebase-message.sh | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 22 deletions(-)
Show changes to 2 files +22 −22

t/t3400-rebase.sh, t/t3406-rebase-message.sh

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b436ef4..45a55e9 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -59,28 +59,6 @@ test_expect_success 'rebase against master' '
 	git rebase master
 '
 
-test_expect_success 'rebase against master twice' '
-	git rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase against master twice with --force' '
-	git rebase --force-rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date, rebase forced" out
-'
-
-test_expect_success 'rebase against master twice from another branch' '
-	git checkout my-topic-branch^ &&
-	git rebase master my-topic-branch >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase fast-forward to master' '
-	git checkout my-topic-branch^ &&
-	git rebase my-topic-branch >out &&
-	test_i18ngrep "Fast-forwarded HEAD to my-topic-branch" out
-'
-
 test_expect_success 'the rebase operation should not have destroyed author information' '
 	! (git log | grep "Author:" | grep "<>")
 '
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index fe8c27f..0392e36 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -30,6 +30,28 @@ test_expect_success 'rebase -m' '
 	test_cmp expect actual
 '
 
+test_expect_success 'rebase against master twice' '
+	git rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase against master twice with --force' '
+	git rebase --force-rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date, rebase forced" out
+'
+
+test_expect_success 'rebase against master twice from another branch' '
+	git checkout topic^ &&
+	git rebase master topic >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase fast-forward to master' '
+	git checkout topic^ &&
+	git rebase topic >out &&
+	test_i18ngrep "Fast-forwarded HEAD to topic" out
+'
+
 test_expect_success 'rebase --stat' '
 	git reset --hard start &&
         git rebase --stat master >diffstat.txt &&
-- 
1.8.2.674.gd17d3d2
Felipe Contreras· May 29, 2013, 07:10 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v2 0/7] Rebase topology test

On Wed, May 29, 2013 at 1:39 AM, Martin von Zweigbergk <martinvonz@gmail.com> wrote:

Show 5 quoted lines
> After way too long, here is finally a new version of the tests I sent
> at: http://thread.gmane.org/gmane.comp.version-control.git/205796.
>
> I have split the test up into two files. They stil take quite some
> time to run.

This is definitely needed. I'm doing some experiments with 'git rebase' and I see so many problems in the code that these tests do exercise.

I think a lot of the functionality of 'git rebase' should move to 'git cherry-pick', and then all the 'git rebase' code can be simplified greatly, and tests like these would help a lot.

-- 
Felipe Contreras
Ramkumar Ramachandra· May 29, 2013, 12:50 UTC · re: Felipe Contreras · lore

Re: [PATCH v2 0/7] Rebase topology test

Felipe Contreras wrote:
> I think a lot of the functionality of 'git rebase' should move to 'git
> cherry-pick', and then all the 'git rebase' code can be simplified
> greatly, and tests like these would help a lot.

What do we do about the leakages? Want to take on the task of fixing the merge-recursive machinery?

Cf. $gmane/222887
Felipe Contreras· May 29, 2013, 13:54 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH v2 0/7] Rebase topology test

On Wed, May 29, 2013 at 7:50 AM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

Show 9 quoted lines
> Felipe Contreras wrote:
>> I think a lot of the functionality of 'git rebase' should move to 'git
>> cherry-pick', and then all the 'git rebase' code can be simplified
>> greatly, and tests like these would help a lot.
>
> What do we do about the leakages?  Want to take on the task of fixing
> the merge-recursive machinery?
>
> Cf. $gmane/222887
Hmm, I saw that, but I also saw the fix.

Anyway, if this is such a big issue, I'm sure it should be possible to trigger it in our test frameework.

-- 
Felipe Contreras
Martin von Zweigbergk· May 31, 2013, 06:49 UTC · re: Martin von Zweigbergk · lore

[PATCH v3 0/7] Rebase topology test

Patches are now expected to be dropped iff they are on upstream. I've also followed all of Johannes's other suggestions except for the one about topo-order.

Martin von Zweigbergk (7):
  add simple tests of consistency across rebase types
  add tests for rebasing with patch-equivalence present
  add tests for rebasing of empty commits
  add tests for rebasing root
  add tests for rebasing merged history
  t3406: modernize style
  tests: move test for rebase messages from t3400 to t3406
 t/lib-rebase.sh                   |  32 ++++
 t/t3400-rebase.sh                 |  53 +-----
 t/t3401-rebase-partial.sh         |  69 --------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3406-rebase-message.sh         |  50 +++---
 t/t3409-rebase-preserve-merges.sh |  53 ------
 t/t3420-rebase-topology-linear.sh | 350 ++++++++++++++++++++++++++++++++++++++
 t/t3425-rebase-topology-merges.sh | 252 +++++++++++++++++++++++++++
 8 files changed, 666 insertions(+), 203 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3420-rebase-topology-linear.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh
-- 
1.8.2.674.gd17d3d2
Martin von Zweigbergk· May 31, 2013, 06:49 UTC · re: Martin von Zweigbergk · lore

[PATCH v3 1/7] add simple tests of consistency across rebase types

Helped-by: Johannes Sixt <j6t@kdbg.org>
---
 t/lib-rebase.sh                   | 15 ++++++++
 t/t3420-rebase-topology-linear.sh | 78 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)
 create mode 100755 t/t3420-rebase-topology-linear.sh
Show changes to 2 files +93 −0

t/lib-rebase.sh, t/t3420-rebase-topology-linear.sh

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6ccf797..62b3887 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -65,3 +65,18 @@ EOF
 	test_set_editor "$(pwd)/fake-editor.sh"
 	chmod a+x fake-editor.sh
 }
+
+# checks that the revisions in "$2" represent a linear range with the
+# subjects in "$1"
+test_linear_range () {
+	! { git log --format=%p "$2" | sane_grep " " ;} &&
+	expected=$1
+	set -- $(git log --reverse --format=%s "$2")
+	test "$expected" = "$*"
+}
+
+reset_rebase () {
+	git rebase --abort # may fail; ignore exit code
+	git reset --hard &&
+	git clean -f
+}
diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
new file mode 100755
index 0000000..c4b32db
--- /dev/null
+++ b/t/t3420-rebase-topology-linear.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='basic rebase topology tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+# a---b---c
+#      \
+#       d---e
+test_expect_success 'setup' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "simple rebase $*" "
+		reset_rebase &&
+		git rebase $* c e &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd e' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* is no-op if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* b e &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f rewrites even if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* -f b e &&
+		! test_cmp_rev e HEAD &&
+		test_cmp_rev b HEAD~2 &&
+		test_linear_range 'd e' b..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* fast-forwards if an ancestor of upstream" "
+		reset_rebase &&
+		git rebase $* e b &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_done
-- 
1.8.2.674.gd17d3d2
Martin von Zweigbergk· May 31, 2013, 06:49 UTC · re: Martin von Zweigbergk · lore

[PATCH v3 2/7] add tests for rebasing with patch-equivalence present

---
 t/lib-rebase.sh                   | 17 ++++++++
 t/t3420-rebase-topology-linear.sh | 85 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)
Show changes to 2 files +102 −0

t/lib-rebase.sh, t/t3420-rebase-topology-linear.sh

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 62b3887..16eeb1c 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -80,3 +80,20 @@ reset_rebase () {
 	git reset --hard &&
 	git clean -f
 }
+
+cherry_pick () {
+	git cherry-pick -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+revert () {
+	git revert -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+make_empty () {
+	git commit --allow-empty -m "$1" &&
+	git tag "$1"
+}
diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
index c4b32db..75cc476 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -75,4 +75,89 @@ test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#
+# uppercase = cherry-picked
+# h = reverted g
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for range selection tests' '
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops patches in upstream" "
+		reset_rebase &&
+		git rebase $* h i &&
+		test_cmp_rev h HEAD~2 &&
+		test_linear_range 'd i' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* can drop last patch if in upstream" "
+		reset_rebase &&
+		git rebase $* h G &&
+		test_cmp_rev h HEAD^ &&
+		test_linear_range 'd' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto drops patches in upstream" "
+		reset_rebase &&
+		git rebase $* --onto f h i &&
+		test_cmp_rev f HEAD~2 &&
+		test_linear_range 'd i' f..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto does not drop patches in onto" "
+		reset_rebase &&
+		git rebase $* --onto h f i &&
+		test_cmp_rev h HEAD~3 &&
+		test_linear_range 'd G i' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
 test_done
-- 
1.8.2.674.gd17d3d2
Martin von Zweigbergk· May 31, 2013, 06:49 UTC · re: Martin von Zweigbergk · lore

[PATCH v3 3/7] add tests for rebasing of empty commits

---
 t/t3401-rebase-partial.sh         | 24 ----------------
 t/t3420-rebase-topology-linear.sh | 58 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 24 deletions(-)
Show changes to 2 files +58 −24

t/t3401-rebase-partial.sh, t/t3420-rebase-topology-linear.sh

diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index 58f4823..7ba1797 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -42,28 +42,4 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr
 	test_path_is_missing .git/rebase-merge
 '
 
-test_expect_success 'rebase ignores empty commit' '
-	git reset --hard A &&
-	git commit --allow-empty -m empty &&
-	test_commit D &&
-	git rebase C &&
-	test "$(git log --format=%s C..)" = "D"
-'
-
-test_expect_success 'rebase --keep-empty' '
-	git reset --hard D &&
-	git rebase --keep-empty C &&
-	test "$(git log --format=%s C..)" = "D
-empty"
-'
-
-test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' '
-	git reset --hard A &&
-	git commit --allow-empty -m also-empty &&
-	git rebase --keep-empty D &&
-	test "$(git log --format=%s A..)" = "also-empty
-D
-empty"
-'
-
 test_done
diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
index 75cc476..81e3d59 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -160,4 +160,62 @@ test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
+# a---b---c---j!
+#      \
+#       d---k!--l
+#
+# ! = empty
+test_expect_success 'setup of linear history for empty commit tests' '
+	git checkout c &&
+	make_empty j &&
+	git checkout d &&
+	make_empty k &&
+	test_commit l
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops empty commit" "
+		reset_rebase &&
+		git rebase $* c l &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty" "
+		reset_rebase &&
+		git rebase $* --keep-empty c l &&
+		test_cmp_rev c HEAD~3 &&
+		test_linear_range 'd k l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty keeps empty even if already in upstream" "
+		reset_rebase &&
+		git rebase $* --keep-empty j l &&
+		test_cmp_rev j HEAD~3 &&
+		test_linear_range 'd k l' j..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
 test_done
-- 
1.8.2.674.gd17d3d2
Martin von Zweigbergk· May 31, 2013, 06:49 UTC · re: Martin von Zweigbergk · lore

[PATCH v3 4/7] add tests for rebasing root

---
 t/t3420-rebase-topology-linear.sh | 129 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)
Show changes to t/t3420-rebase-topology-linear.sh +129 −0
diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
index 81e3d59..659a7b3 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -218,4 +218,133 @@ test_run_rebase failure -m
 test_run_rebase failure -i
 test_run_rebase failure -p
 
+#       m
+#      /
+# a---b---c---g
+#
+# x---y---B
+#
+# uppercase = cherry-picked
+# m = reverted b
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for test involving root' '
+	git checkout b &&
+	revert m b &&
+	git checkout --orphan disjoint &&
+	git rm -rf . &&
+	test_commit x &&
+	test_commit y &&
+	cherry_pick B b
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root" "
+		reset_rebase &&
+		git rebase $* --onto c --root y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history" "
+		reset_rebase &&
+		git rebase $* c y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root drops patch in onto" "
+		reset_rebase &&
+		git rebase $* --onto m --root B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root with merge-base does not go to root" "
+		reset_rebase &&
+		git rebase $* --onto m --root g &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'c g' m..
+	"
+}
+
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history drops patch in onto" "
+		reset_rebase &&
+		git rebase $* m B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --root on linear history is a no-op" "
+		reset_rebase &&
+		git rebase $* --root c &&
+		test_cmp_rev c HEAD
+	"
+}
+test_run_rebase failure ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f --root on linear history causes re-write" "
+		reset_rebase &&
+		git rebase $* -f --root c &&
+		! test_cmp_rev a HEAD~2 &&
+		test_linear_range 'a b c' HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
 test_done
-- 
1.8.2.674.gd17d3d2
Martin von Zweigbergk· May 31, 2013, 06:49 UTC · re: Martin von Zweigbergk · lore

[PATCH v3 5/7] add tests for rebasing merged history

---
 t/t3400-rebase.sh                 |  31 +----
 t/t3401-rebase-partial.sh         |  45 -------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3409-rebase-preserve-merges.sh |  53 --------
 t/t3425-rebase-topology-merges.sh | 252 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 254 insertions(+), 137 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh
Show changes to 5 files +254 −137

t/t3400-rebase.sh, t/t3401-rebase-partial.sh, t/t3404-rebase-interactive.sh, t/t3409-rebase-preserve-merges.sh, t/t3425-rebase-topology-merges.sh

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..b436ef4 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' '
 	echo Side >>C &&
 	git add C &&
 	git commit -m "Add C" &&
-	git checkout -b nonlinear my-topic-branch &&
-	echo Edit >>B &&
-	git add B &&
-	git commit -m "Modify B" &&
-	git merge side &&
-	git checkout -b upstream-merged-nonlinear &&
-	git merge master &&
 	git checkout -f my-topic-branch &&
 	git tag topic
 '
@@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' '
 	git rebase master
 '
 
-test_expect_success 'rebase after merge master' '
-	git checkout --detach refs/tags/topic &&
-	git branch -D topic &&
-	git reset --hard topic &&
-	git merge master &&
-	git rebase master &&
-	! (git show | grep "^Merge:")
-'
-
-test_expect_success 'rebase of history with merges is linearized' '
-	git checkout nonlinear &&
-	test 4 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
-test_expect_success 'rebase of history with merges after upstream merge is linearized' '
-	git checkout upstream-merged-nonlinear &&
-	test 5 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
 test_expect_success 'rebase a single mode change' '
 	git checkout master &&
+	git branch -D topic &&
 	echo 1 >X &&
 	git add X &&
 	test_tick &&
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
deleted file mode 100755
index 7ba1797..0000000
--- a/t/t3401-rebase-partial.sh
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland
-#
-
-test_description='git rebase should detect patches integrated upstream
-
-This test cherry-picks one local change of two into master branch, and
-checks that git rebase succeeds with only the second patch in the
-local branch.
-'
-. ./test-lib.sh
-
-test_expect_success 'prepare repository with topic branch' '
-	test_commit A &&
-	git checkout -b my-topic-branch &&
-	test_commit B &&
-	test_commit C &&
-	git checkout -f master &&
-	test_commit A2 A.t
-'
-
-test_expect_success 'pick top patch from topic branch into master' '
-	git cherry-pick C &&
-	git checkout -f my-topic-branch
-'
-
-test_debug '
-	git cherry master &&
-	git format-patch -k --stdout --full-index master >/dev/null &&
-	gitk --all & sleep 1
-'
-
-test_expect_success 'rebase topic branch against new master and check git am did not get halted' '
-	git rebase master &&
-	test_path_is_missing .git/rebase-apply
-'
-
-test_expect_success 'rebase --merge topic branch that was partially merged upstream' '
-	git reset --hard C &&
-	git rebase --merge master &&
-	test_path_is_missing .git/rebase-merge
-'
-
-test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index a58406d..ffcaf02 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' '
 	test $one = $(git rev-parse HEAD~2)
 '
 
-test_expect_success 'ignore patch if in upstream' '
-	HEAD=$(git rev-parse HEAD) &&
-	git checkout -b has-cherry-picked HEAD^ &&
+test_expect_success '--continue tries to commit, even for "edit"' '
 	echo unrelated > file7 &&
 	git add file7 &&
 	test_tick &&
 	git commit -m "unrelated change" &&
-	git cherry-pick $HEAD &&
-	EXPECT_COUNT=1 git rebase -i $HEAD &&
-	test $HEAD = $(git rev-parse HEAD^)
-'
-
-test_expect_success '--continue tries to commit, even for "edit"' '
 	parent=$(git rev-parse HEAD^) &&
 	test_tick &&
 	FAKE_LINES="edit 1" git rebase -i HEAD^ &&
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 6de4e22..2e0c364 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -11,14 +11,6 @@ Run "git rebase -p" and check that merges are properly carried along
 GIT_AUTHOR_EMAIL=bogus_email_address
 export GIT_AUTHOR_EMAIL
 
-# Clone 1 (trivial merge):
-#
-# A1--A2  <-- origin/master
-#  \   \
-#   B1--M  <-- topic
-#    \
-#     B2  <-- origin/topic
-#
 # Clone 2 (conflicting merge):
 #
 # A1--A2--B3   <-- origin/master
@@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL
 #     \--A3    <-- topic2
 #      \
 #       B2     <-- origin/topic
-#
-# Clone 4 (merge using second parent as base):
-#
-# A1--A2--B3   <-- origin/master
-#  \
-#   B1--A3--M  <-- topic
-#    \     /
-#     \--A4    <-- topic2
-#      \
-#       B2     <-- origin/topic
 
 test_expect_success 'setup for merge-preserving rebase' \
 	'echo First > A &&
@@ -58,20 +40,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git checkout -f master &&
 	echo Third >> A &&
 	git commit -a -m "Modify A2" &&
-
-	git clone ./. clone1 &&
-	(cd clone1 &&
-	git checkout -b topic origin/topic &&
-	git merge origin/master
-	) &&
-
-	git clone ./. clone4 &&
-	(
-		cd clone4 &&
-		git checkout -b topic origin/topic &&
-		git merge origin/master
-	) &&
-
 	echo Fifth > B &&
 	git add B &&
 	git commit -m "Add different B" &&
@@ -101,16 +69,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git commit -a -m "Modify B2"
 '
 
-test_expect_success 'rebase -p fakes interactive rebase' '
-	(
-	cd clone1 &&
-	git fetch &&
-	git rebase -p origin/topic &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_expect_success '--continue works after a conflict' '
 	(
 	cd clone2 &&
@@ -138,15 +96,4 @@ test_expect_success 'rebase -p preserves no-ff merges' '
 	)
 '
 
-test_expect_success 'rebase -p works when base inside second parent' '
-	(
-	cd clone4 &&
-	git fetch &&
-	git rebase -p HEAD^2 &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify B" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_done
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
new file mode 100755
index 0000000..72fa76d
--- /dev/null
+++ b/t/t3425-rebase-topology-merges.sh
@@ -0,0 +1,252 @@
+#!/bin/sh
+
+test_description='rebase topology tests with merges'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+test_revision_subjects () {
+	expected="$1"
+	shift
+	set -- $(git log --format=%s --no-walk=unsorted "$@")
+	test "$expected" = "$*"
+}
+
+# a---b-----------c
+#      \           \
+#       d-------e   \
+#        \       \   \
+#         n---o---w---v
+#              \
+#               z
+test_expect_success 'setup of non-linear-history' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+
+	git checkout d &&
+	test_commit n &&
+	test_commit o &&
+	test_merge w e &&
+	test_merge v c &&
+	git checkout o &&
+	test_commit z
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* after merge from upstream" "
+		reset_rebase &&
+		git rebase $* e w &&
+		test_cmp_rev e HEAD~2 &&
+		test_linear_range 'n o' e..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized in place" "
+		reset_rebase &&
+		git rebase $* d w &&
+		test_cmp_rev d HEAD~3 &&
+		test_linear_range "\'"$expected"\'" d..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'e n o' ''
+test_run_rebase success 'e n o' -m
+test_run_rebase success 'n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized upstream" "
+		reset_rebase &&
+		git rebase $* c w &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history with merges after upstream merge is linearized" "
+		reset_rebase &&
+		git rebase $* c v &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_expect_success "rebase -p is no-op in non-linear history" "
+	reset_rebase &&
+	git rebase -p d w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p is no-op when base inside second parent" "
+	reset_rebase &&
+	git rebase -p e w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_failure "rebase -p --root on non-linear history is a no-op" "
+	reset_rebase &&
+	git rebase -p --root w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p re-creates merge from side branch" "
+	reset_rebase &&
+	git rebase -p z w &&
+	test_cmp_rev z HEAD^ &&
+	test_cmp_rev w^2 HEAD^2
+"
+
+test_expect_success "rebase -p re-creates internal merge" "
+	reset_rebase &&
+	git rebase -p c w &&
+	test_cmp_rev c HEAD~4 &&
+	test_revision_subjects 'd n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p can re-create two branches on onto" "
+	reset_rebase &&
+	git rebase -p --onto c d w &&
+	test_cmp_rev c HEAD~3 &&
+	test_revision_subjects 'n e o w' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#        \       \
+#         e-------u
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for patch-equivalence tests' '
+	git checkout e &&
+	test_merge u i
+'
+
+test_expect_success "rebase -p re-creates history around dropped commit matching upstream" "
+	reset_rebase &&
+	git rebase -p h u &&
+	test_cmp_rev h HEAD~3 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history drops patches in upstream" "
+	reset_rebase &&
+	git rebase -p --onto f h u &&
+	test_cmp_rev f HEAD~3 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history does not drop patches in onto" "
+	reset_rebase &&
+	git rebase -p --onto h f u &&
+	test_cmp_rev h HEAD~3 &&
+	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+"
+
+# a---b---c---g---h
+#      \
+#       d---G---s
+#        \   \ /
+#         \   X
+#          \ / \
+#           e---t
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for dropping whole side' '
+	git checkout G &&
+	test_merge s e &&
+	git checkout e &&
+	test_merge t G
+'
+
+test_expect_failure "rebase -p drops merge commit when entire first-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h s &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+test_expect_success "rebase -p drops merge commit when entire second-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h t &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+# a---b---c
+#      \
+#       d---e
+#        \   \
+#         n---r
+#          \
+#           o
+#
+# r = tree-same with n
+test_expect_success 'setup of non-linear-history for empty commits' '
+	git checkout n &&
+	git merge --no-commit e &&
+	git reset n . &&
+	git commit -m r &&
+	git reset --hard &&
+	git clean -f &&
+	git tag r
+'
+
+test_expect_success "rebase -p re-creates empty internal merge commit" "
+	reset_rebase &&
+	git rebase -p c r &&
+	test_cmp_rev c HEAD~3 &&
+	test_revision_subjects 'd e n r' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p re-creates empty merge commit" "
+	reset_rebase &&
+	git rebase -p o r &&
+	test_cmp_rev e HEAD^2 &&
+	test_cmp_rev o HEAD^ &&
+	test_revision_subjects 'r' HEAD
+"
+
+test_done
-- 
1.8.2.674.gd17d3d2
Johannes Sixt· May 31, 2013, 12:19 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v3 5/7] add tests for rebasing merged history

Am 31.05.2013 08:49, schrieb Martin von Zweigbergk:
Show 10 quoted lines
> +#       f
> +#      /
> +# a---b---c---g---h
> +#      \
> +#       d---G---i
> +#        \       \
> +#         e-------u
> +#
> +# uppercase = cherry-picked
> +# h = reverted g
...
Show 13 quoted lines
> +test_expect_success "rebase -p --onto in merged history drops patches in upstream" "
> +	reset_rebase &&
> +	git rebase -p --onto f h u &&
> +	test_cmp_rev f HEAD~3 &&
> +	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
> +"
> +
> +test_expect_success "rebase -p --onto in merged history does not drop patches in onto" "
> +	reset_rebase &&
> +	git rebase -p --onto h f u &&
> +	test_cmp_rev h HEAD~3 &&
> +	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
> +"
I noticed one new aspect:
The interdiff between v2 and v3 looks like this:
-test_expect_failure "rebase -p --onto in merged history does not lose patches in upstream" "
+test_expect_success "rebase -p --onto in merged history drops patches in upstream" "
 	reset_rebase &&
 	git rebase -p --onto f h u &&
 	test_cmp_rev f HEAD~3 &&
-	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
 "
 
-test_expect_success "rebase -p --onto in merged history drops patches in onto" "
+test_expect_success "rebase -p --onto in merged history does not drop patches in onto" "
 	reset_rebase &&
 	git rebase -p --onto h f u &&
 	test_cmp_rev h HEAD~3 &&
-	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD

The expectations that these two tests check changed from v2 to v3. Notice that former test goes from expect_failure to expect_success, as it should, but the latter does not change. Strange, isn't it?

The reason is that this check is incomplete:
	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
and allowed the latter test in the v2 form to pass.
It should be
	test_revision_subjects 'd i d e u' HEAD^2^ HEAD^2 HEAD~2 HEAD^ HEAD
The check of the latter test should be:
	test_revision_subjects 'd G i d e u' HEAD^2~2 HEAD^2^ HEAD^2 HEAD~2 HEAD^ HEAD

i.e. check all the way back to the mergebase via both branches. This can be extrapolated to all tests that reconstruct mergy history (not just these two cases).

-- Hannes
Martin von Zweigbergk· Jun 1, 2013, 21:36 UTC · re: Johannes Sixt · lore

[PATCH v4 5/7] add tests for rebasing merged history

---
> The reason is that this check is incomplete:
>
>    test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD

Nice catch! This should fix it. I couldn't use the method you suggested because of how test_revision_subjects works (repeated revisions are ignored), but this makes the check stricter anyway.

Junio, all the previous patches are unchanged since v3, so I'm not resending them. Thanks.

 t/t3400-rebase.sh                 |  31 +----
 t/t3401-rebase-partial.sh         |  45 -------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3409-rebase-preserve-merges.sh |  53 --------
 t/t3425-rebase-topology-merges.sh | 258 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 260 insertions(+), 137 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh
Show changes to 5 files +260 −137

t/t3400-rebase.sh, t/t3401-rebase-partial.sh, t/t3404-rebase-interactive.sh, t/t3409-rebase-preserve-merges.sh, t/t3425-rebase-topology-merges.sh

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..b436ef4 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' '
 	echo Side >>C &&
 	git add C &&
 	git commit -m "Add C" &&
-	git checkout -b nonlinear my-topic-branch &&
-	echo Edit >>B &&
-	git add B &&
-	git commit -m "Modify B" &&
-	git merge side &&
-	git checkout -b upstream-merged-nonlinear &&
-	git merge master &&
 	git checkout -f my-topic-branch &&
 	git tag topic
 '
@@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' '
 	git rebase master
 '
 
-test_expect_success 'rebase after merge master' '
-	git checkout --detach refs/tags/topic &&
-	git branch -D topic &&
-	git reset --hard topic &&
-	git merge master &&
-	git rebase master &&
-	! (git show | grep "^Merge:")
-'
-
-test_expect_success 'rebase of history with merges is linearized' '
-	git checkout nonlinear &&
-	test 4 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
-test_expect_success 'rebase of history with merges after upstream merge is linearized' '
-	git checkout upstream-merged-nonlinear &&
-	test 5 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
 test_expect_success 'rebase a single mode change' '
 	git checkout master &&
+	git branch -D topic &&
 	echo 1 >X &&
 	git add X &&
 	test_tick &&
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
deleted file mode 100755
index 7ba1797..0000000
--- a/t/t3401-rebase-partial.sh
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland
-#
-
-test_description='git rebase should detect patches integrated upstream
-
-This test cherry-picks one local change of two into master branch, and
-checks that git rebase succeeds with only the second patch in the
-local branch.
-'
-. ./test-lib.sh
-
-test_expect_success 'prepare repository with topic branch' '
-	test_commit A &&
-	git checkout -b my-topic-branch &&
-	test_commit B &&
-	test_commit C &&
-	git checkout -f master &&
-	test_commit A2 A.t
-'
-
-test_expect_success 'pick top patch from topic branch into master' '
-	git cherry-pick C &&
-	git checkout -f my-topic-branch
-'
-
-test_debug '
-	git cherry master &&
-	git format-patch -k --stdout --full-index master >/dev/null &&
-	gitk --all & sleep 1
-'
-
-test_expect_success 'rebase topic branch against new master and check git am did not get halted' '
-	git rebase master &&
-	test_path_is_missing .git/rebase-apply
-'
-
-test_expect_success 'rebase --merge topic branch that was partially merged upstream' '
-	git reset --hard C &&
-	git rebase --merge master &&
-	test_path_is_missing .git/rebase-merge
-'
-
-test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index a58406d..ffcaf02 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' '
 	test $one = $(git rev-parse HEAD~2)
 '
 
-test_expect_success 'ignore patch if in upstream' '
-	HEAD=$(git rev-parse HEAD) &&
-	git checkout -b has-cherry-picked HEAD^ &&
+test_expect_success '--continue tries to commit, even for "edit"' '
 	echo unrelated > file7 &&
 	git add file7 &&
 	test_tick &&
 	git commit -m "unrelated change" &&
-	git cherry-pick $HEAD &&
-	EXPECT_COUNT=1 git rebase -i $HEAD &&
-	test $HEAD = $(git rev-parse HEAD^)
-'
-
-test_expect_success '--continue tries to commit, even for "edit"' '
 	parent=$(git rev-parse HEAD^) &&
 	test_tick &&
 	FAKE_LINES="edit 1" git rebase -i HEAD^ &&
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 6de4e22..2e0c364 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -11,14 +11,6 @@ Run "git rebase -p" and check that merges are properly carried along
 GIT_AUTHOR_EMAIL=bogus_email_address
 export GIT_AUTHOR_EMAIL
 
-# Clone 1 (trivial merge):
-#
-# A1--A2  <-- origin/master
-#  \   \
-#   B1--M  <-- topic
-#    \
-#     B2  <-- origin/topic
-#
 # Clone 2 (conflicting merge):
 #
 # A1--A2--B3   <-- origin/master
@@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL
 #     \--A3    <-- topic2
 #      \
 #       B2     <-- origin/topic
-#
-# Clone 4 (merge using second parent as base):
-#
-# A1--A2--B3   <-- origin/master
-#  \
-#   B1--A3--M  <-- topic
-#    \     /
-#     \--A4    <-- topic2
-#      \
-#       B2     <-- origin/topic
 
 test_expect_success 'setup for merge-preserving rebase' \
 	'echo First > A &&
@@ -58,20 +40,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git checkout -f master &&
 	echo Third >> A &&
 	git commit -a -m "Modify A2" &&
-
-	git clone ./. clone1 &&
-	(cd clone1 &&
-	git checkout -b topic origin/topic &&
-	git merge origin/master
-	) &&
-
-	git clone ./. clone4 &&
-	(
-		cd clone4 &&
-		git checkout -b topic origin/topic &&
-		git merge origin/master
-	) &&
-
 	echo Fifth > B &&
 	git add B &&
 	git commit -m "Add different B" &&
@@ -101,16 +69,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git commit -a -m "Modify B2"
 '
 
-test_expect_success 'rebase -p fakes interactive rebase' '
-	(
-	cd clone1 &&
-	git fetch &&
-	git rebase -p origin/topic &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_expect_success '--continue works after a conflict' '
 	(
 	cd clone2 &&
@@ -138,15 +96,4 @@ test_expect_success 'rebase -p preserves no-ff merges' '
 	)
 '
 
-test_expect_success 'rebase -p works when base inside second parent' '
-	(
-	cd clone4 &&
-	git fetch &&
-	git rebase -p HEAD^2 &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify B" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_done
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
new file mode 100755
index 0000000..efd7dfd
--- /dev/null
+++ b/t/t3425-rebase-topology-merges.sh
@@ -0,0 +1,258 @@
+#!/bin/sh
+
+test_description='rebase topology tests with merges'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+test_revision_subjects () {
+	expected="$1"
+	shift
+	set -- $(git log --format=%s --no-walk=unsorted "$@")
+	test "$expected" = "$*"
+}
+
+# a---b-----------c
+#      \           \
+#       d-------e   \
+#        \       \   \
+#         n---o---w---v
+#              \
+#               z
+test_expect_success 'setup of non-linear-history' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+
+	git checkout d &&
+	test_commit n &&
+	test_commit o &&
+	test_merge w e &&
+	test_merge v c &&
+	git checkout o &&
+	test_commit z
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* after merge from upstream" "
+		reset_rebase &&
+		git rebase $* e w &&
+		test_cmp_rev e HEAD~2 &&
+		test_linear_range 'n o' e..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized in place" "
+		reset_rebase &&
+		git rebase $* d w &&
+		test_cmp_rev d HEAD~3 &&
+		test_linear_range "\'"$expected"\'" d..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'e n o' ''
+test_run_rebase success 'e n o' -m
+test_run_rebase success 'n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized upstream" "
+		reset_rebase &&
+		git rebase $* c w &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history with merges after upstream merge is linearized" "
+		reset_rebase &&
+		git rebase $* c v &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_expect_success "rebase -p is no-op in non-linear history" "
+	reset_rebase &&
+	git rebase -p d w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p is no-op when base inside second parent" "
+	reset_rebase &&
+	git rebase -p e w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_failure "rebase -p --root on non-linear history is a no-op" "
+	reset_rebase &&
+	git rebase -p --root w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p re-creates merge from side branch" "
+	reset_rebase &&
+	git rebase -p z w &&
+	test_cmp_rev z HEAD^ &&
+	test_cmp_rev w^2 HEAD^2
+"
+
+test_expect_success "rebase -p re-creates internal merge" "
+	reset_rebase &&
+	git rebase -p c w &&
+	test_cmp_rev c HEAD~4 &&
+	test_cmp_rev HEAD^2^ HEAD~3 &&
+	test_revision_subjects 'd n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p can re-create two branches on onto" "
+	reset_rebase &&
+	git rebase -p --onto c d w &&
+	test_cmp_rev c HEAD~3 &&
+	test_cmp_rev c HEAD^2^ &&
+	test_revision_subjects 'n e o w' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#        \       \
+#         e-------u
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for patch-equivalence tests' '
+	git checkout e &&
+	test_merge u i
+'
+
+test_expect_success "rebase -p re-creates history around dropped commit matching upstream" "
+	reset_rebase &&
+	git rebase -p h u &&
+	test_cmp_rev h HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history drops patches in upstream" "
+	reset_rebase &&
+	git rebase -p --onto f h u &&
+	test_cmp_rev f HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history does not drop patches in onto" "
+	reset_rebase &&
+	git rebase -p --onto h f u &&
+	test_cmp_rev h HEAD~3 &&
+	test_cmp_rev HEAD^2~2 HEAD~2 &&
+	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+"
+
+# a---b---c---g---h
+#      \
+#       d---G---s
+#        \   \ /
+#         \   X
+#          \ / \
+#           e---t
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for dropping whole side' '
+	git checkout G &&
+	test_merge s e &&
+	git checkout e &&
+	test_merge t G
+'
+
+test_expect_failure "rebase -p drops merge commit when entire first-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h s &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+test_expect_success "rebase -p drops merge commit when entire second-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h t &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+# a---b---c
+#      \
+#       d---e
+#        \   \
+#         n---r
+#          \
+#           o
+#
+# r = tree-same with n
+test_expect_success 'setup of non-linear-history for empty commits' '
+	git checkout n &&
+	git merge --no-commit e &&
+	git reset n . &&
+	git commit -m r &&
+	git reset --hard &&
+	git clean -f &&
+	git tag r
+'
+
+test_expect_success "rebase -p re-creates empty internal merge commit" "
+	reset_rebase &&
+	git rebase -p c r &&
+	test_cmp_rev c HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd e n r' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p re-creates empty merge commit" "
+	reset_rebase &&
+	git rebase -p o r &&
+	test_cmp_rev e HEAD^2 &&
+	test_cmp_rev o HEAD^ &&
+	test_revision_subjects 'r' HEAD
+"
+
+test_done
-- 
1.8.2.674.gd17d3d2
Martin von Zweigbergk· May 31, 2013, 06:49 UTC · re: Martin von Zweigbergk · lore

[PATCH v3 6/7] t3406: modernize style

Update the following:
 - Quote 'setup'
 - Remove blank lines within test case body
 - Use test_commit instead of custom quick_one
 - Create branch "topic" from tag created by test_commit
---
 t/t3406-rebase-message.sh | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)
Show changes to t/t3406-rebase-message.sh +9 −21
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index e6a9a0d..fe8c27f 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -4,27 +4,17 @@ test_description='messages from rebase operation'
 
 . ./test-lib.sh
 
-quick_one () {
-	echo "$1" >"file$1" &&
-	git add "file$1" &&
-	test_tick &&
-	git commit -m "$1"
-}
+test_expect_success 'setup' '
+	test_commit O fileO &&
+	test_commit X fileX &&
+	test_commit A fileA &&
+	test_commit B fileB &&
+	test_commit Y fileY &&
 
-test_expect_success setup '
-	quick_one O &&
-	git branch topic &&
-	quick_one X &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Y &&
-
-	git checkout topic &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Z &&
+	git checkout -b topic O &&
+	git cherry-pick A B &&
+	test_commit Z fileZ &&
 	git tag start
-
 '
 
 cat >expect <<\EOF
@@ -34,12 +24,10 @@ Committed: 0003 Z
 EOF
 
 test_expect_success 'rebase -m' '
-
 	git rebase -m master >report &&
 	sed -n -e "/^Already applied: /p" \
 		-e "/^Committed: /p" report >actual &&
 	test_cmp expect actual
-
 '
 
 test_expect_success 'rebase --stat' '
-- 
1.8.2.674.gd17d3d2
Martin von Zweigbergk· May 31, 2013, 06:49 UTC · re: Martin von Zweigbergk · lore

[PATCH v3 7/7] tests: move test for rebase messages from t3400 to t3406

t3406 is supposed to test "messages from rebase operation", so let's
move tests in t3400 that fit that description into 3406. Most of the
functionality they tested, except for the messages, has now been
subsumed by t3420.
---
 t/t3400-rebase.sh         | 22 ----------------------
 t/t3406-rebase-message.sh | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 22 deletions(-)
Show changes to 2 files +22 −22

t/t3400-rebase.sh, t/t3406-rebase-message.sh

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b436ef4..45a55e9 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -59,28 +59,6 @@ test_expect_success 'rebase against master' '
 	git rebase master
 '
 
-test_expect_success 'rebase against master twice' '
-	git rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase against master twice with --force' '
-	git rebase --force-rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date, rebase forced" out
-'
-
-test_expect_success 'rebase against master twice from another branch' '
-	git checkout my-topic-branch^ &&
-	git rebase master my-topic-branch >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase fast-forward to master' '
-	git checkout my-topic-branch^ &&
-	git rebase my-topic-branch >out &&
-	test_i18ngrep "Fast-forwarded HEAD to my-topic-branch" out
-'
-
 test_expect_success 'the rebase operation should not have destroyed author information' '
 	! (git log | grep "Author:" | grep "<>")
 '
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index fe8c27f..0392e36 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -30,6 +30,28 @@ test_expect_success 'rebase -m' '
 	test_cmp expect actual
 '
 
+test_expect_success 'rebase against master twice' '
+	git rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase against master twice with --force' '
+	git rebase --force-rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date, rebase forced" out
+'
+
+test_expect_success 'rebase against master twice from another branch' '
+	git checkout topic^ &&
+	git rebase master topic >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase fast-forward to master' '
+	git checkout topic^ &&
+	git rebase topic >out &&
+	test_i18ngrep "Fast-forwarded HEAD to topic" out
+'
+
 test_expect_success 'rebase --stat' '
 	git reset --hard start &&
         git rebase --stat master >diffstat.txt &&
-- 
1.8.2.674.gd17d3d2
Martin von Zweigbergk· Jun 3, 2013, 20:42 UTC · re: Martin von Zweigbergk · lore

[PATCH v5 0/7] Rebase topology test

The only change since v4 should be that t3420 was renamed t3421.
Martin von Zweigbergk (7):
  add simple tests of consistency across rebase types
  add tests for rebasing with patch-equivalence present
  add tests for rebasing of empty commits
  add tests for rebasing root
  add tests for rebasing merged history
  t3406: modernize style
  tests: move test for rebase messages from t3400 to t3406
 t/lib-rebase.sh                   |  32 ++++
 t/t3400-rebase.sh                 |  53 +-----
 t/t3401-rebase-partial.sh         |  69 --------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3406-rebase-message.sh         |  50 +++---
 t/t3409-rebase-preserve-merges.sh |  53 ------
 t/t3421-rebase-topology-linear.sh | 350 ++++++++++++++++++++++++++++++++++++++
 t/t3425-rebase-topology-merges.sh | 258 ++++++++++++++++++++++++++++
 8 files changed, 672 insertions(+), 203 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3421-rebase-topology-linear.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh
-- 
1.8.3.497.g83fddbe
Martin von Zweigbergk· Jun 3, 2013, 20:42 UTC · re: Martin von Zweigbergk · lore

[PATCH v5 1/7] add simple tests of consistency across rebase types

Helped-by: Johannes Sixt <j6t@kdbg.org>
---
 t/lib-rebase.sh                   | 15 ++++++++
 t/t3421-rebase-topology-linear.sh | 78 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)
 create mode 100755 t/t3421-rebase-topology-linear.sh
Show changes to 2 files +93 −0

t/lib-rebase.sh, t/t3421-rebase-topology-linear.sh

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6ccf797..62b3887 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -65,3 +65,18 @@ EOF
 	test_set_editor "$(pwd)/fake-editor.sh"
 	chmod a+x fake-editor.sh
 }
+
+# checks that the revisions in "$2" represent a linear range with the
+# subjects in "$1"
+test_linear_range () {
+	! { git log --format=%p "$2" | sane_grep " " ;} &&
+	expected=$1
+	set -- $(git log --reverse --format=%s "$2")
+	test "$expected" = "$*"
+}
+
+reset_rebase () {
+	git rebase --abort # may fail; ignore exit code
+	git reset --hard &&
+	git clean -f
+}
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
new file mode 100755
index 0000000..c4b32db
--- /dev/null
+++ b/t/t3421-rebase-topology-linear.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='basic rebase topology tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+# a---b---c
+#      \
+#       d---e
+test_expect_success 'setup' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "simple rebase $*" "
+		reset_rebase &&
+		git rebase $* c e &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd e' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* is no-op if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* b e &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f rewrites even if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* -f b e &&
+		! test_cmp_rev e HEAD &&
+		test_cmp_rev b HEAD~2 &&
+		test_linear_range 'd e' b..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* fast-forwards if an ancestor of upstream" "
+		reset_rebase &&
+		git rebase $* e b &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_done
-- 
1.8.3.497.g83fddbe
Junio C Hamano· Jun 3, 2013, 22:28 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

Martin von Zweigbergk <martinvonz@gmail.com> writes:
Show 20 quoted lines
> Helped-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  t/lib-rebase.sh                   | 15 ++++++++
>  t/t3421-rebase-topology-linear.sh | 78 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 93 insertions(+)
>  create mode 100755 t/t3421-rebase-topology-linear.sh
>
> diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> index 6ccf797..62b3887 100644
> --- a/t/lib-rebase.sh
> +++ b/t/lib-rebase.sh
> @@ -65,3 +65,18 @@ EOF
>  	test_set_editor "$(pwd)/fake-editor.sh"
>  	chmod a+x fake-editor.sh
>  }
> +
> +# checks that the revisions in "$2" represent a linear range with the
> +# subjects in "$1"
> +test_linear_range () {
> +	! { git log --format=%p "$2" | sane_grep " " ;} &&
An interesting way to spell:
    test $(git rev-list --merges "$2" | wc -l) = 0
I think I am fine with either, though.
> +	expected=$1
> +	set -- $(git log --reverse --format=%s "$2")
> +	test "$expected" = "$*"
OK.
> +}
> +
> +reset_rebase () {
> +	git rebase --abort # may fail; ignore exit code
test_might_fail to catch unusual exit codes?
Show 62 quoted lines
> +	git reset --hard &&
> +	git clean -f
> +}
> diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
> new file mode 100755
> index 0000000..c4b32db
> --- /dev/null
> +++ b/t/t3421-rebase-topology-linear.sh
> @@ -0,0 +1,78 @@
> +#!/bin/sh
> +
> +test_description='basic rebase topology tests'
> +. ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-rebase.sh
> +
> +# a---b---c
> +#      \
> +#       d---e
> +test_expect_success 'setup' '
> +	test_commit a &&
> +	test_commit b &&
> +	test_commit c &&
> +	git checkout b &&
> +	test_commit d &&
> +	test_commit e
> +'
> +
> +test_run_rebase () {
> +	result=$1
> +	shift
> +	test_expect_$result "simple rebase $*" "
> +		reset_rebase &&
> +		git rebase $* c e &&
> +		test_cmp_rev c HEAD~2 &&
> +		test_linear_range 'd e' c..
> +	"
> +}
> +test_run_rebase success ''
> +test_run_rebase success -m
> +test_run_rebase success -i
> +test_run_rebase success -p
> +
> +test_run_rebase () {
> +	result=$1
> +	shift
> +	test_expect_$result "rebase $* is no-op if upstream is an ancestor" "
> +		reset_rebase &&
> +		git rebase $* b e &&
> +		test_cmp_rev e HEAD
> +	"
> +}
> +test_run_rebase success ''
> +test_run_rebase success -m
> +test_run_rebase success -i
> +test_run_rebase success -p
> +
> +test_run_rebase () {
> +	result=$1
> +	shift
> +	test_expect_$result "rebase $* -f rewrites even if upstream is an ancestor" "
> +		reset_rebase &&
> +		git rebase $* -f b e &&

Asking to rebase the history leading to e from b on top of the merge base (which happens to be b) may be no-op or force-create a new history that is parallel. OK.

Show 14 quoted lines
> +		! test_cmp_rev e HEAD &&
> +		test_cmp_rev b HEAD~2 &&
> +		test_linear_range 'd e' b..
> +	"
> +}
> +test_run_rebase success ''
> +test_run_rebase success -m
> +test_run_rebase success -i
> +test_run_rebase failure -p
> +
> +test_run_rebase () {
> +	result=$1
> +	shift
> +	test_expect_$result "rebase $* fast-forwards if an ancestor of upstream" "

The description is a non-sentence, and while I can tell what it wants to say, I do not have a good suggestion for rephrasing this.

This is asking to rebase the history leading to b on top of e, but e already includes everything in b, so it just turns into a no-op of not moving from e. So it is not even a fast-forward.

Show 11 quoted lines
> +		reset_rebase &&
> +		git rebase $* e b &&
> +		test_cmp_rev e HEAD
> +	"
> +}
> +test_run_rebase success ''
> +test_run_rebase success -m
> +test_run_rebase success -i
> +test_run_rebase success -p
> +
> +test_done
Martin von Zweigbergk· Jun 4, 2013, 05:14 UTC · re: Junio C Hamano · lore

Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
>> +
>> +# checks that the revisions in "$2" represent a linear range with the
>> +# subjects in "$1"
>> +test_linear_range () {
>> +     ! { git log --format=%p "$2" | sane_grep " " ;} &&
>
> An interesting way to spell:
>
>     test $(git rev-list --merges "$2" | wc -l) = 0

Heh, true. I'll change that. ("My" version was based on the one in git-rebase.sh, around line 495.)

>> +reset_rebase () {
>> +     git rebase --abort # may fail; ignore exit code
>
> test_might_fail to catch unusual exit codes?
Will change.
>> +# a---b---c
>> +#      \
>> +#       d---e
Show 7 quoted lines
>> +test_run_rebase () {
>> +     result=$1
>> +     shift
>> +     test_expect_$result "rebase $* fast-forwards if an ancestor of upstream" "
>
> The description is a non-sentence, and while I can tell what it
> wants to say, I do not have a good suggestion for rephrasing this.
Changing description to "... fast-forwards from an ancestor of upstream".
Show 7 quoted lines
> This is asking to rebase the history leading to b on top of e, but e
> already includes everything in b, so it just turns into a no-op of
> not moving from e.  So it is not even a fast-forward.
>
>> +             reset_rebase &&
>> +             git rebase $* e b &&
>> +             test_cmp_rev e HEAD

Well, "git rebase e b" is of course a kind of short form of "git checkout b && git rebase e". While it's true that the implementation doesn't bother checking out b first, that's just an optimization, but let me know if you meant something else.

Thanks. Will wait another day or two for further comments before I send another version.

Junio C Hamano· Jun 4, 2013, 05:49 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

Martin von Zweigbergk <martinvonz@gmail.com> writes:
> Thanks. Will wait another day or two for further comments before I
> send another version.
Thanks; I just noticed that your patches lack S-o-b:.
Johannes Sixt· Jun 4, 2013, 06:15 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

Am 6/4/2013 7:14, schrieb Martin von Zweigbergk:
Show 12 quoted lines
> On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> +
>>> +# checks that the revisions in "$2" represent a linear range with the
>>> +# subjects in "$1"
>>> +test_linear_range () {
>>> +     ! { git log --format=%p "$2" | sane_grep " " ;} &&
>>
>> An interesting way to spell:
>>
>>     test $(git rev-list --merges "$2" | wc -l) = 0
> 
> Heh, true. I'll change that.
Then I think it would be even better written as
	revlist_merges=$(git rev-list --merges "$2") &&
	test -z "$revlist_merges"

so as not to ignore errors in the git invocation (and at least one less fork()).

-- Hannes
Martin von Zweigbergk· Jun 5, 2013, 04:31 UTC · re: Johannes Sixt · lore

Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

On Mon, Jun 3, 2013 at 11:15 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
Show 21 quoted lines
> Am 6/4/2013 7:14, schrieb Martin von Zweigbergk:
>> On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> +
>>>> +# checks that the revisions in "$2" represent a linear range with the
>>>> +# subjects in "$1"
>>>> +test_linear_range () {
>>>> +     ! { git log --format=%p "$2" | sane_grep " " ;} &&
>>>
>>> An interesting way to spell:
>>>
>>>     test $(git rev-list --merges "$2" | wc -l) = 0
>>
>> Heh, true. I'll change that.
>
> Then I think it would be even better written as
>
>         revlist_merges=$(git rev-list --merges "$2") &&
>         test -z "$revlist_merges"
>
> so as not to ignore errors in the git invocation (and at least one less
> fork()).
Done. I'll send it out in a day or two.
Martin von Zweigbergk· Jun 3, 2013, 20:42 UTC · re: Martin von Zweigbergk · lore

[PATCH v5 2/7] add tests for rebasing with patch-equivalence present

---
 t/lib-rebase.sh                   | 17 ++++++++
 t/t3421-rebase-topology-linear.sh | 85 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)
Show changes to 2 files +102 −0

t/lib-rebase.sh, t/t3421-rebase-topology-linear.sh

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 62b3887..16eeb1c 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -80,3 +80,20 @@ reset_rebase () {
 	git reset --hard &&
 	git clean -f
 }
+
+cherry_pick () {
+	git cherry-pick -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+revert () {
+	git revert -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+make_empty () {
+	git commit --allow-empty -m "$1" &&
+	git tag "$1"
+}
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index c4b32db..75cc476 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -75,4 +75,89 @@ test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#
+# uppercase = cherry-picked
+# h = reverted g
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for range selection tests' '
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops patches in upstream" "
+		reset_rebase &&
+		git rebase $* h i &&
+		test_cmp_rev h HEAD~2 &&
+		test_linear_range 'd i' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* can drop last patch if in upstream" "
+		reset_rebase &&
+		git rebase $* h G &&
+		test_cmp_rev h HEAD^ &&
+		test_linear_range 'd' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto drops patches in upstream" "
+		reset_rebase &&
+		git rebase $* --onto f h i &&
+		test_cmp_rev f HEAD~2 &&
+		test_linear_range 'd i' f..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto does not drop patches in onto" "
+		reset_rebase &&
+		git rebase $* --onto h f i &&
+		test_cmp_rev h HEAD~3 &&
+		test_linear_range 'd G i' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
 test_done
-- 
1.8.3.497.g83fddbe
Martin von Zweigbergk· Jun 3, 2013, 20:42 UTC · re: Martin von Zweigbergk · lore

[PATCH v5 3/7] add tests for rebasing of empty commits

---
 t/t3401-rebase-partial.sh         | 24 ----------------
 t/t3421-rebase-topology-linear.sh | 58 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 24 deletions(-)
Show changes to 2 files +58 −24

t/t3401-rebase-partial.sh, t/t3421-rebase-topology-linear.sh

diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index 58f4823..7ba1797 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -42,28 +42,4 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr
 	test_path_is_missing .git/rebase-merge
 '
 
-test_expect_success 'rebase ignores empty commit' '
-	git reset --hard A &&
-	git commit --allow-empty -m empty &&
-	test_commit D &&
-	git rebase C &&
-	test "$(git log --format=%s C..)" = "D"
-'
-
-test_expect_success 'rebase --keep-empty' '
-	git reset --hard D &&
-	git rebase --keep-empty C &&
-	test "$(git log --format=%s C..)" = "D
-empty"
-'
-
-test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' '
-	git reset --hard A &&
-	git commit --allow-empty -m also-empty &&
-	git rebase --keep-empty D &&
-	test "$(git log --format=%s A..)" = "also-empty
-D
-empty"
-'
-
 test_done
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index 75cc476..81e3d59 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -160,4 +160,62 @@ test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
+# a---b---c---j!
+#      \
+#       d---k!--l
+#
+# ! = empty
+test_expect_success 'setup of linear history for empty commit tests' '
+	git checkout c &&
+	make_empty j &&
+	git checkout d &&
+	make_empty k &&
+	test_commit l
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops empty commit" "
+		reset_rebase &&
+		git rebase $* c l &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty" "
+		reset_rebase &&
+		git rebase $* --keep-empty c l &&
+		test_cmp_rev c HEAD~3 &&
+		test_linear_range 'd k l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty keeps empty even if already in upstream" "
+		reset_rebase &&
+		git rebase $* --keep-empty j l &&
+		test_cmp_rev j HEAD~3 &&
+		test_linear_range 'd k l' j..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
 test_done
-- 
1.8.3.497.g83fddbe
Martin von Zweigbergk· Jun 3, 2013, 20:42 UTC · re: Martin von Zweigbergk · lore

[PATCH v5 4/7] add tests for rebasing root

---
 t/t3421-rebase-topology-linear.sh | 129 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)
Show changes to t/t3421-rebase-topology-linear.sh +129 −0
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index 81e3d59..659a7b3 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -218,4 +218,133 @@ test_run_rebase failure -m
 test_run_rebase failure -i
 test_run_rebase failure -p
 
+#       m
+#      /
+# a---b---c---g
+#
+# x---y---B
+#
+# uppercase = cherry-picked
+# m = reverted b
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for test involving root' '
+	git checkout b &&
+	revert m b &&
+	git checkout --orphan disjoint &&
+	git rm -rf . &&
+	test_commit x &&
+	test_commit y &&
+	cherry_pick B b
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root" "
+		reset_rebase &&
+		git rebase $* --onto c --root y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history" "
+		reset_rebase &&
+		git rebase $* c y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root drops patch in onto" "
+		reset_rebase &&
+		git rebase $* --onto m --root B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root with merge-base does not go to root" "
+		reset_rebase &&
+		git rebase $* --onto m --root g &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'c g' m..
+	"
+}
+
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history drops patch in onto" "
+		reset_rebase &&
+		git rebase $* m B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --root on linear history is a no-op" "
+		reset_rebase &&
+		git rebase $* --root c &&
+		test_cmp_rev c HEAD
+	"
+}
+test_run_rebase failure ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f --root on linear history causes re-write" "
+		reset_rebase &&
+		git rebase $* -f --root c &&
+		! test_cmp_rev a HEAD~2 &&
+		test_linear_range 'a b c' HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
 test_done
-- 
1.8.3.497.g83fddbe
Martin von Zweigbergk· Jun 3, 2013, 20:42 UTC · re: Martin von Zweigbergk · lore

[PATCH v5 5/7] add tests for rebasing merged history

---
 t/t3400-rebase.sh                 |  31 +----
 t/t3401-rebase-partial.sh         |  45 -------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3409-rebase-preserve-merges.sh |  53 --------
 t/t3425-rebase-topology-merges.sh | 258 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 260 insertions(+), 137 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh
Show changes to 5 files +260 −137

t/t3400-rebase.sh, t/t3401-rebase-partial.sh, t/t3404-rebase-interactive.sh, t/t3409-rebase-preserve-merges.sh, t/t3425-rebase-topology-merges.sh

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..b436ef4 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' '
 	echo Side >>C &&
 	git add C &&
 	git commit -m "Add C" &&
-	git checkout -b nonlinear my-topic-branch &&
-	echo Edit >>B &&
-	git add B &&
-	git commit -m "Modify B" &&
-	git merge side &&
-	git checkout -b upstream-merged-nonlinear &&
-	git merge master &&
 	git checkout -f my-topic-branch &&
 	git tag topic
 '
@@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' '
 	git rebase master
 '
 
-test_expect_success 'rebase after merge master' '
-	git checkout --detach refs/tags/topic &&
-	git branch -D topic &&
-	git reset --hard topic &&
-	git merge master &&
-	git rebase master &&
-	! (git show | grep "^Merge:")
-'
-
-test_expect_success 'rebase of history with merges is linearized' '
-	git checkout nonlinear &&
-	test 4 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
-test_expect_success 'rebase of history with merges after upstream merge is linearized' '
-	git checkout upstream-merged-nonlinear &&
-	test 5 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
 test_expect_success 'rebase a single mode change' '
 	git checkout master &&
+	git branch -D topic &&
 	echo 1 >X &&
 	git add X &&
 	test_tick &&
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
deleted file mode 100755
index 7ba1797..0000000
--- a/t/t3401-rebase-partial.sh
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland
-#
-
-test_description='git rebase should detect patches integrated upstream
-
-This test cherry-picks one local change of two into master branch, and
-checks that git rebase succeeds with only the second patch in the
-local branch.
-'
-. ./test-lib.sh
-
-test_expect_success 'prepare repository with topic branch' '
-	test_commit A &&
-	git checkout -b my-topic-branch &&
-	test_commit B &&
-	test_commit C &&
-	git checkout -f master &&
-	test_commit A2 A.t
-'
-
-test_expect_success 'pick top patch from topic branch into master' '
-	git cherry-pick C &&
-	git checkout -f my-topic-branch
-'
-
-test_debug '
-	git cherry master &&
-	git format-patch -k --stdout --full-index master >/dev/null &&
-	gitk --all & sleep 1
-'
-
-test_expect_success 'rebase topic branch against new master and check git am did not get halted' '
-	git rebase master &&
-	test_path_is_missing .git/rebase-apply
-'
-
-test_expect_success 'rebase --merge topic branch that was partially merged upstream' '
-	git reset --hard C &&
-	git rebase --merge master &&
-	test_path_is_missing .git/rebase-merge
-'
-
-test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index a58406d..ffcaf02 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' '
 	test $one = $(git rev-parse HEAD~2)
 '
 
-test_expect_success 'ignore patch if in upstream' '
-	HEAD=$(git rev-parse HEAD) &&
-	git checkout -b has-cherry-picked HEAD^ &&
+test_expect_success '--continue tries to commit, even for "edit"' '
 	echo unrelated > file7 &&
 	git add file7 &&
 	test_tick &&
 	git commit -m "unrelated change" &&
-	git cherry-pick $HEAD &&
-	EXPECT_COUNT=1 git rebase -i $HEAD &&
-	test $HEAD = $(git rev-parse HEAD^)
-'
-
-test_expect_success '--continue tries to commit, even for "edit"' '
 	parent=$(git rev-parse HEAD^) &&
 	test_tick &&
 	FAKE_LINES="edit 1" git rebase -i HEAD^ &&
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 6de4e22..2e0c364 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -11,14 +11,6 @@ Run "git rebase -p" and check that merges are properly carried along
 GIT_AUTHOR_EMAIL=bogus_email_address
 export GIT_AUTHOR_EMAIL
 
-# Clone 1 (trivial merge):
-#
-# A1--A2  <-- origin/master
-#  \   \
-#   B1--M  <-- topic
-#    \
-#     B2  <-- origin/topic
-#
 # Clone 2 (conflicting merge):
 #
 # A1--A2--B3   <-- origin/master
@@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL
 #     \--A3    <-- topic2
 #      \
 #       B2     <-- origin/topic
-#
-# Clone 4 (merge using second parent as base):
-#
-# A1--A2--B3   <-- origin/master
-#  \
-#   B1--A3--M  <-- topic
-#    \     /
-#     \--A4    <-- topic2
-#      \
-#       B2     <-- origin/topic
 
 test_expect_success 'setup for merge-preserving rebase' \
 	'echo First > A &&
@@ -58,20 +40,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git checkout -f master &&
 	echo Third >> A &&
 	git commit -a -m "Modify A2" &&
-
-	git clone ./. clone1 &&
-	(cd clone1 &&
-	git checkout -b topic origin/topic &&
-	git merge origin/master
-	) &&
-
-	git clone ./. clone4 &&
-	(
-		cd clone4 &&
-		git checkout -b topic origin/topic &&
-		git merge origin/master
-	) &&
-
 	echo Fifth > B &&
 	git add B &&
 	git commit -m "Add different B" &&
@@ -101,16 +69,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git commit -a -m "Modify B2"
 '
 
-test_expect_success 'rebase -p fakes interactive rebase' '
-	(
-	cd clone1 &&
-	git fetch &&
-	git rebase -p origin/topic &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_expect_success '--continue works after a conflict' '
 	(
 	cd clone2 &&
@@ -138,15 +96,4 @@ test_expect_success 'rebase -p preserves no-ff merges' '
 	)
 '
 
-test_expect_success 'rebase -p works when base inside second parent' '
-	(
-	cd clone4 &&
-	git fetch &&
-	git rebase -p HEAD^2 &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify B" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_done
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
new file mode 100755
index 0000000..efd7dfd
--- /dev/null
+++ b/t/t3425-rebase-topology-merges.sh
@@ -0,0 +1,258 @@
+#!/bin/sh
+
+test_description='rebase topology tests with merges'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+test_revision_subjects () {
+	expected="$1"
+	shift
+	set -- $(git log --format=%s --no-walk=unsorted "$@")
+	test "$expected" = "$*"
+}
+
+# a---b-----------c
+#      \           \
+#       d-------e   \
+#        \       \   \
+#         n---o---w---v
+#              \
+#               z
+test_expect_success 'setup of non-linear-history' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+
+	git checkout d &&
+	test_commit n &&
+	test_commit o &&
+	test_merge w e &&
+	test_merge v c &&
+	git checkout o &&
+	test_commit z
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* after merge from upstream" "
+		reset_rebase &&
+		git rebase $* e w &&
+		test_cmp_rev e HEAD~2 &&
+		test_linear_range 'n o' e..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized in place" "
+		reset_rebase &&
+		git rebase $* d w &&
+		test_cmp_rev d HEAD~3 &&
+		test_linear_range "\'"$expected"\'" d..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'e n o' ''
+test_run_rebase success 'e n o' -m
+test_run_rebase success 'n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized upstream" "
+		reset_rebase &&
+		git rebase $* c w &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history with merges after upstream merge is linearized" "
+		reset_rebase &&
+		git rebase $* c v &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_expect_success "rebase -p is no-op in non-linear history" "
+	reset_rebase &&
+	git rebase -p d w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p is no-op when base inside second parent" "
+	reset_rebase &&
+	git rebase -p e w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_failure "rebase -p --root on non-linear history is a no-op" "
+	reset_rebase &&
+	git rebase -p --root w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p re-creates merge from side branch" "
+	reset_rebase &&
+	git rebase -p z w &&
+	test_cmp_rev z HEAD^ &&
+	test_cmp_rev w^2 HEAD^2
+"
+
+test_expect_success "rebase -p re-creates internal merge" "
+	reset_rebase &&
+	git rebase -p c w &&
+	test_cmp_rev c HEAD~4 &&
+	test_cmp_rev HEAD^2^ HEAD~3 &&
+	test_revision_subjects 'd n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p can re-create two branches on onto" "
+	reset_rebase &&
+	git rebase -p --onto c d w &&
+	test_cmp_rev c HEAD~3 &&
+	test_cmp_rev c HEAD^2^ &&
+	test_revision_subjects 'n e o w' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#        \       \
+#         e-------u
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for patch-equivalence tests' '
+	git checkout e &&
+	test_merge u i
+'
+
+test_expect_success "rebase -p re-creates history around dropped commit matching upstream" "
+	reset_rebase &&
+	git rebase -p h u &&
+	test_cmp_rev h HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history drops patches in upstream" "
+	reset_rebase &&
+	git rebase -p --onto f h u &&
+	test_cmp_rev f HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history does not drop patches in onto" "
+	reset_rebase &&
+	git rebase -p --onto h f u &&
+	test_cmp_rev h HEAD~3 &&
+	test_cmp_rev HEAD^2~2 HEAD~2 &&
+	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+"
+
+# a---b---c---g---h
+#      \
+#       d---G---s
+#        \   \ /
+#         \   X
+#          \ / \
+#           e---t
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for dropping whole side' '
+	git checkout G &&
+	test_merge s e &&
+	git checkout e &&
+	test_merge t G
+'
+
+test_expect_failure "rebase -p drops merge commit when entire first-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h s &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+test_expect_success "rebase -p drops merge commit when entire second-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h t &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+# a---b---c
+#      \
+#       d---e
+#        \   \
+#         n---r
+#          \
+#           o
+#
+# r = tree-same with n
+test_expect_success 'setup of non-linear-history for empty commits' '
+	git checkout n &&
+	git merge --no-commit e &&
+	git reset n . &&
+	git commit -m r &&
+	git reset --hard &&
+	git clean -f &&
+	git tag r
+'
+
+test_expect_success "rebase -p re-creates empty internal merge commit" "
+	reset_rebase &&
+	git rebase -p c r &&
+	test_cmp_rev c HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd e n r' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p re-creates empty merge commit" "
+	reset_rebase &&
+	git rebase -p o r &&
+	test_cmp_rev e HEAD^2 &&
+	test_cmp_rev o HEAD^ &&
+	test_revision_subjects 'r' HEAD
+"
+
+test_done
-- 
1.8.3.497.g83fddbe
Junio C Hamano· Jun 4, 2013, 17:18 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v5 5/7] add tests for rebasing merged history

Martin von Zweigbergk <martinvonz@gmail.com> writes:
Show 5 quoted lines
> ---
> +#TODO: make all flavors of rebase use --topo-order
> +test_run_rebase success 'e n o' ''
> +test_run_rebase success 'e n o' -m
> +test_run_rebase success 'n o e' -i
I do not quite follow this TODO.

While I think it would be nice to update "rebase" so that all variants consider replaying the commits in the same order, in this history you have:

+# a---b-----------c +# \ \ +# d-------e \ +# \ \ \ +# n---o---w---v +# \ +# z

as long as o comes after n and e is shown before n or after o, which all three expected results satisify, it is in --topo-order, isn't it?

The same comment applies to the other TODO that talks about eno/noe differences.

Show 6 quoted lines
> +test_expect_success "rebase -p re-creates merge from side branch" "
> +     reset_rebase &&
> +     git rebase -p z w &&
> +     test_cmp_rev z HEAD^ &&
> +     test_cmp_rev w^2 HEAD^2
> +"
Hmm, turning the left one to the right one?

+# d-------e d-------e +# \ \ \ \ +# n---o---w ===> n---o \ +# \ \ \ +# z z---W

If w were a merge of o into e (i.e. w^1 were e not o), what should happen? Would we get the same topology?

In other words, when asked to replay w on top of z, how would we decide which parent to keep (in this case, e is kept)?

Show 7 quoted lines
> +test_expect_success "rebase -p can re-create two branches on onto" "
> +     reset_rebase &&
> +     git rebase -p --onto c d w &&
> +     test_cmp_rev c HEAD~3 &&
> +     test_cmp_rev c HEAD^2^ &&
> +     test_revision_subjects 'n e o w' HEAD~2 HEAD^2 HEAD^ HEAD
> +"
Nice (so are all the rest).
Martin von Zweigbergk· Jun 5, 2013, 05:44 UTC · re: Junio C Hamano · lore

Re: [PATCH v5 5/7] add tests for rebasing merged history

On Tue, Jun 4, 2013 at 10:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 24 quoted lines
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> ---
>> +#TODO: make all flavors of rebase use --topo-order
>> +test_run_rebase success 'e n o' ''
>> +test_run_rebase success 'e n o' -m
>> +test_run_rebase success 'n o e' -i
>
> I do not quite follow this TODO.
>
> While I think it would be nice to update "rebase" so that all
> variants consider replaying the commits in the same order, in this
> history you have:
>
> +# a---b-----------c
> +#      \           \
> +#       d-------e   \
> +#        \       \   \
> +#         n---o---w---v
> +#              \
> +#               z
>
> as long as o comes after n and e is shown before n or after o, which
> all three expected results satisify, it is in --topo-order, isn't it?

True, the TODO was too specific. I intended to get the list of commits to rebase for all kinds of rebase by using 'git rev-list --topo-order', but it doesn't really matter how the order is decided; my goal was just to make it consistent. I'll update the TODOs.

Show 17 quoted lines
>> +test_expect_success "rebase -p re-creates merge from side branch" "
>> +     reset_rebase &&
>> +     git rebase -p z w &&
>> +     test_cmp_rev z HEAD^ &&
>> +     test_cmp_rev w^2 HEAD^2
>> +"
>
> Hmm, turning the left one to the right one?
>
> +#       d-------e               d-------e
> +#        \       \               \       \
> +#         n---o---w     ===>      n---o   \
> +#              \                       \   \
> +#               z                       z---W
>
> If w were a merge of o into e (i.e. w^1 were e not o), what should
> happen?  Would we get the same topology?

Yes, it seems like it does yield the same topology. That seems to be what I tested at first. Search for "wrong" in [1]. I think Johannes's point was that it was not realistic, not that he's against it working in the same way independent of parent order. I don't feel strongly on whether to include a test for each direction. Unless others do, I guess I'll leave it as is. (But I did add a test case just now to see, so it's very little work for me if someone does want it included.)

> In other words, when asked to replay w on top of z, how would we
> decide which parent to keep (in this case, e is kept)?
Keep any parent that is not an ancestor of the new base? Or something like that.
  [1] http://thread.gmane.org/gmane.comp.version-control.git/205796/focus=205806
Johannes Sixt· Jun 5, 2013, 06:12 UTC · re: Junio C Hamano · lore

Re: [PATCH v5 5/7] add tests for rebasing merged history

Am 6/4/2013 19:18, schrieb Junio C Hamano:
Show 24 quoted lines
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
> 
>> ---
>> +#TODO: make all flavors of rebase use --topo-order
>> +test_run_rebase success 'e n o' ''
>> +test_run_rebase success 'e n o' -m
>> +test_run_rebase success 'n o e' -i
> 
> I do not quite follow this TODO.
> 
> While I think it would be nice to update "rebase" so that all
> variants consider replaying the commits in the same order, in this
> history you have:
> 
> +# a---b-----------c
> +#      \           \
> +#       d-------e   \
> +#        \       \   \
> +#         n---o---w---v
> +#              \
> +#               z
> 
> as long as o comes after n and e is shown before n or after o, which
> all three expected results satisify, it is in --topo-order, isn't it?

The comment is really just about the inconsistency, not about a request to have a guaranteed order among the parents of a merge commit.

Having said that, wouldn't it be useful (generally, not just in this context) to have a guarantee in which order --topo-order lists parents of a merge?

Show 17 quoted lines
>> +test_expect_success "rebase -p re-creates merge from side branch" "
>> +     reset_rebase &&
>> +     git rebase -p z w &&
>> +     test_cmp_rev z HEAD^ &&
>> +     test_cmp_rev w^2 HEAD^2
>> +"
> 
> Hmm, turning the left one to the right one?
> 
> +#       d-------e               d-------e    
> +#        \       \               \       \   
> +#         n---o---w     ===>      n---o   \
> +#              \                       \   \ 
> +#               z                       z---W
> 
> If w were a merge of o into e (i.e. w^1 were e not o), what should
> happen?  Would we get the same topology?

'git rebase -p z w' is a nonsense request in this situation. (I.e., there is no requirement on the result.) At best, we could detect it and bail out or warn.

-- Hannes
Martin von Zweigbergk· Jun 3, 2013, 20:42 UTC · re: Martin von Zweigbergk · lore

[PATCH v5 6/7] t3406: modernize style

Update the following:
 - Quote 'setup'
 - Remove blank lines within test case body
 - Use test_commit instead of custom quick_one
 - Create branch "topic" from tag created by test_commit
---
 t/t3406-rebase-message.sh | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)
Show changes to t/t3406-rebase-message.sh +9 −21
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index e6a9a0d..fe8c27f 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -4,27 +4,17 @@ test_description='messages from rebase operation'
 
 . ./test-lib.sh
 
-quick_one () {
-	echo "$1" >"file$1" &&
-	git add "file$1" &&
-	test_tick &&
-	git commit -m "$1"
-}
+test_expect_success 'setup' '
+	test_commit O fileO &&
+	test_commit X fileX &&
+	test_commit A fileA &&
+	test_commit B fileB &&
+	test_commit Y fileY &&
 
-test_expect_success setup '
-	quick_one O &&
-	git branch topic &&
-	quick_one X &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Y &&
-
-	git checkout topic &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Z &&
+	git checkout -b topic O &&
+	git cherry-pick A B &&
+	test_commit Z fileZ &&
 	git tag start
-
 '
 
 cat >expect <<\EOF
@@ -34,12 +24,10 @@ Committed: 0003 Z
 EOF
 
 test_expect_success 'rebase -m' '
-
 	git rebase -m master >report &&
 	sed -n -e "/^Already applied: /p" \
 		-e "/^Committed: /p" report >actual &&
 	test_cmp expect actual
-
 '
 
 test_expect_success 'rebase --stat' '
-- 
1.8.3.497.g83fddbe
Martin von Zweigbergk· Jun 3, 2013, 20:42 UTC · re: Martin von Zweigbergk · lore

[PATCH v5 7/7] tests: move test for rebase messages from t3400 to t3406

t3406 is supposed to test "messages from rebase operation", so let's
move tests in t3400 that fit that description into 3406. Most of the
functionality they tested, except for the messages, has now been
subsumed by t3420.
---
 t/t3400-rebase.sh         | 22 ----------------------
 t/t3406-rebase-message.sh | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 22 deletions(-)
Show changes to 2 files +22 −22

t/t3400-rebase.sh, t/t3406-rebase-message.sh

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b436ef4..45a55e9 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -59,28 +59,6 @@ test_expect_success 'rebase against master' '
 	git rebase master
 '
 
-test_expect_success 'rebase against master twice' '
-	git rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase against master twice with --force' '
-	git rebase --force-rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date, rebase forced" out
-'
-
-test_expect_success 'rebase against master twice from another branch' '
-	git checkout my-topic-branch^ &&
-	git rebase master my-topic-branch >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase fast-forward to master' '
-	git checkout my-topic-branch^ &&
-	git rebase my-topic-branch >out &&
-	test_i18ngrep "Fast-forwarded HEAD to my-topic-branch" out
-'
-
 test_expect_success 'the rebase operation should not have destroyed author information' '
 	! (git log | grep "Author:" | grep "<>")
 '
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index fe8c27f..0392e36 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -30,6 +30,28 @@ test_expect_success 'rebase -m' '
 	test_cmp expect actual
 '
 
+test_expect_success 'rebase against master twice' '
+	git rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase against master twice with --force' '
+	git rebase --force-rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date, rebase forced" out
+'
+
+test_expect_success 'rebase against master twice from another branch' '
+	git checkout topic^ &&
+	git rebase master topic >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase fast-forward to master' '
+	git checkout topic^ &&
+	git rebase topic >out &&
+	test_i18ngrep "Fast-forwarded HEAD to topic" out
+'
+
 test_expect_success 'rebase --stat' '
 	git reset --hard start &&
         git rebase --stat master >diffstat.txt &&
-- 
1.8.3.497.g83fddbe
Martin von Zweigbergk· Jun 7, 2013, 06:11 UTC · re: Martin von Zweigbergk · lore

[PATCH v6 0/8] Rebase topology test

Changes since v5:
 * Improved test_linear_range
 * Changed TODOs to be about consistency, not --topo-order
Martin von Zweigbergk (7):
  add simple tests of consistency across rebase types
  add tests for rebasing with patch-equivalence present
  add tests for rebasing of empty commits
  add tests for rebasing root
  add tests for rebasing merged history
  t3406: modernize style
  tests: move test for rebase messages from t3400 to t3406
 t/lib-rebase.sh                   |  33 ++++
 t/t3400-rebase.sh                 |  53 +-----
 t/t3401-rebase-partial.sh         |  69 --------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3406-rebase-message.sh         |  50 +++---
 t/t3409-rebase-preserve-merges.sh |  53 ------
 t/t3421-rebase-topology-linear.sh | 350 ++++++++++++++++++++++++++++++++++++++
 t/t3425-rebase-topology-merges.sh | 258 ++++++++++++++++++++++++++++
 8 files changed, 673 insertions(+), 203 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3421-rebase-topology-linear.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh
-- 
1.8.3.497.g83fddbe
Martin von Zweigbergk· Jun 7, 2013, 06:11 UTC · re: Martin von Zweigbergk · lore

[PATCH v6 1/7] add simple tests of consistency across rebase types

Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
 t/lib-rebase.sh                   | 16 ++++++++
 t/t3421-rebase-topology-linear.sh | 78 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+)
 create mode 100755 t/t3421-rebase-topology-linear.sh
Show changes to 2 files +94 −0

t/lib-rebase.sh, t/t3421-rebase-topology-linear.sh

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6ccf797..1e0ff28 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -65,3 +65,19 @@ EOF
 	test_set_editor "$(pwd)/fake-editor.sh"
 	chmod a+x fake-editor.sh
 }
+
+# checks that the revisions in "$2" represent a linear range with the
+# subjects in "$1"
+test_linear_range () {
+	revlist_merges=$(git rev-list --merges "$2") &&
+	test -z "$revlist_merges" &&
+	expected=$1
+	set -- $(git log --reverse --format=%s "$2")
+	test "$expected" = "$*"
+}
+
+reset_rebase () {
+	test_might_fail git rebase --abort &&
+	git reset --hard &&
+	git clean -f
+}
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
new file mode 100755
index 0000000..60365d1
--- /dev/null
+++ b/t/t3421-rebase-topology-linear.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='basic rebase topology tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+# a---b---c
+#      \
+#       d---e
+test_expect_success 'setup' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "simple rebase $*" "
+		reset_rebase &&
+		git rebase $* c e &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd e' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* is no-op if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* b e &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f rewrites even if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* -f b e &&
+		! test_cmp_rev e HEAD &&
+		test_cmp_rev b HEAD~2 &&
+		test_linear_range 'd e' b..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* fast-forwards from ancestor of upstream" "
+		reset_rebase &&
+		git rebase $* e b &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_done
-- 
1.8.3.497.g83fddbe
Martin von Zweigbergk· Jun 7, 2013, 06:11 UTC · re: Martin von Zweigbergk · lore

[PATCH v6 2/7] add tests for rebasing with patch-equivalence present

Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
 t/lib-rebase.sh                   | 17 ++++++++
 t/t3421-rebase-topology-linear.sh | 85 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)
Show changes to 2 files +102 −0

t/lib-rebase.sh, t/t3421-rebase-topology-linear.sh

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 1e0ff28..4b74ae4 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -81,3 +81,20 @@ reset_rebase () {
 	git reset --hard &&
 	git clean -f
 }
+
+cherry_pick () {
+	git cherry-pick -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+revert () {
+	git revert -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+make_empty () {
+	git commit --allow-empty -m "$1" &&
+	git tag "$1"
+}
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index 60365d1..ddcbfc6 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -75,4 +75,89 @@ test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#
+# uppercase = cherry-picked
+# h = reverted g
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for range selection tests' '
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops patches in upstream" "
+		reset_rebase &&
+		git rebase $* h i &&
+		test_cmp_rev h HEAD~2 &&
+		test_linear_range 'd i' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* can drop last patch if in upstream" "
+		reset_rebase &&
+		git rebase $* h G &&
+		test_cmp_rev h HEAD^ &&
+		test_linear_range 'd' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto drops patches in upstream" "
+		reset_rebase &&
+		git rebase $* --onto f h i &&
+		test_cmp_rev f HEAD~2 &&
+		test_linear_range 'd i' f..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto does not drop patches in onto" "
+		reset_rebase &&
+		git rebase $* --onto h f i &&
+		test_cmp_rev h HEAD~3 &&
+		test_linear_range 'd G i' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
 test_done
-- 
1.8.3.497.g83fddbe
Martin von Zweigbergk· Jun 7, 2013, 06:11 UTC · re: Martin von Zweigbergk · lore

[PATCH v6 3/7] add tests for rebasing of empty commits

Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
 t/t3401-rebase-partial.sh         | 24 ----------------
 t/t3421-rebase-topology-linear.sh | 58 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 24 deletions(-)
Show changes to 2 files +58 −24

t/t3401-rebase-partial.sh, t/t3421-rebase-topology-linear.sh

diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index 58f4823..7ba1797 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -42,28 +42,4 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr
 	test_path_is_missing .git/rebase-merge
 '
 
-test_expect_success 'rebase ignores empty commit' '
-	git reset --hard A &&
-	git commit --allow-empty -m empty &&
-	test_commit D &&
-	git rebase C &&
-	test "$(git log --format=%s C..)" = "D"
-'
-
-test_expect_success 'rebase --keep-empty' '
-	git reset --hard D &&
-	git rebase --keep-empty C &&
-	test "$(git log --format=%s C..)" = "D
-empty"
-'
-
-test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' '
-	git reset --hard A &&
-	git commit --allow-empty -m also-empty &&
-	git rebase --keep-empty D &&
-	test "$(git log --format=%s A..)" = "also-empty
-D
-empty"
-'
-
 test_done
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index ddcbfc6..f19f0d0 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -160,4 +160,62 @@ test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
+# a---b---c---j!
+#      \
+#       d---k!--l
+#
+# ! = empty
+test_expect_success 'setup of linear history for empty commit tests' '
+	git checkout c &&
+	make_empty j &&
+	git checkout d &&
+	make_empty k &&
+	test_commit l
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops empty commit" "
+		reset_rebase &&
+		git rebase $* c l &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty" "
+		reset_rebase &&
+		git rebase $* --keep-empty c l &&
+		test_cmp_rev c HEAD~3 &&
+		test_linear_range 'd k l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty keeps empty even if already in upstream" "
+		reset_rebase &&
+		git rebase $* --keep-empty j l &&
+		test_cmp_rev j HEAD~3 &&
+		test_linear_range 'd k l' j..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
 test_done
-- 
1.8.3.497.g83fddbe
Martin von Zweigbergk· Jun 7, 2013, 06:11 UTC · re: Martin von Zweigbergk · lore

[PATCH v6 4/7] add tests for rebasing root

Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
 t/t3421-rebase-topology-linear.sh | 129 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)
Show changes to t/t3421-rebase-topology-linear.sh +129 −0
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index f19f0d0..e67add6 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -218,4 +218,133 @@ test_run_rebase failure -m
 test_run_rebase failure -i
 test_run_rebase failure -p
 
+#       m
+#      /
+# a---b---c---g
+#
+# x---y---B
+#
+# uppercase = cherry-picked
+# m = reverted b
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for test involving root' '
+	git checkout b &&
+	revert m b &&
+	git checkout --orphan disjoint &&
+	git rm -rf . &&
+	test_commit x &&
+	test_commit y &&
+	cherry_pick B b
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root" "
+		reset_rebase &&
+		git rebase $* --onto c --root y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history" "
+		reset_rebase &&
+		git rebase $* c y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root drops patch in onto" "
+		reset_rebase &&
+		git rebase $* --onto m --root B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root with merge-base does not go to root" "
+		reset_rebase &&
+		git rebase $* --onto m --root g &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'c g' m..
+	"
+}
+
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history drops patch in onto" "
+		reset_rebase &&
+		git rebase $* m B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --root on linear history is a no-op" "
+		reset_rebase &&
+		git rebase $* --root c &&
+		test_cmp_rev c HEAD
+	"
+}
+test_run_rebase failure ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f --root on linear history causes re-write" "
+		reset_rebase &&
+		git rebase $* -f --root c &&
+		! test_cmp_rev a HEAD~2 &&
+		test_linear_range 'a b c' HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
 test_done
-- 
1.8.3.497.g83fddbe
Martin von Zweigbergk· Jun 7, 2013, 06:11 UTC · re: Martin von Zweigbergk · lore

[PATCH v6 5/7] add tests for rebasing merged history

Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
 t/t3400-rebase.sh                 |  31 +----
 t/t3401-rebase-partial.sh         |  45 -------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3409-rebase-preserve-merges.sh |  53 --------
 t/t3425-rebase-topology-merges.sh | 258 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 260 insertions(+), 137 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh
Show changes to 5 files +260 −137

t/t3400-rebase.sh, t/t3401-rebase-partial.sh, t/t3404-rebase-interactive.sh, t/t3409-rebase-preserve-merges.sh, t/t3425-rebase-topology-merges.sh

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..b436ef4 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' '
 	echo Side >>C &&
 	git add C &&
 	git commit -m "Add C" &&
-	git checkout -b nonlinear my-topic-branch &&
-	echo Edit >>B &&
-	git add B &&
-	git commit -m "Modify B" &&
-	git merge side &&
-	git checkout -b upstream-merged-nonlinear &&
-	git merge master &&
 	git checkout -f my-topic-branch &&
 	git tag topic
 '
@@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' '
 	git rebase master
 '
 
-test_expect_success 'rebase after merge master' '
-	git checkout --detach refs/tags/topic &&
-	git branch -D topic &&
-	git reset --hard topic &&
-	git merge master &&
-	git rebase master &&
-	! (git show | grep "^Merge:")
-'
-
-test_expect_success 'rebase of history with merges is linearized' '
-	git checkout nonlinear &&
-	test 4 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
-test_expect_success 'rebase of history with merges after upstream merge is linearized' '
-	git checkout upstream-merged-nonlinear &&
-	test 5 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
 test_expect_success 'rebase a single mode change' '
 	git checkout master &&
+	git branch -D topic &&
 	echo 1 >X &&
 	git add X &&
 	test_tick &&
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
deleted file mode 100755
index 7ba1797..0000000
--- a/t/t3401-rebase-partial.sh
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland
-#
-
-test_description='git rebase should detect patches integrated upstream
-
-This test cherry-picks one local change of two into master branch, and
-checks that git rebase succeeds with only the second patch in the
-local branch.
-'
-. ./test-lib.sh
-
-test_expect_success 'prepare repository with topic branch' '
-	test_commit A &&
-	git checkout -b my-topic-branch &&
-	test_commit B &&
-	test_commit C &&
-	git checkout -f master &&
-	test_commit A2 A.t
-'
-
-test_expect_success 'pick top patch from topic branch into master' '
-	git cherry-pick C &&
-	git checkout -f my-topic-branch
-'
-
-test_debug '
-	git cherry master &&
-	git format-patch -k --stdout --full-index master >/dev/null &&
-	gitk --all & sleep 1
-'
-
-test_expect_success 'rebase topic branch against new master and check git am did not get halted' '
-	git rebase master &&
-	test_path_is_missing .git/rebase-apply
-'
-
-test_expect_success 'rebase --merge topic branch that was partially merged upstream' '
-	git reset --hard C &&
-	git rebase --merge master &&
-	test_path_is_missing .git/rebase-merge
-'
-
-test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index a58406d..ffcaf02 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' '
 	test $one = $(git rev-parse HEAD~2)
 '
 
-test_expect_success 'ignore patch if in upstream' '
-	HEAD=$(git rev-parse HEAD) &&
-	git checkout -b has-cherry-picked HEAD^ &&
+test_expect_success '--continue tries to commit, even for "edit"' '
 	echo unrelated > file7 &&
 	git add file7 &&
 	test_tick &&
 	git commit -m "unrelated change" &&
-	git cherry-pick $HEAD &&
-	EXPECT_COUNT=1 git rebase -i $HEAD &&
-	test $HEAD = $(git rev-parse HEAD^)
-'
-
-test_expect_success '--continue tries to commit, even for "edit"' '
 	parent=$(git rev-parse HEAD^) &&
 	test_tick &&
 	FAKE_LINES="edit 1" git rebase -i HEAD^ &&
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 6de4e22..2e0c364 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -11,14 +11,6 @@ Run "git rebase -p" and check that merges are properly carried along
 GIT_AUTHOR_EMAIL=bogus_email_address
 export GIT_AUTHOR_EMAIL
 
-# Clone 1 (trivial merge):
-#
-# A1--A2  <-- origin/master
-#  \   \
-#   B1--M  <-- topic
-#    \
-#     B2  <-- origin/topic
-#
 # Clone 2 (conflicting merge):
 #
 # A1--A2--B3   <-- origin/master
@@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL
 #     \--A3    <-- topic2
 #      \
 #       B2     <-- origin/topic
-#
-# Clone 4 (merge using second parent as base):
-#
-# A1--A2--B3   <-- origin/master
-#  \
-#   B1--A3--M  <-- topic
-#    \     /
-#     \--A4    <-- topic2
-#      \
-#       B2     <-- origin/topic
 
 test_expect_success 'setup for merge-preserving rebase' \
 	'echo First > A &&
@@ -58,20 +40,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git checkout -f master &&
 	echo Third >> A &&
 	git commit -a -m "Modify A2" &&
-
-	git clone ./. clone1 &&
-	(cd clone1 &&
-	git checkout -b topic origin/topic &&
-	git merge origin/master
-	) &&
-
-	git clone ./. clone4 &&
-	(
-		cd clone4 &&
-		git checkout -b topic origin/topic &&
-		git merge origin/master
-	) &&
-
 	echo Fifth > B &&
 	git add B &&
 	git commit -m "Add different B" &&
@@ -101,16 +69,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git commit -a -m "Modify B2"
 '
 
-test_expect_success 'rebase -p fakes interactive rebase' '
-	(
-	cd clone1 &&
-	git fetch &&
-	git rebase -p origin/topic &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_expect_success '--continue works after a conflict' '
 	(
 	cd clone2 &&
@@ -138,15 +96,4 @@ test_expect_success 'rebase -p preserves no-ff merges' '
 	)
 '
 
-test_expect_success 'rebase -p works when base inside second parent' '
-	(
-	cd clone4 &&
-	git fetch &&
-	git rebase -p HEAD^2 &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify B" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_done
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
new file mode 100755
index 0000000..5400a05
--- /dev/null
+++ b/t/t3425-rebase-topology-merges.sh
@@ -0,0 +1,258 @@
+#!/bin/sh
+
+test_description='rebase topology tests with merges'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+test_revision_subjects () {
+	expected="$1"
+	shift
+	set -- $(git log --format=%s --no-walk=unsorted "$@")
+	test "$expected" = "$*"
+}
+
+# a---b-----------c
+#      \           \
+#       d-------e   \
+#        \       \   \
+#         n---o---w---v
+#              \
+#               z
+test_expect_success 'setup of non-linear-history' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+
+	git checkout d &&
+	test_commit n &&
+	test_commit o &&
+	test_merge w e &&
+	test_merge v c &&
+	git checkout o &&
+	test_commit z
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* after merge from upstream" "
+		reset_rebase &&
+		git rebase $* e w &&
+		test_cmp_rev e HEAD~2 &&
+		test_linear_range 'n o' e..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized in place" "
+		reset_rebase &&
+		git rebase $* d w &&
+		test_cmp_rev d HEAD~3 &&
+		test_linear_range "\'"$expected"\'" d..
+	"
+}
+#TODO: make order consistent across all flavors of rebase
+test_run_rebase success 'e n o' ''
+test_run_rebase success 'e n o' -m
+test_run_rebase success 'n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized upstream" "
+		reset_rebase &&
+		git rebase $* c w &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make order consistent across all flavors of rebase
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history with merges after upstream merge is linearized" "
+		reset_rebase &&
+		git rebase $* c v &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make order consistent across all flavors of rebase
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_expect_success "rebase -p is no-op in non-linear history" "
+	reset_rebase &&
+	git rebase -p d w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p is no-op when base inside second parent" "
+	reset_rebase &&
+	git rebase -p e w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_failure "rebase -p --root on non-linear history is a no-op" "
+	reset_rebase &&
+	git rebase -p --root w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p re-creates merge from side branch" "
+	reset_rebase &&
+	git rebase -p z w &&
+	test_cmp_rev z HEAD^ &&
+	test_cmp_rev w^2 HEAD^2
+"
+
+test_expect_success "rebase -p re-creates internal merge" "
+	reset_rebase &&
+	git rebase -p c w &&
+	test_cmp_rev c HEAD~4 &&
+	test_cmp_rev HEAD^2^ HEAD~3 &&
+	test_revision_subjects 'd n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p can re-create two branches on onto" "
+	reset_rebase &&
+	git rebase -p --onto c d w &&
+	test_cmp_rev c HEAD~3 &&
+	test_cmp_rev c HEAD^2^ &&
+	test_revision_subjects 'n e o w' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#        \       \
+#         e-------u
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for patch-equivalence tests' '
+	git checkout e &&
+	test_merge u i
+'
+
+test_expect_success "rebase -p re-creates history around dropped commit matching upstream" "
+	reset_rebase &&
+	git rebase -p h u &&
+	test_cmp_rev h HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history drops patches in upstream" "
+	reset_rebase &&
+	git rebase -p --onto f h u &&
+	test_cmp_rev f HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history does not drop patches in onto" "
+	reset_rebase &&
+	git rebase -p --onto h f u &&
+	test_cmp_rev h HEAD~3 &&
+	test_cmp_rev HEAD^2~2 HEAD~2 &&
+	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+"
+
+# a---b---c---g---h
+#      \
+#       d---G---s
+#        \   \ /
+#         \   X
+#          \ / \
+#           e---t
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for dropping whole side' '
+	git checkout G &&
+	test_merge s e &&
+	git checkout e &&
+	test_merge t G
+'
+
+test_expect_failure "rebase -p drops merge commit when entire first-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h s &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+test_expect_success "rebase -p drops merge commit when entire second-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h t &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+# a---b---c
+#      \
+#       d---e
+#        \   \
+#         n---r
+#          \
+#           o
+#
+# r = tree-same with n
+test_expect_success 'setup of non-linear-history for empty commits' '
+	git checkout n &&
+	git merge --no-commit e &&
+	git reset n . &&
+	git commit -m r &&
+	git reset --hard &&
+	git clean -f &&
+	git tag r
+'
+
+test_expect_success "rebase -p re-creates empty internal merge commit" "
+	reset_rebase &&
+	git rebase -p c r &&
+	test_cmp_rev c HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd e n r' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p re-creates empty merge commit" "
+	reset_rebase &&
+	git rebase -p o r &&
+	test_cmp_rev e HEAD^2 &&
+	test_cmp_rev o HEAD^ &&
+	test_revision_subjects 'r' HEAD
+"
+
+test_done
-- 
1.8.3.497.g83fddbe
Martin von Zweigbergk· Jun 7, 2013, 06:11 UTC · re: Martin von Zweigbergk · lore

[PATCH v6 6/7] t3406: modernize style

Update the following:
 - Quote 'setup'
 - Remove blank lines within test case body
 - Use test_commit instead of custom quick_one
 - Create branch "topic" from tag created by test_commit
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
 t/t3406-rebase-message.sh | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)
Show changes to t/t3406-rebase-message.sh +9 −21
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index e6a9a0d..fe8c27f 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -4,27 +4,17 @@ test_description='messages from rebase operation'
 
 . ./test-lib.sh
 
-quick_one () {
-	echo "$1" >"file$1" &&
-	git add "file$1" &&
-	test_tick &&
-	git commit -m "$1"
-}
+test_expect_success 'setup' '
+	test_commit O fileO &&
+	test_commit X fileX &&
+	test_commit A fileA &&
+	test_commit B fileB &&
+	test_commit Y fileY &&
 
-test_expect_success setup '
-	quick_one O &&
-	git branch topic &&
-	quick_one X &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Y &&
-
-	git checkout topic &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Z &&
+	git checkout -b topic O &&
+	git cherry-pick A B &&
+	test_commit Z fileZ &&
 	git tag start
-
 '
 
 cat >expect <<\EOF
@@ -34,12 +24,10 @@ Committed: 0003 Z
 EOF
 
 test_expect_success 'rebase -m' '
-
 	git rebase -m master >report &&
 	sed -n -e "/^Already applied: /p" \
 		-e "/^Committed: /p" report >actual &&
 	test_cmp expect actual
-
 '
 
 test_expect_success 'rebase --stat' '
-- 
1.8.3.497.g83fddbe
Martin von Zweigbergk· Jun 7, 2013, 06:11 UTC · re: Martin von Zweigbergk · lore

[PATCH v6 7/7] tests: move test for rebase messages from t3400 to t3406

t3406 is supposed to test "messages from rebase operation", so let's move tests in t3400 that fit that description into 3406. Most of the functionality they tested, except for the messages, has now been subsumed by t3420.

Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
 t/t3400-rebase.sh         | 22 ----------------------
 t/t3406-rebase-message.sh | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 22 deletions(-)
Show changes to 2 files +22 −22

t/t3400-rebase.sh, t/t3406-rebase-message.sh

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b436ef4..45a55e9 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -59,28 +59,6 @@ test_expect_success 'rebase against master' '
 	git rebase master
 '
 
-test_expect_success 'rebase against master twice' '
-	git rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase against master twice with --force' '
-	git rebase --force-rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date, rebase forced" out
-'
-
-test_expect_success 'rebase against master twice from another branch' '
-	git checkout my-topic-branch^ &&
-	git rebase master my-topic-branch >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase fast-forward to master' '
-	git checkout my-topic-branch^ &&
-	git rebase my-topic-branch >out &&
-	test_i18ngrep "Fast-forwarded HEAD to my-topic-branch" out
-'
-
 test_expect_success 'the rebase operation should not have destroyed author information' '
 	! (git log | grep "Author:" | grep "<>")
 '
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index fe8c27f..0392e36 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -30,6 +30,28 @@ test_expect_success 'rebase -m' '
 	test_cmp expect actual
 '
 
+test_expect_success 'rebase against master twice' '
+	git rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase against master twice with --force' '
+	git rebase --force-rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date, rebase forced" out
+'
+
+test_expect_success 'rebase against master twice from another branch' '
+	git checkout topic^ &&
+	git rebase master topic >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase fast-forward to master' '
+	git checkout topic^ &&
+	git rebase topic >out &&
+	test_i18ngrep "Fast-forwarded HEAD to topic" out
+'
+
 test_expect_success 'rebase --stat' '
 	git reset --hard start &&
         git rebase --stat master >diffstat.txt &&
-- 
1.8.3.497.g83fddbe
Johannes Sixt· Jun 7, 2013, 19:37 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH v6 0/8] Rebase topology test

Am 07.06.2013 08:11, schrieb Martin von Zweigbergk:
Show 13 quoted lines
> Changes since v5:
> 
>  * Improved test_linear_range
>  * Changed TODOs to be about consistency, not --topo-order
> 
> Martin von Zweigbergk (7):
>   add simple tests of consistency across rebase types
>   add tests for rebasing with patch-equivalence present
>   add tests for rebasing of empty commits
>   add tests for rebasing root
>   add tests for rebasing merged history
>   t3406: modernize style
>   tests: move test for rebase messages from t3400 to t3406
I looked at the interdiff to v3 and have nothing to add.
Well done! Thanks.
-- Hannes
Johannes Sixt· Jun 18, 2013, 07:28 UTC · re: Martin von Zweigbergk · lore

[PATCH mz/rebase-tests] rebase topology tests: fix commit names on case-insensitive file systems

From: Johannes Sixt <j6t@kdbg.org>

The recently introduced tests used uppercase letters to denote cherry-picks of commits having the corresponding lowercase letter names. The helper functions also set up tags with the names of the commits.

But this constellation fails on case-insensitive file systems because there cannot be distinct tags with names that differ only in case.

Use a less subtle convention for the names of cherry-picked commits.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 Knowing that the tests would take their time to complete on Windows,
 today was the first time I ran them, and there were some unexpected
 issues. Fixed by this patch.
 t/t3421-rebase-topology-linear.sh | 20 ++++++++++----------
 t/t3425-rebase-topology-merges.sh | 16 ++++++++--------
 2 files changed, 18 insertions(+), 18 deletions(-)
Show changes to 2 files +18 −18

t/t3421-rebase-topology-linear.sh, t/t3425-rebase-topology-merges.sh

diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index e67add6..9c55cba 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -79,9 +79,9 @@ test_run_rebase success -p
 #      /
 # a---b---c---g---h
 #      \
-#       d---G---i
+#       d---gp--i
 #
-# uppercase = cherry-picked
+# gp = cherry-picked g
 # h = reverted g
 #
 # Reverted patches are there for tests to be able to check if a commit
@@ -94,7 +94,7 @@ test_expect_success 'setup of linear history for range selection tests' '
 	test_commit g &&
 	revert h g &&
 	git checkout d &&
-	cherry_pick G g &&
+	cherry_pick gp g &&
 	test_commit i &&
 	git checkout b &&
 	test_commit f
@@ -120,7 +120,7 @@ test_run_rebase () {
 	shift
 	test_expect_$result "rebase $* can drop last patch if in upstream" "
 		reset_rebase &&
-		git rebase $* h G &&
+		git rebase $* h gp &&
 		test_cmp_rev h HEAD^ &&
 		test_linear_range 'd' h..
 	"
@@ -152,7 +152,7 @@ test_run_rebase () {
 		reset_rebase &&
 		git rebase $* --onto h f i &&
 		test_cmp_rev h HEAD~3 &&
-		test_linear_range 'd G i' h..
+		test_linear_range 'd gp i' h..
 	"
 }
 test_run_rebase success ''
@@ -222,9 +222,9 @@ test_run_rebase failure -p
 #      /
 # a---b---c---g
 #
-# x---y---B
+# x---y---bp
 #
-# uppercase = cherry-picked
+# bp = cherry-picked b
 # m = reverted b
 #
 # Reverted patches are there for tests to be able to check if a commit
@@ -239,7 +239,7 @@ test_expect_success 'setup of linear history for test involving root' '
 	git rm -rf . &&
 	test_commit x &&
 	test_commit y &&
-	cherry_pick B b
+	cherry_pick bp b
 '
 
 test_run_rebase () {
@@ -277,7 +277,7 @@ test_run_rebase () {
 	shift
 	test_expect_$result "rebase $* --onto --root drops patch in onto" "
 		reset_rebase &&
-		git rebase $* --onto m --root B &&
+		git rebase $* --onto m --root bp &&
 		test_cmp_rev m HEAD~2 &&
 		test_linear_range 'x y' m..
 	"
@@ -308,7 +308,7 @@ test_run_rebase () {
 	shift
 	test_expect_$result "rebase $* without --onto --root with disjoint history drops patch in onto" "
 		reset_rebase &&
-		git rebase $* m B &&
+		git rebase $* m bp &&
 		test_cmp_rev m HEAD~2 &&
 		test_linear_range 'x y' m..
 	"
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
index 5400a05..1d195fb 100755
--- a/t/t3425-rebase-topology-merges.sh
+++ b/t/t3425-rebase-topology-merges.sh
@@ -30,7 +30,7 @@ test_expect_success 'setup of non-linear-history' '
 	test_commit g &&
 	revert h g &&
 	git checkout d &&
-	cherry_pick G g &&
+	cherry_pick gp g &&
 	test_commit i &&
 	git checkout b &&
 	test_commit f
@@ -154,11 +154,11 @@ test_expect_success "rebase -p can re-create two branches on onto" "
 #      /
 # a---b---c---g---h
 #      \
-#       d---G---i
+#       d---gp--i
 #        \       \
 #         e-------u
 #
-# uppercase = cherry-picked
+# gp = cherry-picked g
 # h = reverted g
 test_expect_success 'setup of non-linear-history for patch-equivalence tests' '
 	git checkout e &&
@@ -186,24 +186,24 @@ test_expect_success "rebase -p --onto in merged history does not drop patches in
 	git rebase -p --onto h f u &&
 	test_cmp_rev h HEAD~3 &&
 	test_cmp_rev HEAD^2~2 HEAD~2 &&
-	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+	test_revision_subjects 'd gp i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
 "
 
 # a---b---c---g---h
 #      \
-#       d---G---s
+#       d---gp--s
 #        \   \ /
 #         \   X
 #          \ / \
 #           e---t
 #
-# uppercase = cherry-picked
+# gp = cherry-picked g
 # h = reverted g
 test_expect_success 'setup of non-linear-history for dropping whole side' '
-	git checkout G &&
+	git checkout gp &&
 	test_merge s e &&
 	git checkout e &&
-	test_merge t G
+	test_merge t gp
 '
 
 test_expect_failure "rebase -p drops merge commit when entire first-parent side is dropped" "
-- 
1.8.3.1.1749.g223650b
Junio C Hamano· Jun 18, 2013, 15:45 UTC · re: Johannes Sixt · lore

Re: [PATCH mz/rebase-tests] rebase topology tests: fix commit names on case-insensitive file systems

Johannes Sixt <j.sixt@viscovery.net> writes:
Show 16 quoted lines
> From: Johannes Sixt <j6t@kdbg.org>
>
> The recently introduced tests used uppercase letters to denote
> cherry-picks of commits having the corresponding lowercase letter names.
> The helper functions also set up tags with the names of the commits.
>
> But this constellation fails on case-insensitive file systems because
> there cannot be distinct tags with names that differ only in case.
>
> Use a less subtle convention for the names of cherry-picked commits.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  Knowing that the tests would take their time to complete on Windows,
>  today was the first time I ran them, and there were some unexpected
>  issues. Fixed by this patch.
Thanks, will queue on top of that series.
Martin von Zweigbergk· Jun 18, 2013, 15:53 UTC · re: Johannes Sixt · lore

Re: [PATCH mz/rebase-tests] rebase topology tests: fix commit names on case-insensitive file systems

On Tue, Jun 18, 2013 at 12:28 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
Show 9 quoted lines
>
> The recently introduced tests used uppercase letters to denote
> cherry-picks of commits having the corresponding lowercase letter names.
> The helper functions also set up tags with the names of the commits.
>
> But this constellation fails on case-insensitive file systems because
> there cannot be distinct tags with names that differ only in case.
>
> Use a less subtle convention for the names of cherry-picked commits.
Makes sense, and the patch looks good. Thanks.
>  Knowing that the tests would take their time to complete on Windows,
I'm curious just how slow it is. Are we talking minutes?
Johannes Sixt· Jun 19, 2013, 05:52 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH mz/rebase-tests] rebase topology tests: fix commit names on case-insensitive file systems

Am 6/18/2013 17:53, schrieb Martin von Zweigbergk:
> On Tue, Jun 18, 2013 at 12:28 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>  Knowing that the tests would take their time to complete on Windows,
> 
> I'm curious just how slow it is. Are we talking minutes?

D:\Src\mingw-git\t>bash -c "time make t342[15]*" ... real 5m7.160s user 0m21.831s sys 0m6.376s

The real time is correct, but I don't think that we can trust user or sys times as the box was otherwise idle.

-- Hannes

← back to recent threads