Re: [PATCH] tests: Introduce test_seq

Jeff King [off-list ref] writes:

On Fri, Aug 03, 2012 at 09:57:15PM +0200, Michał Kiedrowicz wrote:

quoted

Jeff King wrote:

	The seq command is GNU-ism, and is missing at least in older BSD
	releases and their derivatives, not to mention antique
	commercial Unixes.

	We already purged it in b3431bc (Don't use seq in tests, not
	everyone has it, 2007-05-02), but a few new instances have crept
	in. They went unnoticed because they are in scripts that are not
	run by default.

This commit replaces them with test_seq that is implemented with a Perl
snippet (proposed by Jeff).

Just say "Replace them with test_seq...", without "This commit".

Fine explanation, but...

quoted

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 5580c22..a1361e5 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh

@@ -163,7 +163,7 @@ test_perf () {
 		else
 			echo "perf $test_count - $1:"
 		fi
-		for i in $(seq 1 $GIT_PERF_REPEAT_COUNT); do
+		for i in $(test_seq 1 $GIT_PERF_REPEAT_COUNT); do

Two args to test_seq, but...

quoted

+# test_seq is a portable replacement for seq(1).
+# It may be used like:
+#
+#	for i in `test_seq 100`; do
+#		echo $i
+#	done
+
+test_seq () {
+	test $# = 1 ||
+	error "bug in the test script: not 1 parameter to test_seq"
+	last=$1
+	"$PERL_PATH" -le "print for 1..$last"
+}

it wants only one.

I think you would want:

  test $# = 1 && set -- 1 "$@"
  "$PERL_PATH" -le "print for $1..$2"

It might also be worth quoting the parameters like this:

  "$PERL_PATH" -le "print for '$1'..'$2'"

so that "test_seq a f" works, too.

Yeah, I like that last one, but then unlike the claim in the comment
before the function definition, it is not "a portable replacement
for seq(1)" at all, but something a lot more suited for our purpose.
So at least the comment needs to be updated.  I do not have strong
opinion on calling this test_seq when it acts differently from seq;
it is not confusing enough to make me push something longer that is
different from "seq", e.g. test_sequence.

Wouldn't it be cleaner and readable to write it like this

	"$PERL_PATH" -le 'print for $ARGV[0]..$ARGV[1]' "$1" "$2"

by the way?