Re: [PATCH] tests: Introduce test_seq

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michał Kiedrowicz <michal.kiedrowicz@xxxxxxxxx> writes:

> 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.
>
> Replace them with test_seq that is implemented with a Perl snippet
> (proposed by Jeff).  This is better than inlining this snippet
> everywhere it's needed because it's easier to read and it's easier to
> change the implementation (e.g. to C) if we ever decide to remove Perl
> from the test suite.
>
> Note that test_seq is not a complete replacement for seq(1).  It just
> has what we need now.
>
> There are also many places that do `for i in 1 2 3 ...` but I'm not sure
> if it's worth converting them to test_seq.  That would introduce running
> more processes of Perl.
>
> Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@xxxxxxxxx>
> ---

Thanks; Jeff, ack?

I have one minor nit that I am tempted to fix while queuing---see
below.

> Changes since previous version:
>
> 	* Removed "This commit replaces" from commit message
> 	* Reworded test_seq description
> 	* Now $first and $last are passed to Perl as arguments
>
>  t/perf/perf-lib.sh      |  2 +-
>  t/t5551-http-fetch.sh   |  2 +-
>  t/test-lib-functions.sh | 20 ++++++++++++++++++++
>  3 files changed, 22 insertions(+), 2 deletions(-)
>
> 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
>  			say >&3 "running: $2"
>  			if test_run_perf_ "$2"
>  			then
> diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
> index fadf2f2..91eaf53 100755
> --- a/t/t5551-http-fetch.sh
> +++ b/t/t5551-http-fetch.sh
> @@ -114,7 +114,7 @@ test -n "$GIT_TEST_LONG" && test_set_prereq EXPENSIVE
>  test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '
>  	(
>  	cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> -	for i in `seq 50000`
> +	for i in `test_seq 50000`
>  	do
>  		echo "commit refs/heads/too-many-refs"
>  		echo "mark :$i"
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 80daaca..c8b4ae3 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -530,6 +530,26 @@ test_cmp() {
>  	$GIT_TEST_CMP "$@"
>  }
>  
> +# Print a sequence of numbers or letters in increasing order.  This is
> +# similar to GNU seq(1), but the latter might not be available
> +# everywhere.  It may be used like:
> +#
> +#	for i in `test_seq 100`; do
> +#		for j in `test_seq 10 20`; do
> +#			for k in `test_seq a z`; do
> +#				echo $i-$j-$k
> +#			done
> +#		done
> +#	done
> +
> +test_seq () {
> +	test $# = 2 && { first=$1; shift; } || first=1
> +	test $# = 1 ||
> +	error "bug in the test script: not 1 or 2 parameters to test_seq"
> +	last=$1
> +	"$PERL_PATH" -le 'print for "$ARGV[0]".."$ARGV[1]"' "$first" "$last"

I'd prefer not to have dq around $ARGV[]; is there a reason to have
one around these?

> +}
> +
>  # This function can be used to schedule some commands to be run
>  # unconditionally at the end of the test to restore sanity:
>  #
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]