git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/2] test-lib: teach test_seq the -f option

From
Todd Zullinger <tmz@pobox.com>
Date
Jun 23, 2025, 17:27 UTC
Message-ID
<aFmOazVXXGpt8bLB@teonanacatl.net>
In-Reply-To
<20250623105625.GB654412@coredump.intra.peff.net>
Jeff King wrote:
Show 5 quoted lines
> The "seq" tool has a "-f" option to produce printf-style formatted
> lines. Let's teach our test_seq helper the same trick. This lets us get
> rid of some shell loops in test snippets (which are particularly verbose
> in our test suite because we have to "|| return 1" to keep the &&-chain
> going).
This is a nice improvement.
Show 27 quoted lines
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index bee4a2ca34..8c176f4efc 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -1454,6 +1454,13 @@ test_cmp_fspath () {
>  # from 1.
>  
>  test_seq () {
> +	local fmt="%d"
> +	case "$1" in
> +	-f)
> +		fmt="$2"
> +		shift 2
> +		;;
> +	esac
>  	case $# in
>  	1)	set 1 "$@" ;;
>  	2)	;;
> @@ -1462,7 +1469,7 @@ test_seq () {
>  	test_seq_counter__=$1
>  	while test "$test_seq_counter__" -le "$2"
>  	do
> -		echo "$test_seq_counter__"
> +		printf "$fmt\n" "$test_seq_counter__"
>  		test_seq_counter__=$(( $test_seq_counter__ + 1 ))
>  	done
>  }

Is it a sharp edge worth caring about that someone might write `test_seq -f 1 5` where we'd pass 1 as the format string?

If so, perhaps a check like this might be sufficient to catch it early?

	diff --git i/t/test-lib-functions.sh w/t/test-lib-functions.sh
	index 8c176f4efc..87b59d5895 100644
	--- i/t/test-lib-functions.sh
	+++ w/t/test-lib-functions.sh
	@@ -1458,6 +1458,10 @@ test_seq () {
		case "$1" in
		-f)
			fmt="$2"
	+		case "$fmt" in
	+			*%*)	: ;;
	+			*)	BUG "no % in -f argument" ;;
	+		esac
			shift 2
			;;
		esac

I don't know whether it's worth the extra code or not. I just wondered about how it would fail in the face of a minor typo. It certainly should cause any test to fail if it were to output 1 instead of the intended format string, so it's arguably fine as-is.

Adding -f to the usage note above, as Justin suggested might help folks avoid making the mistake of cuddling the format string against -f, e.g.: -f%d. That is caught by the parameter count check (though perhaps not everyone would notice why, thinking they did pass an argument to -f).

-- 
Todd
Previous: Justin ToblerNext: Jeff King
Message 10 of 14 in “test_seq format option”
  1. 0/2 test_seq format optionJeff King, Jun 23, 2025
  2. 1/2 t7422: replace confusing printf with echoJeff King, Jun 23, 2025
  3. Eric SunshineJun 23, 2025
  4. Jeff KingJun 24, 2025
  5. 2/2 test-lib: teach test_seq the -f optionJeff King, Jun 23, 2025
  6. Junio C HamanoJun 23, 2025
  7. Justin ToblerJun 23, 2025
  8. 3/2 test-lib: document test_seq's "-f" optionJeff King, Jun 24, 2025
  9. Justin ToblerJun 25, 2025
  10. Todd ZullingerJun 23, 2025
  11. Jeff KingJun 24, 2025
  12. Todd ZullingerJun 24, 2025
  13. Eric SunshineJun 24, 2025
  14. Jeff KingJun 24, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.