Am 23.07.2014 23:28, schrieb Christoph Bonitz:
> 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.
> ---
Please sign off your patch.
> 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 &&
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. You need braces like this:
{ test "$src" = file2 || test "$src" = file10 || test "$src" = file11; } &&
or you wrap it up in a case statement.
> 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)"
> )
> '
>
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.
-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html