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

[Johannes Sixt <j6t@kdbg.org>]

Am 31.05.2013 08:49, schrieb Martin von Zweigbergk:
> +#       f
> +#      /
> +# a---b---c---g---h
> +#      \
> +#       d---G---i
> +#        \       \
> +#         e-------u
> +#
> +# uppercase = cherry-picked
> +# h = reverted g
...
> +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