Re: [PATCH v2 2/6] test-reach: add run_three_modes method

Junio C Hamano [off-list ref] writes:

SZEDER Gábor [off-list ref] writes:

quoted

quoted

While inspecting this code, I realized that the final test for
'commit_contains --tag' is silently dropping the '--tag' argument.
It should be quoted to include both.

Nit: while quoting the function's arguments does fix the issue, it
leaves the tests prone to the same issue in the future.  Wouldn't it
be better to use $@ inside the function to refer to all its arguments?

IOW, do it more like this?

quoted

quoted

-test_three_modes () {
+run_three_modes () {
 	test_when_finished rm -rf .git/objects/info/commit-graph &&
-	test-tool reach $1 <input >actual &&
+	$1 <input >actual &&

	"$@" <input >actual

i.e. treat each parameter as separate things without further getting
split at $IFS and ...

quoted

quoted

+test_three_modes () {
+	run_three_modes "test-tool reach $1"

	run_three_modes test-tool reach "$1"

... make sure there three things are sent as separate, by quoting
"$1" inside dq.

I think that makes sense.

I also noticed that 2/6 made "commti_contains --tag" enclosed in dq
pair for one test, but the next test after it has the identical one.

Here is what I queued in the meantime.

diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index 1b18e12a4e..1377849bf8 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh

@@ -55,18 +55,18 @@ test_expect_success 'setup' '
 
 run_three_modes () {
 	test_when_finished rm -rf .git/objects/info/commit-graph &&
-	$1 <input >actual &&
+	"$@" <input >actual &&
 	test_cmp expect actual &&
 	cp commit-graph-full .git/objects/info/commit-graph &&
-	$1 <input >actual &&
+	"$@" <input >actual &&
 	test_cmp expect actual &&
 	cp commit-graph-half .git/objects/info/commit-graph &&
-	$1 <input >actual &&
+	"$@" <input >actual &&
 	test_cmp expect actual
 }
 
 test_three_modes () {
-	run_three_modes "test-tool reach $1"
+	run_three_modes test-tool reach "$1"
 }
 
 test_expect_success 'ref_newer:miss' '

@@ -223,7 +223,7 @@ test_expect_success 'commit_contains:hit' '
 	EOF
 	echo "commit_contains(_,A,X,_):1" >expect &&
 	test_three_modes commit_contains &&
-	test_three_modes "commit_contains --tag"
+	test_three_modes commit_contains --tag
 '
 
 test_expect_success 'commit_contains:miss' '

-- 
2.19.0-216-g2d3b1c576c