Re: [PATCH 2/7] test-lib: add say_color_tap helper to emit TAP format

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 10, 2021, 00:39 UTC
Message-ID: <xmqqy2evyh7n.fsf@gitster.c.googlers.com>
In-Reply-To: <20210309160219.13779-3-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Follow-up 5099b99d25 (test-lib: Adjust output to be valid TAP format,
> 2010-06-24) and make it easy to distinguishing when we're emitting TAP
> directives (with the new "say_color_tap"), and when we just want to
> say something with colored text "say_color".

OK.

Show 6 quoted lines

> @@ -666,7 +673,7 @@ test_ok_ () {
>  		write_junit_xml_testcase "$*"
>  	fi
>  	test_success=$(($test_success + 1))
> -	say_color "" "ok $test_count - $@"
> +	say_color_tap "" "ok $test_count - $@"

Not your fault, but let's not forget that we should clean this up not to use "$@" here (and instead use $*).

> -	say_color warn "not ok $test_count - $@ # TODO known breakage"
> +	say_color_tap warn "not ok $test_count - $@ # TODO known breakage"

Likewise.

Yes, I know say_color (and hence its heir say_color_tap) accepts multiple arguments and eventually pastes them into a single string with "$*", but there is no point in producing multiple arguments out of "$@" here in this case.

fn0 () {
	cnt=0
	for arg
	do
		echo "$cnt: <$arg>"
		cnt=$(( cnt+1 ))
	done
}

fn1 () {
	fn0 "not - $@ # TODO"
}

fn1 1 "2 3"

--->

0: <not - 1> 1: <2 3 # TODO>