[PATCH v2 0/3] Tests for merge-recursive rename options
no replies
Revision v2 of 2 in this series.
7 messages,
2 authors,
2016-06-15 · open the first message on its own page
Get rid of blatant bash-isms in favour of simple and portable constructions.
Felipe Gonçalves Assis (3):
t3034: add rename threshold tests
t3034: test option to disable renames
t3034: test deprecated interface
...s.sh => t3032-merge-recursive-space-options.sh} | 2 +-
t/t3034-merge-recursive-rename-options.sh | 309 +++++++++++++++++++++
2 files changed, 310 insertions(+), 1 deletion(-)
rename t/{t3032-merge-recursive-options.sh => t3032-merge-recursive-space-options.sh} (99%)
create mode 100755 t/t3034-merge-recursive-rename-options.sh
--
2.7.1.492.gd821b20
From: Felipe Gonçalves Assis <redacted>
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"
Signed-off-by: Felipe Gonçalves Assis <redacted>
---
...s.sh => t3032-merge-recursive-space-options.sh} | 2 +-
t/t3034-merge-recursive-rename-options.sh | 235 +++++++++++++++++++++
2 files changed, 236 insertions(+), 1 deletion(-)
rename t/{t3032-merge-recursive-options.sh => t3032-merge-recursive-space-options.sh} (99%)
create mode 100755 t/t3034-merge-recursive-rename-options.sh
diff --git a/t/t3032-merge-recursive-options.sh b/t/t3032-merge-recursive-space-options.sh
similarity index 99%
rename from t/t3032-merge-recursive-options.sh
rename to t/t3032-merge-recursive-space-options.sh
index 4029c9c..b56180e 100755
--- a/t/t3032-merge-recursive-options.sh
+++ b/t/t3032-merge-recursive-space-options.sh @@ -1,6 +1,6 @@
#!/bin/sh
- test_description = ' merge-recursive options+ test_description = ' merge-recursive space options
* [ master] Clarify
! [ remote] Remove cruft diff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh
new file mode 100755
index 0000000..fbec68c
--- /dev/null
+++ b/t/t3034-merge-recursive-rename-options.sh @@ -0,0 +1,235 @@
+ #!/bin/sh + + test_description = ' merge-recursive rename options+ + Test rename detection by examining rename/delete conflicts.+ + * ( HEAD -> rename) rename+ | * ( master) delete+ | /+ * base+ + git diff --name-status base master+ D 0 -old+ D 1 -old+ D 2 -old+ D 3 -old+ + git diff --name-status -M01 base rename+ R025 0 -old 0 -new+ R050 1 -old 1 -new+ R075 2 -old 2 -new+ R100 3 -old 3 -new+ + Actual similarity indices are parsed from diff output. We rely on the fact that+ they are rounded down ( see, e.g., Documentation/diff-generate-patch.txt, which+ mentions this in a different context) .+ ' + + . ./test-lib.sh+ + get_expected_stages () { + git checkout rename -- $1 -new && + git ls-files --stage $1 -new >expected-stages-undetected-$1 && + sed "s/ 0 / 2 /" <expected-stages-undetected-$1 \ + >expected-stages-detected-$1 && + git read-tree -u --reset HEAD+ } + + rename_detected () { + git ls-files --stage $1 -old $1 -new >stages-actual-$1 && + test_cmp expected-stages-detected-$1 stages-actual-$1 + } + + rename_undetected () { + git ls-files --stage $1 -old $1 -new >stages-actual-$1 && + test_cmp expected-stages-undetected-$1 stages-actual-$1 + } + + check_common () { + git ls-files --stage >stages-actual && + test_line_count = 4 stages-actual+ } + + check_threshold_0 () { + check_common && + rename_detected 0 && + rename_detected 1 && + rename_detected 2 && + rename_detected 3 + } + + check_threshold_1 () { + check_common && + rename_undetected 0 && + rename_detected 1 && + rename_detected 2 && + rename_detected 3 + } + + check_threshold_2 () { + check_common && + rename_undetected 0 && + rename_undetected 1 && + rename_detected 2 && + rename_detected 3 + } + + check_exact_renames () { + check_common && + rename_undetected 0 && + rename_undetected 1 && + rename_undetected 2 && + rename_detected 3 + } + + test_expect_success 'setup 1/2: basic setup' ' + cat <<-\E OF >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" && + 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 && + test_cmp diff-output grep-output && + 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 && + 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 + ' + + test_expect_success 'low rename threshold' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --find-renames= $th0 $tail && + check_threshold_0+ ' + + test_expect_success 'medium rename threshold' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --find-renames= $th1 $tail && + check_threshold_1+ ' + + test_expect_success 'high rename threshold' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --find-renames= $th2 $tail && + check_threshold_2+ ' + + test_expect_success 'exact renames only' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --find-renames= 100 % $tail && + check_exact_renames+ ' + + test_expect_success 'rename threshold is truncated' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --find-renames= 200 % $tail && + check_exact_renames+ ' + + test_expect_success 'last wins in --find-renames=<m> --find-renames=<n>' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive \ + --find-renames= $th0 --find-renames= $th2 $tail && + check_threshold_2+ ' + + test_expect_success '--find-renames resets threshold' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive \ + --find-renames= $th0 --find-renames $tail && + $check_50 + ' + + test_expect_success 'assumption for further tests: trivial merge succeeds' ' + git read-tree --reset -u HEAD && + git merge-recursive HEAD -- HEAD HEAD && + git diff --quiet --cached && + git merge-recursive --find-renames= $th0 HEAD -- HEAD HEAD && + git diff --quiet --cached && + git merge-recursive --find-renames= $th2 HEAD -- HEAD HEAD && + git diff --quiet --cached && + git merge-recursive --find-renames= 100 % HEAD -- HEAD HEAD && + git diff --quiet --cached+ ' + + test_expect_success '--find-renames rejects negative argument' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --find-renames= -25 \ + HEAD -- HEAD HEAD && + git diff --quiet --cached+ ' + + test_expect_success '--find-renames rejects non-numbers' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --find-renames= 0xf \ + HEAD -- HEAD HEAD && + git diff --quiet --cached+ ' + + test_done--
2.7.1.492.gd821b20
From: Felipe Gonçalves Assis <redacted>
Signed-off-by: Felipe Gonçalves Assis <redacted>
---
t/t3034-merge-recursive-rename-options.sh | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh
index fbec68c..c07a4cb 100755
--- a/t/t3034-merge-recursive-rename-options.sh
+++ b/t/t3034-merge-recursive-rename-options.sh @@ -83,6 +83,14 @@ check_exact_renames () {
rename_detected 3
}
+ check_no_renames () { + check_common && + rename_undetected 0 && + rename_undetected 1 && + rename_undetected 2 && + rename_undetected 3 + } + test_expect_success 'setup 1/2: basic setup' '
cat <<-\E OF >3-old &&
33a @@ -192,6 +200,12 @@ test_expect_success 'rename threshold is truncated' '
check_exact_renames
'
+ test_expect_success 'disabled rename detection' ' + git read-tree --reset -u HEAD && + git merge-recursive --no-renames $tail && + check_no_renames+ ' + test_expect_success 'last wins in --find-renames=<m> --find-renames=<n>' '
git read-tree --reset -u HEAD &&
test_must_fail git merge-recursive \ @@ -206,6 +220,18 @@ test_expect_success '--find-renames resets threshold' '
$check_50
'
+ test_expect_success 'last wins in --no-renames --find-renames' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --no-renames --find-renames $tail && + $check_50 + ' + + test_expect_success 'last wins in --find-renames --no-renames' ' + git read-tree --reset -u HEAD && + git merge-recursive --find-renames --no-renames $tail && + check_no_renames+ ' + test_expect_success 'assumption for further tests: trivial merge succeeds' '
git read-tree --reset -u HEAD &&
git merge-recursive HEAD -- HEAD HEAD && @@ -215,6 +241,8 @@ test_expect_success 'assumption for further tests: trivial merge succeeds' '
git merge-recursive --find-renames= $th2 HEAD -- HEAD HEAD &&
git diff --quiet --cached &&
git merge-recursive --find-renames= 100 % HEAD -- HEAD HEAD &&
+ git diff --quiet --cached && + git merge-recursive --no-renames HEAD -- HEAD HEAD && git diff --quiet --cached
'
--
2.7.1.492.gd821b20
From: Felipe Gonçalves Assis <redacted>
--find-renames= and --rename-threshold= should be aliases.
Signed-off-by: Felipe Gonçalves Assis <redacted>
---
t/t3034-merge-recursive-rename-options.sh | 46 +++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh
index c07a4cb..96d8767 100755
--- a/t/t3034-merge-recursive-rename-options.sh
+++ b/t/t3034-merge-recursive-rename-options.sh @@ -260,4 +260,50 @@ test_expect_success '--find-renames rejects non-numbers' '
git diff --quiet --cached
'
+ test_expect_success 'rename-threshold=<n> is a synonym for find-renames=<n>' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --rename-threshold= $th0 $tail && + check_threshold_0+ ' + + test_expect_success 'last wins in --no-renames --rename-threshold=<n>' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --no-renames --rename-threshold= $th0 $tail && + check_threshold_0+ ' + + test_expect_success 'last wins in --rename-threshold=<n> --no-renames' ' + git read-tree --reset -u HEAD && + git merge-recursive --rename-threshold= $th0 --no-renames $tail && + check_no_renames+ ' + + test_expect_success '--rename-threshold=<n> rejects negative argument' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --rename-threshold= -25 \ + HEAD -- HEAD HEAD && + git diff --quiet --cached+ ' + + test_expect_success '--rename-threshold=<n> rejects non-numbers' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --rename-threshold= 0xf \ + HEAD -- HEAD HEAD && + git diff --quiet --cached+ ' + + test_expect_success 'last wins in --rename-threshold=<m> --find-renames=<n>' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive \ + --rename-threshold= $th0 --find-renames= $th2 $tail && + check_threshold_2+ ' + + test_expect_success 'last wins in --find-renames=<m> --rename-threshold=<n>' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive \ + --find-renames= $th2 --rename-threshold= $th0 $tail && + check_threshold_0+ ' + test_done --
2.7.1.492.gd821b20
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 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 + 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 + 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 + 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 + 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
On Tue, Feb 23, 2016 at 7:50 PM, Eric Sunshine [off-list ref] wrote: On Tue, Feb 23, 2016 at 6:48 PM, Felipe Gonçalves Assis
[off-list ref] wrote: quoted + 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 &&
Oh, I totally forgot to mention a few style issues:
1. use 'test' rather than '['
2. place 'then' on its own line
3. drop semicolon
if test 50 -le "$th0"
then
...
elif test ...
then
...
On 23 February 2016 at 21:50, Eric Sunshine [off-list ref] wrote: On Tue, Feb 23, 2016 at 6:48 PM, Felipe Gonçalves Assis
[off-list ref] wrote: quoted 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 + 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?
Oh, it might be a minor thing, but I would like to make tests using
check_50 fail if setup 2/2 is skipped, instead of succeeding without
any actual checks.
quoted + 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.
Here I favoured the stricter test. If the string is not exactly "100",
then a lot should be reviewed.
Once more, thanks for the review. Your other comments should be
included in the next version of the patches.