Re: [PATCHv5 20/23] git notes merge: Add testcases for merging notes trees at different fanouts

Subject: [PATCH] portability fix for [20/23]

    test "$(line generator | wc -l)" = $expected_number_of_lines (bad)

is not portable, as "wc -l" can prefix the number with whitespaces.

Either write the $(... | wc -l) without enclosing in a dq pair, i.e.

    test $(line generator | wc -l) = $expected_number_of_lines (good)

or compare them numerically with 

    test "$(... | wc -l)" -eq $num (ok)

The former is preferred for readability.

---
 t/t3311-notes-merge-fanout.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t3311-notes-merge-fanout.sh b/t/t3311-notes-merge-fanout.sh
index d1c7b69..93516ef 100755
--- a/t/t3311-notes-merge-fanout.sh
+++ b/t/t3311-notes-merge-fanout.sh

@@ -123,8 +123,8 @@ test_expect_success 'Add a few hundred commits w/notes to trigger fanout (x -> y
 	done &&
 	test "$(git rev-parse refs/notes/y)" != "$(git rev-parse refs/notes/x)" &&
 	# Expected number of commits and notes
-	test "$(git rev-list HEAD | wc -l)" = "$num" &&
-	test "$(git notes list | wc -l)" = "$num" &&
+	test $(git rev-list HEAD | wc -l) = $num &&
+	test $(git notes list | wc -l) = $num &&
 	# 5 first notes unchanged
 	verify_notes y commit5
 '

-- 
1.7.3.2.193.g78bbb

On Friday 29 October 2010, Junio C Hamano wrote:

Subject: [PATCH] portability fix for [20/23]

    test "$(line generator | wc -l)" = $expected_number_of_lines (bad)

is not portable, as "wc -l" can prefix the number with whitespaces.

Either write the $(... | wc -l) without enclosing in a dq pair, i.e.

    test $(line generator | wc -l) = $expected_number_of_lines (good)

or compare them numerically with

    test "$(... | wc -l)" -eq $num (ok)

The former is preferred for readability.

Ack. I'll fold this fix into the next iteration.

Thanks!

...Johan

-- 
Johan Herland, [off-list ref]
www.herland.net