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 <p...@padd.com> wrote: > ml.christophbon...@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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
