{"thread":{"id":"41492","subject":"[PATCH v2 0/3] Tests for merge-recursive rename options","startedAt":"2016-02-23T23:48:09Z","lastAt":"2016-02-24T01:15:35Z","messageCount":7,"participants":["Felipe Gonçalves Assis","Eric Sunshine"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"279096","messageId":"1456271292-4652-1-git-send-email-felipegassis@gmail.com","threadId":"41492","inReplyTo":null,"subject":"[PATCH v2 0/3] Tests for merge-recursive rename options","fromName":"Felipe Gonçalves Assis","fromEmail":"felipeg.assis@gmail.com","sentAt":"2016-02-23T23:48:09Z","receivedAt":"2016-02-23T23:48:09Z","isPatch":true,"sender":{"key":"felipeg.assis@gmail.com","avatar":null},"body":"Get rid of blatant bash-isms in favour of simple and portable constructions.\n\nFelipe Gonçalves Assis (3):\n t3034: add rename threshold tests\n t3034: test option to disable renames\n t3034: test deprecated interface\n\n ...s.sh => t3032-merge-recursive-space-options.sh} | 2 +-\n t/t3034-merge-recursive-rename-options.sh | 309 +++++++++++++++++++++\n 2 files changed, 310 insertions(+), 1 deletion(-)\n rename t/{t3032-merge-recursive-options.sh => t3032-merge-recursive-space-options.sh} (99%)\n create mode 100755 t/t3034-merge-recursive-rename-options.sh\n\n-- \n2.7.1.492.gd821b20\n"},{"id":"279097","messageId":"1456271292-4652-2-git-send-email-felipegassis@gmail.com","threadId":"41492","inReplyTo":"1456271292-4652-1-git-send-email-felipegassis@gmail.com","subject":"[PATCH 1/3] t3034: add rename threshold tests","fromName":"Felipe Gonçalves Assis","fromEmail":"felipeg.assis@gmail.com","sentAt":"2016-02-23T23:48:10Z","receivedAt":"2016-02-23T23:48:10Z","isPatch":true,"sender":{"key":"felipeg.assis@gmail.com","avatar":null},"body":"From: Felipe Gonçalves Assis \n\n10ae752 (merge-recursive: option to specify rename threshold,\n2010-09-27) introduced this feature but did not include any tests.\n\nThe tests use the new option --find-renames, which replaces the then\nintroduced and now deprecated option --rename-threshold.\n\nAlso update name and description of t3032 for consistency:\n\"merge-recursive options\" -> \"merge-recursive space options\"\n\nSigned-off-by: Felipe Gonçalves Assis \n---\n ...s.sh => t3032-merge-recursive-space-options.sh} | 2 +-\n t/t3034-merge-recursive-rename-options.sh | 235 +++++++++++++++++++++\n 2 files changed, 236 insertions(+), 1 deletion(-)\n rename t/{t3032-merge-recursive-options.sh => t3032-merge-recursive-space-options.sh} (99%)\n create mode 100755 t/t3034-merge-recursive-rename-options.sh\n\ndiff --git a/t/t3032-merge-recursive-options.sh b/t/t3032-merge-recursive-space-options.sh\nsimilarity index 99%\nrename from t/t3032-merge-recursive-options.sh\nrename to t/t3032-merge-recursive-space-options.sh\nindex 4029c9c..b56180e 100755\n--- a/t/t3032-merge-recursive-options.sh\n+++ b/t/t3032-merge-recursive-space-options.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='merge-recursive options\n+test_description='merge-recursive space options\n \n * [master] Clarify\n ! [remote] Remove cruft\ndiff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh\nnew file mode 100755\nindex 0000000..fbec68c\n--- /dev/null\n+++ b/t/t3034-merge-recursive-rename-options.sh\n@@ -0,0 +1,235 @@\n+#!/bin/sh\n+\n+test_description='merge-recursive rename options\n+\n+Test rename detection by examining rename/delete conflicts.\n+\n+* (HEAD -> rename) rename\n+| * (master) delete\n+|/\n+* base\n+\n+git diff --name-status base master\n+D\t0-old\n+D\t1-old\n+D\t2-old\n+D\t3-old\n+\n+git diff --name-status -M01 base rename\n+R025 0-old 0-new\n+R050 1-old 1-new\n+R075 2-old 2-new\n+R100 3-old 3-new\n+\n+Actual similarity indices are parsed from diff output. We rely on the fact that\n+they are rounded down (see, e.g., Documentation/diff-generate-patch.txt, which\n+mentions this in a different context).\n+'\n+\n+. ./test-lib.sh\n+\n+get_expected_stages () {\n+\tgit checkout rename -- $1-new &&\n+\tgit ls-files --stage $1-new >expected-stages-undetected-$1 &&\n+\tsed \"s/ 0\t/ 2\t/\" expected-stages-detected-$1 &&\n+\tgit read-tree -u --reset HEAD\n+}\n+\n+rename_detected () {\n+\tgit ls-files --stage $1-old $1-new >stages-actual-$1 &&\n+\ttest_cmp expected-stages-detected-$1 stages-actual-$1\n+}\n+\n+rename_undetected () {\n+\tgit ls-files --stage $1-old $1-new >stages-actual-$1 &&\n+\ttest_cmp expected-stages-undetected-$1 stages-actual-$1\n+}\n+\n+check_common () {\n+\tgit ls-files --stage >stages-actual &&\n+\ttest_line_count = 4 stages-actual\n+}\n+\n+check_threshold_0 () {\n+\tcheck_common &&\n+\trename_detected 0 &&\n+\trename_detected 1 &&\n+\trename_detected 2 &&\n+\trename_detected 3\n+}\n+\n+check_threshold_1 () {\n+\tcheck_common &&\n+\trename_undetected 0 &&\n+\trename_detected 1 &&\n+\trename_detected 2 &&\n+\trename_detected 3\n+}\n+\n+check_threshold_2 () {\n+\tcheck_common &&\n+\trename_undetected 0 &&\n+\trename_undetected 1 &&\n+\trename_detected 2 &&\n+\trename_detected 3\n+}\n+\n+check_exact_renames () {\n+\tcheck_common &&\n+\trename_undetected 0 &&\n+\trename_undetected 1 &&\n+\trename_undetected 2 &&\n+\trename_detected 3\n+}\n+\n+test_expect_success 'setup 1/2: basic setup' '\n+\tcat <<-\\EOF >3-old &&\n+\t33a\n+\t33b\n+\t33c\n+\t33d\n+\tEOF\n+\tsed s/33/22/ <3-old >2-old &&\n+\tsed s/33/11/ <3-old >1-old &&\n+\tsed s/33/00/ <3-old >0-old &&\n+\tgit add [0-3]-old &&\n+\tgit commit -m base &&\n+\tgit rm [0-3]-old &&\n+\tgit commit -m delete &&\n+\tgit checkout -b rename HEAD^ &&\n+\tcp 3-old 3-new &&\n+\tsed 1,1s/./x/ <2-old >2-new &&\n+\tsed 1,2s/./x/ <1-old >1-new &&\n+\tsed 1,3s/./x/ <0-old >0-new &&\n+\tgit add [0-3]-new &&\n+\tgit rm [0-3]-old &&\n+\tgit commit -m rename &&\n+\tget_expected_stages 0 &&\n+\tget_expected_stages 1 &&\n+\tget_expected_stages 2 &&\n+\tget_expected_stages 3 &&\n+\tcheck_50=\"false\" &&\n+\ttail=\"HEAD^ -- HEAD master\"\n+'\n+\n+test_expect_success 'setup 2/2: threshold array' '\n+\tgit diff --name-status -M01 HEAD^ HEAD >diff-output &&\n+\ttest_debug \"cat diff-output\" &&\n+\ttest_line_count = 4 diff-output &&\n+\tgrep \"R[0-9]\\{3\\}\t\\([0-3]\\)-old\t\\1-new\" diff-output \\\n+\t\t>grep-output &&\n+\ttest_cmp diff-output grep-output &&\n+\tth0=$(sed -n \"s/R\\(...\\)\t0-old\t0-new/\\1/p\" diff-output-0 &&\n+\tgit diff --name-status -M$th1 --diff-filter=R HEAD^ HEAD \\\n+\t\t>diff-output-1 &&\n+\tgit diff --name-status -M$th2 --diff-filter=R HEAD^ HEAD \\\n+\t\t>diff-output-2 &&\n+\tgit diff --name-status -M100% --diff-filter=R HEAD^ HEAD \\\n+\t\t>diff-output-3 &&\n+\ttest_line_count = 4 diff-output-0 &&\n+\ttest_line_count = 3 diff-output-1 &&\n+\ttest_line_count = 2 diff-output-2 &&\n+\ttest_line_count = 1 diff-output-3\n+'\n+\n+test_expect_success 'default similarity threshold is 50%' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive $tail &&\n+\t$check_50\n+'\n+\n+test_expect_success 'low rename threshold' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive --find-renames=$th0 $tail &&\n+\tcheck_threshold_0\n+'\n+\n+test_expect_success 'medium rename threshold' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive --find-renames=$th1 $tail &&\n+\tcheck_threshold_1\n+'\n+\n+test_expect_success 'high rename threshold' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive --find-renames=$th2 $tail &&\n+\tcheck_threshold_2\n+'\n+\n+test_expect_success 'exact renames only' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive --find-renames=100% $tail &&\n+\tcheck_exact_renames\n+'\n+\n+test_expect_success 'rename threshold is truncated' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive --find-renames=200% $tail &&\n+\tcheck_exact_renames\n+'\n+\n+test_expect_success 'last wins in --find-renames= --find-renames=' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive \\\n+\t\t--find-renames=$th0 --find-renames=$th2 $tail &&\n+\tcheck_threshold_2\n+'\n+\n+test_expect_success '--find-renames resets threshold' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive \\\n+\t\t--find-renames=$th0 --find-renames $tail &&\n+\t$check_50\n+'\n+\n+test_expect_success 'assumption for further tests: trivial merge succeeds' '\n+\tgit read-tree --reset -u HEAD &&\n+\tgit merge-recursive HEAD -- HEAD HEAD &&\n+\tgit diff --quiet --cached &&\n+\tgit merge-recursive --find-renames=$th0 HEAD -- HEAD HEAD &&\n+\tgit diff --quiet --cached &&\n+\tgit merge-recursive --find-renames=$th2 HEAD -- HEAD HEAD &&\n+\tgit diff --quiet --cached &&\n+\tgit merge-recursive --find-renames=100% HEAD -- HEAD HEAD &&\n+\tgit diff --quiet --cached\n+'\n+\n+test_expect_success '--find-renames rejects negative argument' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive --find-renames=-25 \\\n+\t\tHEAD -- HEAD HEAD &&\n+\tgit diff --quiet --cached\n+'\n+\n+test_expect_success '--find-renames rejects non-numbers' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive --find-renames=0xf \\\n+\t\tHEAD -- HEAD HEAD &&\n+\tgit diff --quiet --cached\n+'\n+\n+test_done\n-- \n2.7.1.492.gd821b20\n"},{"id":"279098","messageId":"1456271292-4652-3-git-send-email-felipegassis@gmail.com","threadId":"41492","inReplyTo":"1456271292-4652-1-git-send-email-felipegassis@gmail.com","subject":"[PATCH 2/3] t3034: test option to disable renames","fromName":"Felipe Gonçalves Assis","fromEmail":"felipeg.assis@gmail.com","sentAt":"2016-02-23T23:48:11Z","receivedAt":"2016-02-23T23:48:11Z","isPatch":true,"sender":{"key":"felipeg.assis@gmail.com","avatar":null},"body":"From: Felipe Gonçalves Assis \n\nSigned-off-by: Felipe Gonçalves Assis \n---\n t/t3034-merge-recursive-rename-options.sh | 28 ++++++++++++++++++++++++++++\n 1 file changed, 28 insertions(+)\n\ndiff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh\nindex fbec68c..c07a4cb 100755\n--- a/t/t3034-merge-recursive-rename-options.sh\n+++ b/t/t3034-merge-recursive-rename-options.sh\n@@ -83,6 +83,14 @@ check_exact_renames () {\n \trename_detected 3\n }\n \n+check_no_renames () {\n+\tcheck_common &&\n+\trename_undetected 0 &&\n+\trename_undetected 1 &&\n+\trename_undetected 2 &&\n+\trename_undetected 3\n+}\n+\n test_expect_success 'setup 1/2: basic setup' '\n \tcat <<-\\EOF >3-old &&\n \t33a\n@@ -192,6 +200,12 @@ test_expect_success 'rename threshold is truncated' '\n \tcheck_exact_renames\n '\n \n+test_expect_success 'disabled rename detection' '\n+\tgit read-tree --reset -u HEAD &&\n+\tgit merge-recursive --no-renames $tail &&\n+\tcheck_no_renames\n+'\n+\n test_expect_success 'last wins in --find-renames= --find-renames=' '\n \tgit read-tree --reset -u HEAD &&\n \ttest_must_fail git merge-recursive \\\n@@ -206,6 +220,18 @@ test_expect_success '--find-renames resets threshold' '\n \t$check_50\n '\n \n+test_expect_success 'last wins in --no-renames --find-renames' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive --no-renames --find-renames $tail &&\n+\t$check_50\n+'\n+\n+test_expect_success 'last wins in --find-renames --no-renames' '\n+\tgit read-tree --reset -u HEAD &&\n+\tgit merge-recursive --find-renames --no-renames $tail &&\n+\tcheck_no_renames\n+'\n+\n test_expect_success 'assumption for further tests: trivial merge succeeds' '\n \tgit read-tree --reset -u HEAD &&\n \tgit merge-recursive HEAD -- HEAD HEAD &&\n@@ -215,6 +241,8 @@ test_expect_success 'assumption for further tests: trivial merge succeeds' '\n \tgit merge-recursive --find-renames=$th2 HEAD -- HEAD HEAD &&\n \tgit diff --quiet --cached &&\n \tgit merge-recursive --find-renames=100% HEAD -- HEAD HEAD &&\n+\tgit diff --quiet --cached &&\n+\tgit merge-recursive --no-renames HEAD -- HEAD HEAD &&\n \tgit diff --quiet --cached\n '\n \n-- \n2.7.1.492.gd821b20\n"},{"id":"279099","messageId":"1456271292-4652-4-git-send-email-felipegassis@gmail.com","threadId":"41492","inReplyTo":"1456271292-4652-1-git-send-email-felipegassis@gmail.com","subject":"[PATCH 3/3] t3034: test deprecated interface","fromName":"Felipe Gonçalves Assis","fromEmail":"felipeg.assis@gmail.com","sentAt":"2016-02-23T23:48:12Z","receivedAt":"2016-02-23T23:48:12Z","isPatch":true,"sender":{"key":"felipeg.assis@gmail.com","avatar":null},"body":"From: Felipe Gonçalves Assis \n\n--find-renames= and --rename-threshold= should be aliases.\n\nSigned-off-by: Felipe Gonçalves Assis \n---\n t/t3034-merge-recursive-rename-options.sh | 46 +++++++++++++++++++++++++++++++\n 1 file changed, 46 insertions(+)\n\ndiff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh\nindex c07a4cb..96d8767 100755\n--- a/t/t3034-merge-recursive-rename-options.sh\n+++ b/t/t3034-merge-recursive-rename-options.sh\n@@ -260,4 +260,50 @@ test_expect_success '--find-renames rejects non-numbers' '\n \tgit diff --quiet --cached\n '\n \n+test_expect_success 'rename-threshold= is a synonym for find-renames=' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive --rename-threshold=$th0 $tail &&\n+\tcheck_threshold_0\n+'\n+\n+test_expect_success 'last wins in --no-renames --rename-threshold=' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive --no-renames --rename-threshold=$th0 $tail &&\n+\tcheck_threshold_0\n+'\n+\n+test_expect_success 'last wins in --rename-threshold= --no-renames' '\n+\tgit read-tree --reset -u HEAD &&\n+\tgit merge-recursive --rename-threshold=$th0 --no-renames $tail &&\n+\tcheck_no_renames\n+'\n+\n+test_expect_success '--rename-threshold= rejects negative argument' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive --rename-threshold=-25 \\\n+\t\tHEAD -- HEAD HEAD &&\n+\tgit diff --quiet --cached\n+'\n+\n+test_expect_success '--rename-threshold= rejects non-numbers' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive --rename-threshold=0xf \\\n+\t\tHEAD -- HEAD HEAD &&\n+\tgit diff --quiet --cached\n+'\n+\n+test_expect_success 'last wins in --rename-threshold= --find-renames=' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive \\\n+\t\t--rename-threshold=$th0 --find-renames=$th2 $tail &&\n+\tcheck_threshold_2\n+'\n+\n+test_expect_success 'last wins in --find-renames= --rename-threshold=' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git merge-recursive \\\n+\t\t--find-renames=$th2 --rename-threshold=$th0 $tail &&\n+\tcheck_threshold_0\n+'\n+\n test_done\n-- \n2.7.1.492.gd821b20\n"},{"id":"279100","messageId":"CAPig+cQ+MF7Nsvp7jASXPtPjNWT_g865ot45Y1Q347Y1iCDeYA@mail.gmail.com","threadId":"41492","inReplyTo":"1456271292-4652-2-git-send-email-felipegassis@gmail.com","subject":"Re: [PATCH 1/3] t3034: add rename threshold tests","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-02-24T00:50:12Z","receivedAt":"2016-02-24T00:50:12Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Feb 23, 2016 at 6:48 PM, Felipe Gonçalves Assis\n wrote:\n> 10ae752 (merge-recursive: option to specify rename threshold,\n> 2010-09-27) introduced this feature but did not include any tests.\n>\n> The tests use the new option --find-renames, which replaces the then\n> introduced and now deprecated option --rename-threshold.\n>\n> Also update name and description of t3032 for consistency:\n> \"merge-recursive options\" -> \"merge-recursive space options\"\n\nA few superficial comments below...\n\n> Signed-off-by: Felipe Gonçalves Assis \n> ---\n> diff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh\n> @@ -0,0 +1,235 @@\n> +test_expect_success 'setup 1/2: basic setup' '\n\nIf someone ever adds a third setup test, it's likely that that person\nwill forget to update this title to say \"1/3\" (ditto regarding \"setup\n2/2\" below). Perhaps just call this \"setup repo\" and 2/2 \"setup\nthresholds\" or something.\n\n> + cat <<-\\EOF >3-old &&\n> + 33a\n> + 33b\n> + 33c\n> + 33d\n> + EOF\n> + sed s/33/22/ <3-old >2-old &&\n> + sed s/33/11/ <3-old >1-old &&\n> + sed s/33/00/ <3-old >0-old &&\n> + git add [0-3]-old &&\n> + git commit -m base &&\n> + git rm [0-3]-old &&\n> + git commit -m delete &&\n> + git checkout -b rename HEAD^ &&\n> + cp 3-old 3-new &&\n> + sed 1,1s/./x/ <2-old >2-new &&\n> + sed 1,2s/./x/ <1-old >1-new &&\n> + sed 1,3s/./x/ <0-old >0-new &&\n> + git add [0-3]-new &&\n> + git rm [0-3]-old &&\n> + git commit -m rename &&\n> + get_expected_stages 0 &&\n> + get_expected_stages 1 &&\n> + get_expected_stages 2 &&\n> + get_expected_stages 3 &&\n> + check_50=\"false\" &&\n\nWhy isn't this assignment done in setup 2/2 where all the other\nassignments to 'check_50' are done?\n\n> + tail=\"HEAD^ -- HEAD master\"\n> +'\n> +\n> +test_expect_success 'setup 2/2: threshold array' '\n> + git diff --name-status -M01 HEAD^ HEAD >diff-output &&\n> + test_debug \"cat diff-output\" &&\n> + test_line_count = 4 diff-output &&\n> + grep \"R[0-9]\\{3\\} \\([0-3]\\)-old \\1-new\" diff-output \\\n> + >grep-output &&\n\nI'd be slightly concerned about the use of \\{3\\} with older grep's\nsuch as on Solaris, and would just code it as \"[0-9][0-9][0-9]\" and be\ndone with it, but perhaps it's not worth worrying about until someone\ncomplains.\n\n> + test_cmp diff-output grep-output &&\n\nSo, this is asserting that you only see \"renames\" and nothing else. Okay.\n\n> + th0=$(sed -n \"s/R\\(...\\) 0-old 0-new/\\1/p\" + th1=$(sed -n \"s/R\\(...\\) 1-old 1-new/\\1/p\" + th2=$(sed -n \"s/R\\(...\\) 2-old 2-new/\\1/p\" + th3=$(sed -n \"s/R\\(...\\) 3-old 3-new/\\1/p\" + test \"$th0\" -lt \"$th1\" &&\n> + test \"$th1\" -lt \"$th2\" &&\n> + test \"$th2\" -lt \"$th3\" &&\n> + test \"$th3\" = 100 &&\n\nIt's very slightly odd to see '=' rather than '-eq' among all these\nother algebraic operators ('-lt', '-le'), but not so odd that I'd\nmention it (unless I just did), and not necessarily worth changing.\n\n> + if [ 50 -le \"$th0\" ]; then\n> + check_50=check_threshold_0\n> + elif [ 50 -le \"$th1\" ]; then\n> + check_50=check_threshold_1\n> + elif [ 50 -le \"$th2\" ]; then\n> + check_50=check_threshold_2\n> + fi &&\n> + th0=\"$th0%\" &&\n> + th1=\"$th1%\" &&\n> + th2=\"$th2%\" &&\n> + th3=\"$th3%\"\n> +'\n> +\n> +test_expect_success 'assumption for tests: rename detection with diff' '\n> + git diff --name-status -M$th0 --diff-filter=R HEAD^ HEAD \\\n> + >diff-output-0 &&\n> + git diff --name-status -M$th1 --diff-filter=R HEAD^ HEAD \\\n> + >diff-output-1 &&\n> + git diff --name-status -M$th2 --diff-filter=R HEAD^ HEAD \\\n> + >diff-output-2 &&\n> + git diff --name-status -M100% --diff-filter=R HEAD^ HEAD \\\n> + >diff-output-3 &&\n> + test_line_count = 4 diff-output-0 &&\n> + test_line_count = 3 diff-output-1 &&\n> + test_line_count = 2 diff-output-2 &&\n> + test_line_count = 1 diff-output-3\n> +'\n> +\n> +test_expect_success 'default similarity threshold is 50%' '\n> + git read-tree --reset -u HEAD &&\n> + test_must_fail git merge-recursive $tail &&\n> + $check_50\n"},{"id":"279101","messageId":"CAPig+cR_p2aKU9VRpiyomdOo+Ucnxr82dh2uqBaR6TTsN9B4qw@mail.gmail.com","threadId":"41492","inReplyTo":"CAPig+cQ+MF7Nsvp7jASXPtPjNWT_g865ot45Y1Q347Y1iCDeYA@mail.gmail.com","subject":"Re: [PATCH 1/3] t3034: add rename threshold tests","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-02-24T00:55:53Z","receivedAt":"2016-02-24T00:55:53Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Feb 23, 2016 at 7:50 PM, Eric Sunshine wrote:\n> On Tue, Feb 23, 2016 at 6:48 PM, Felipe Gonçalves Assis\n> wrote:\n>> + if [ 50 -le \"$th0\" ]; then\n>> + check_50=check_threshold_0\n>> + elif [ 50 -le \"$th1\" ]; then\n>> + check_50=check_threshold_1\n>> + elif [ 50 -le \"$th2\" ]; then\n>> + check_50=check_threshold_2\n>> + fi &&\n\nOh, I totally forgot to mention a few style issues:\n\n1. use 'test' rather than '['\n2. place 'then' on its own line\n3. drop semicolon\n\n if test 50 -le \"$th0\"\n then\n ...\n elif test ...\n then\n ...\n"},{"id":"279103","messageId":"CALMa68oN-d1t67QZ8+7KetApbLgFXoZkJV93-QOeviT_JVPGdw@mail.gmail.com","threadId":"41492","inReplyTo":"CAPig+cQ+MF7Nsvp7jASXPtPjNWT_g865ot45Y1Q347Y1iCDeYA@mail.gmail.com","subject":"Re: [PATCH 1/3] t3034: add rename threshold tests","fromName":"Felipe Gonçalves Assis","fromEmail":"felipeg.assis@gmail.com","sentAt":"2016-02-24T01:15:35Z","receivedAt":"2016-02-24T01:15:35Z","isPatch":true,"sender":{"key":"felipeg.assis@gmail.com","avatar":null},"body":"On 23 February 2016 at 21:50, Eric Sunshine wrote:\n> On Tue, Feb 23, 2016 at 6:48 PM, Felipe Gonçalves Assis\n> wrote:\n>> 10ae752 (merge-recursive: option to specify rename threshold,\n>> 2010-09-27) introduced this feature but did not include any tests.\n>>\n>> The tests use the new option --find-renames, which replaces the then\n>> introduced and now deprecated option --rename-threshold.\n>>\n>> Also update name and description of t3032 for consistency:\n>> \"merge-recursive options\" -> \"merge-recursive space options\"\n>\n> A few superficial comments below...\n>\n>> + cat <<-\\EOF >3-old &&\n>> + 33a\n>> + 33b\n>> + 33c\n>> + 33d\n>> + EOF\n>> + sed s/33/22/ <3-old >2-old &&\n>> + sed s/33/11/ <3-old >1-old &&\n>> + sed s/33/00/ <3-old >0-old &&\n>> + git add [0-3]-old &&\n>> + git commit -m base &&\n>> + git rm [0-3]-old &&\n>> + git commit -m delete &&\n>> + git checkout -b rename HEAD^ &&\n>> + cp 3-old 3-new &&\n>> + sed 1,1s/./x/ <2-old >2-new &&\n>> + sed 1,2s/./x/ <1-old >1-new &&\n>> + sed 1,3s/./x/ <0-old >0-new &&\n>> + git add [0-3]-new &&\n>> + git rm [0-3]-old &&\n>> + git commit -m rename &&\n>> + get_expected_stages 0 &&\n>> + get_expected_stages 1 &&\n>> + get_expected_stages 2 &&\n>> + get_expected_stages 3 &&\n>> + check_50=\"false\" &&\n>\n> Why isn't this assignment done in setup 2/2 where all the other\n> assignments to 'check_50' are done?\n>\n\nOh, it might be a minor thing, but I would like to make tests using\ncheck_50 fail if setup 2/2 is skipped, instead of succeeding without\nany actual checks.\n\n>> + th0=$(sed -n \"s/R\\(...\\) 0-old 0-new/\\1/p\" > + th1=$(sed -n \"s/R\\(...\\) 1-old 1-new/\\1/p\" > + th2=$(sed -n \"s/R\\(...\\) 2-old 2-new/\\1/p\" > + th3=$(sed -n \"s/R\\(...\\) 3-old 3-new/\\1/p\" > + test \"$th0\" -lt \"$th1\" &&\n>> + test \"$th1\" -lt \"$th2\" &&\n>> + test \"$th2\" -lt \"$th3\" &&\n>> + test \"$th3\" = 100 &&\n>\n> It's very slightly odd to see '=' rather than '-eq' among all these\n> other algebraic operators ('-lt', '-le'), but not so odd that I'd\n> mention it (unless I just did), and not necessarily worth changing.\n>\n\nHere I favoured the stricter test. If the string is not exactly \"100\",\nthen a lot should be reviewed.\n\nOnce more, thanks for the review. Your other comments should be\nincluded in the next version of the patches.\n"}]}