Re: [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?
On Thu, Jul 24, 2014 at 8:45 PM, Johannes Sixt j...@kdbg.org wrote: Am 23.07.2014 23:28, schrieb Christoph Bonitz: - test $src = file10 || test $src = file11 + test $src = file2 || test $src = file10 || test $src = file11 You can't test for alternatives in this way. It's already wrong in the original line, which is from 795fcb0e (avoid test cond -a/-o cond), and breaks the chain. Thank you, I'm sorry I missed that! -- 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
Re: [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?
On Fri, Jul 25, 2014 at 12:05 AM, Junio C Hamano gits...@pobox.com wrote: Johannes Sixt j...@kdbg.org writes: I see a few other no-nos in the context of the changes, in particular, pipelines where git is not the last command; these would not catch failures in the git commands. But a fix for that is certainly outside the scope of this patch. Yuck. Thanks for spotting. Perhaps we should apply a preliminary clean-up before doing anything else, perhaps? The change in 9814 is a post 2.0 regression. Apart from your change and the word wrap adjustments suggested by Pete, would the following also make sense, to fix the other flaw Johannes pointed out? With regards to failing, git diff-tree should be idempotent. I think those are the two occurrences in this file: diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh index 1fc1f5f..7815f9a 100755 --- a/t/t9814-git-p4-rename.sh +++ b/t/t9814-git-p4-rename.sh @@ -176,6 +176,7 @@ 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/C test -n $level test $level -gt 0 test $level -lt 98 + 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 || test $src = file11 git config git-p4.detectCopies $(($level + 2)) @@ -190,6 +191,7 @@ 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/C test -n $level test $level -gt 2 test $level -lt 100 + 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 || test $src = file11 || test $src = file12 git config git-p4.detectCopies $(($level - 2)) -- 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
Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?
On Mon, Jul 7, 2014 at 11:14 PM, Junio C Hamano gits...@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 -- 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
[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?
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
Test failure in t9814-git-p4-rename.sh - my environment or bad test?
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 -- 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
