Test failure in t9814-git-p4-rename.sh - my environment or bad test?

Test failure in t9814-git-p4-rename.sh - my environment or bad test?

[Christoph Bonitz]

Hi,

I'm trying to get the git p4 tests to pass on my machine (OS X
Mavericks) from master before making some changes. I'm experiencing a
test failure in "detect copies" of the rename test.

The test creates file2 with some content, creates a few copies (each
with a commit), then does the following (no git write operations
omitted):
echo "file2" >>file2 &&
cp file2 file10 &&
git add file2 file10 &&
git commit -a -m "Modify and copy file2 to file10" &&
... (some non-write-operations) ...
cp file2 file11 &&
git add file11 &&
git commit -a -m "Copy file2 to file11" &&
git diff-tree -r -C --find-copies-harder HEAD &&
src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
test "$src" = file10 &&

This is where it fails on my machine. The git diff-tree output is this
:100644 100644 22a35c17c4c0779f75142036beef6ccd58525b9c
22a35c17c4c0779f75142036beef6ccd58525b9c C100 file2 file11
so git diff-tree sees file2 as the copy source, not file10. In my
opinion, the diff-tree result is legitimate (at that point, file2,
file10 and file11 are identical). Later in the tests, after making
more copies of file2, the conditions are more flexible, e.g.
test "$src" = file10 || test "$src" = file11 || test "$src" = file12 &&

IMO, the test discounts the legitimate possibility of diff-tree
detecting file2 as source, making unnecessary assumptions about
implementation details. Is this correct, or do I misunderstand the
workings of diff-tree?

I'd be grateful for advice, both on whether this is a bug, and if so,
which branch to base a patch on.

Best regards
Christoph Bonitz

Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

[Pete Wyckoff]

ml.christophbonitz@gmail.com wrote on Sun, 06 Jul 2014 16:32 +0200:
> I'm trying to get the git p4 tests to pass on my machine (OS X
> Mavericks) from master before making some changes. I'm experiencing a
> test failure in "detect copies" of the rename test.
> 
> The test creates file2 with some content, creates a few copies (each
> with a commit), then does the following (no git write operations
> omitted):
> echo "file2" >>file2 &&
> cp file2 file10 &&
> git add file2 file10 &&
> git commit -a -m "Modify and copy file2 to file10" &&
> ... (some non-write-operations) ...
> cp file2 file11 &&
> git add file11 &&
> git commit -a -m "Copy file2 to file11" &&
> git diff-tree -r -C --find-copies-harder HEAD &&
> src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
> test "$src" = file10 &&
> 
> This is where it fails on my machine. The git diff-tree output is this
> :100644 100644 22a35c17c4c0779f75142036beef6ccd58525b9c
> 22a35c17c4c0779f75142036beef6ccd58525b9c C100 file2 file11
> so git diff-tree sees file2 as the copy source, not file10. In my
> opinion, the diff-tree result is legitimate (at that point, file2,
> file10 and file11 are identical). Later in the tests, after making
> more copies of file2, the conditions are more flexible, e.g.
> test "$src" = file10 || test "$src" = file11 || test "$src" = file12 &&
> 
> IMO, the test discounts the legitimate possibility of diff-tree
> detecting file2 as source, making unnecessary assumptions about
> implementation details. Is this correct, or do I misunderstand the
> workings of diff-tree?
> 
> I'd be grateful for advice, both on whether this is a bug, and if so,
> which branch to base a patch on.

I think your analysis is correct.  And I agree that later tests
have noticed this ambiguity and added multiple comparisons like
you quote.

I'm not sure how to robustify this.  At least doing the multiple
comparisons should make the tests work again.  The goal of this
series of tests is to make sure that copy detection is working,
not to verify that the correct copy choice was made.  That should
be in other (non-p4) tests.

Do send patches based on Junio's master.  I can ack, and they'll
show up in a future git release.

Thanks!

		-- Pete

Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

[Junio C Hamano]

Pete Wyckoff <pw@padd.com> writes:

> I'm not sure how to robustify this.  At least doing the multiple
> comparisons should make the tests work again.  The goal of this
> series of tests is to make sure that copy detection is working,
> not to verify that the correct copy choice was made.  That should
> be in other (non-p4) tests.

"Choosing any of these as the copy source is fine" is a sensible way
to fix the problem with this test.  Would it be a better solution to
avoid having multiple/ambiguous copy source candidates in the first
place, by the way?

> Do send patches based on Junio's master.  I can ack, and they'll
> show up in a future git release.
>
> Thanks!
>
> 		-- Pete

Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

[Christoph Bonitz]

On Mon, Jul 7, 2014 at 11:14 PM, Junio C Hamano <gitster@pobox.com> wrote:

> "Choosing any of these as the copy source is fine" is a sensible way
> to fix the problem with this test.  Would it be a better solution to
> avoid having multiple/ambiguous copy source candidates in the first
> place, by the way?

I agree that this would be the best solution, although I feel more
confident making a less invasive change first.

Thanks,
Christoph