Re: [PATCH] tests: Introduce test_seq

> Jeff King <peff@peff.net> writes:
> 
> > On Fri, Aug 03, 2012 at 09:57:15PM +0200, Michał Kiedrowicz wrote:
> >
> >> 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...
> >
> >> 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...
> >
> >> +# 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?