[PATCH] git p4 test: fix failure in 9814-git-p4-rename.sh Was: Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

[Christoph Bonitz <ml.christophbonitz@gmail.com>]

The scenario in the rename test makes unnecessary assumptions about
which file git file-tree will detect as a source for a copy-operations.
Furthermore, copy detection is not tested by checking the resulting
perforce revision history via p4 filelog, but via git diff-tree.

This patch makes the test more robust by accepting each of the possible
sources, and more rigorous by doing so via p4 filelog.
---
 t/t9814-git-p4-rename.sh | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index 1fc1f5f..4068510 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -156,18 +156,16 @@ test_expect_success 'detect copies' '
  git diff-tree -r -C HEAD &&
  git p4 submit &&
  p4 filelog //depot/file10 &&
- p4 filelog //depot/file10 | grep -q "branch from //depot/file" &&
+ p4 filelog //depot/file10 | grep -q "branch from //depot/file2" &&

  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 &&
  git config git-p4.detectCopiesHarder true &&
  git p4 submit &&
  p4 filelog //depot/file11 &&
- p4 filelog //depot/file11 | grep -q "branch from //depot/file" &&
+ p4 filelog //depot/file11 | grep -q -E "branch from //depot/file(2|10)" &&

  cp file2 file12 &&
  echo "some text" >>file12 &&
@@ -177,7 +175,7 @@ test_expect_success 'detect copies' '
  level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut
-f1 | cut -d" " -f5 | sed "s/C0*//") &&
  test -n "$level" && test "$level" -gt 0 && test "$level" -lt 98 &&
  src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
- test "$src" = file10 || test "$src" = file11 &&
+ test "$src" = file2 || test "$src" = file10 || test "$src" = file11 &&
  git config git-p4.detectCopies $(($level + 2)) &&
  git p4 submit &&
  p4 filelog //depot/file12 &&
@@ -190,12 +188,10 @@ test_expect_success 'detect copies' '
  git diff-tree -r -C --find-copies-harder HEAD &&
  level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut
-f1 | cut -d" " -f5 | sed "s/C0*//") &&
  test -n "$level" && test "$level" -gt 2 && test "$level" -lt 100 &&
- src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
- test "$src" = file10 || test "$src" = file11 || test "$src" = file12 &&
  git config git-p4.detectCopies $(($level - 2)) &&
  git p4 submit &&
  p4 filelog //depot/file13 &&
- p4 filelog //depot/file13 | grep -q "branch from //depot/file"
+ p4 filelog //depot/file13 | grep -q -E "branch from //depot/file(2|10|11|12)"
  )
 '

-- 
2.0.1

On Mon, Jul 7, 2014 at 3:10 AM, Pete Wyckoff <pw@padd.com> wrote:
> 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