Re: [PATCH 1/3] t3034: add rename threshold tests
flat view
From: Eric Sunshine <hidden>
Date: 2016-06-15 23:08:24
On Tue, Feb 23, 2016 at 6:48 PM, Felipe Gonçalves Assis [off-list ref] wrote:
10ae752 (merge-recursive: option to specify rename threshold, 2010-09-27) introduced this feature but did not include any tests. The tests use the new option --find-renames, which replaces the then introduced and now deprecated option --rename-threshold. Also update name and description of t3032 for consistency: "merge-recursive options" -> "merge-recursive space options"
A few superficial comments below...
quoted hunk ↗ jump to hunk
Signed-off-by: Felipe Gonçalves Assis <redacted> ---diff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh@@ -0,0 +1,235 @@ +test_expect_success 'setup 1/2: basic setup' '
If someone ever adds a third setup test, it's likely that that person will forget to update this title to say "1/3" (ditto regarding "setup 2/2" below). Perhaps just call this "setup repo" and 2/2 "setup thresholds" or something.
quoted hunk ↗ jump to hunk
+ cat <<-\EOF >3-old && + 33a + 33b + 33c + 33d + EOF + sed s/33/22/ <3-old >2-old && + sed s/33/11/ <3-old >1-old && + sed s/33/00/ <3-old >0-old && + git add [0-3]-old && + git commit -m base && + git rm [0-3]-old && + git commit -m delete && + git checkout -b rename HEAD^ && + cp 3-old 3-new && + sed 1,1s/./x/ <2-old >2-new && + sed 1,2s/./x/ <1-old >1-new && + sed 1,3s/./x/ <0-old >0-new && + git add [0-3]-new && + git rm [0-3]-old && + git commit -m rename && + get_expected_stages 0 && + get_expected_stages 1 && + get_expected_stages 2 && + get_expected_stages 3 && + check_50="false" &&
Why isn't this assignment done in setup 2/2 where all the other assignments to 'check_50' are done?
quoted hunk ↗ jump to hunk
+ tail="HEAD^ -- HEAD master" +' + +test_expect_success 'setup 2/2: threshold array' ' + git diff --name-status -M01 HEAD^ HEAD >diff-output && + test_debug "cat diff-output" && + test_line_count = 4 diff-output && + grep "R[0-9]\{3\} \([0-3]\)-old \1-new" diff-output \ + >grep-output &&
I'd be slightly concerned about the use of \{3\} with older grep's
such as on Solaris, and would just code it as "[0-9][0-9][0-9]" and be
done with it, but perhaps it's not worth worrying about until someone
complains.
+ test_cmp diff-output grep-output &&
So, this is asserting that you only see "renames" and nothing else. Okay.
quoted hunk ↗ jump to hunk
+ th0=$(sed -n "s/R\(...\) 0-old 0-new/\1/p" <diff-output) && + th1=$(sed -n "s/R\(...\) 1-old 1-new/\1/p" <diff-output) && + th2=$(sed -n "s/R\(...\) 2-old 2-new/\1/p" <diff-output) && + th3=$(sed -n "s/R\(...\) 3-old 3-new/\1/p" <diff-output) && + test "$th0" -lt "$th1" && + test "$th1" -lt "$th2" && + test "$th2" -lt "$th3" && + test "$th3" = 100 &&
It's very slightly odd to see '=' rather than '-eq' among all these
other algebraic operators ('-lt', '-le'), but not so odd that I'd
mention it (unless I just did), and not necessarily worth changing.
quoted hunk ↗ jump to hunk
+ if [ 50 -le "$th0" ]; then + check_50=check_threshold_0 + elif [ 50 -le "$th1" ]; then + check_50=check_threshold_1 + elif [ 50 -le "$th2" ]; then + check_50=check_threshold_2 + fi && + th0="$th0%" && + th1="$th1%" && + th2="$th2%" && + th3="$th3%" +' + +test_expect_success 'assumption for tests: rename detection with diff' ' + git diff --name-status -M$th0 --diff-filter=R HEAD^ HEAD \ + >diff-output-0 && + git diff --name-status -M$th1 --diff-filter=R HEAD^ HEAD \ + >diff-output-1 && + git diff --name-status -M$th2 --diff-filter=R HEAD^ HEAD \ + >diff-output-2 && + git diff --name-status -M100% --diff-filter=R HEAD^ HEAD \ + >diff-output-3 && + test_line_count = 4 diff-output-0 && + test_line_count = 3 diff-output-1 && + test_line_count = 2 diff-output-2 && + test_line_count = 1 diff-output-3 +' + +test_expect_success 'default similarity threshold is 50%' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive $tail && + $check_50