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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 24, 2023, 17:21 UTC
Message-ID: <xmqqfs5dql84.fsf@gitster.g>
In-Reply-To: <20230723162717.68123-3-five231003@gmail.com>

Kousik Sanagavarapu <five231003@gmail.com> writes:

Show 11 quoted lines

> +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.

Show 8 quoted lines

> +		# 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.