Subject: Re: [PATCH v4 2/2] ref-filter: add new "describe" atom

From: Junio C Hamano <[email-masked]>
Date: Mon, 24 Jul 2023 10:21:15 -0700

Kousik Sanagavarapu <[email-masked]> writes:

> +test_expect_success 'err on bad describe atom arg' '
> +	(
> +		cd describe-repo &&
> +
> +		# The bad arg is the only arg passed to describe atom
> +		cat >expect <<-\EOF &&
> +		fatal: unrecognized %(describe) argument: baz
> +		EOF
> +		! git for-each-ref --format="%(describe:baz)" \
> +			refs/heads/master 2>actual &&
> +		test_cmp expect actual &&

Instead of "! git something", use of "test_must_fail git something" is
recommended.  The former would pass upon a crashing "git" happily, but
the latter would complain if "git" segfaults.

> +		# The bad arg is in the middle of the option string
> +		# passed to the describe atom
> +		cat >expect <<-\EOF &&
> +		fatal: unrecognized %(describe) argument: qux=1,abbrev=14
> +		EOF
> +		! git for-each-ref \
> +			--format="%(describe:tags,qux=1,abbrev=14)" \
> +			ref/heads/master 2>actual &&

Ditto.

> +		test_cmp expect actual
> +	)
> +'

Other than that, both patches looked good to me.  Thanks.