Re: [PATCH 06/48] t6039: Add failing testcases for rename/rename/add-{source,dest} conflicts

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 18, 2011, 23:38 UTC
Message-ID: <7voc0rdusj.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1307518278-23814-7-git-send-email-newren@gmail.com>

Elijah Newren <newren@gmail.com> writes:

Show 9 quoted lines

> Add testcases that cover three failures with current git merge, all
> involving renaming one file on both sides of history:
>
> Case 1:
> ...
> Case 3:
> ...
>
> Signed-off-by: Elijah Newren <newren@gmail.com>

Very nicely described.

Show 8 quoted lines

> +test_expect_failure 'detect conflict with rename/rename(1to2)/add-source merge' '
> +	git checkout B^0 &&
> +
> +	test_must_fail git merge -s recursive C^0 &&
> +
> +	test -f a &&
> +	test -f b &&
> +	test -f c

Please check the contents of the index here as well.  I think:

 - "a" should be at stage#0, with "something completely different" from C;
 - "b" should have stage #1 and #2, both with the original "a" from A; and
 - "c" should have stage #1 and #3, both with the original "a" from A

right?  The last test in this patch is nicely done in this regard.

Show 29 quoted lines

> +test_expect_success 'setup rename/rename(1to2)/add-source resolvable conflict' '
> +	git rm -rf . &&
> +	git clean -fdqx &&
> +	rm -rf .git &&
> +	git init &&
> +
> +	>a &&
> +	git add a &&
> +	test_tick &&
> +	git commit -m base &&
> +	git tag A &&
> +
> +	git checkout -b B A &&
> +	git mv a b &&
> +	test_tick &&
> +	git commit -m one &&
> +
> +	git checkout -b C A &&
> +	git mv a b &&
> +	echo important-info >a &&
> +	test_tick &&
> +	git commit -m two
> +'
> +
> +test_expect_failure 'rename/rename/add-source still tracks new a file' '
> +	git checkout C^0 &&
> +	git merge -s recursive B^0 &&
> +
> +	test 2 -eq $(git ls-files -s | wc -l) &&

Instead of just checking the numbers, could you make the expected result a bit more explicit? What do we expect here? Perhaps

 - "a" in the index is at stage #0, with "important-info";
 - "a" in the working tree is clean;
 - "b" in the index is at stage #0, with empty content; and
 - "b" in the working tree is clean.

Again, the last test in this patch is nicely done in this regard.

Show 38 quoted lines

> +test_expect_success 'setup rename/rename(1to2)/add-dest conflict' '
> +	git rm -rf . &&
> +	git clean -fdqx &&
> +	rm -rf .git &&
> +	git init &&
> +
> +	echo stuff >a &&
> +	git add a &&
> +	test_tick &&
> +	git commit -m base &&
> +	git tag A &&
> +
> +	git checkout -b B A &&
> +	git mv a b &&
> +	echo precious-data >c &&
> +	git add c &&
> +	test_tick &&
> +	git commit -m one &&
> +
> +	git checkout -b C A &&
> +	git mv a c &&
> +	echo important-info >b &&
> +	git add b &&
> +	test_tick &&
> +	git commit -m two
> +'
> +
> +test_expect_failure 'rename/rename/add-dest merge still knows about conflicting file versions' '
> +	git checkout C^0 &&
> +	test_must_fail git merge -s recursive B^0 &&
> +
> +	test 5 -eq $(git ls-files -s | wc -l) &&
> +	test 2 -eq $(git ls-files -u b | wc -l) &&
> +	test 2 -eq $(git ls-files -u c | wc -l) &&
> +	test $(git rev-parse :2:b) = $(git rev-parse C:b) &&
> +	test $(git rev-parse :3:b) = $(git rev-parse B:b) &&
> +	test $(git rev-parse :2:c) = $(git rev-parse C:c) &&
> +	test $(git rev-parse :3:c) = $(git rev-parse B:c)

5 - 2 - 2 = 1 so we know that it is merged cleanly in the index, but we would want to also test "a" in the working tree is merged and clean.

Thanks.