Re: [PATCH 1/3] t7004: drop hardcoded tag count in invalid name test

[Junio C Hamano <gitster@pobox.com>]

Siddharth Shrimali <r.siddharth.shrimali@gmail.com> writes:

> The test 'trying to create a tag with a non-valid name should fail',
> checked that exactly one tag existed in the repository before and after
> attempting to create invalid tags.
>
> As pointed out by Junio, this makes the test brittle by relying on a
> specific global tag count. If future tests are added or removed before
> this test, the expected state changes and this test would break for
> completely unrelated reasons.
>
> Since we already use 'test_must_fail' to guarantee that the invalid
> tags are rejected by Git, counting the tags before and after is redundant.
>
> Drop the 'test_line_count = 1' checks so the test doesn't rely on the
> exact number of tags left behind by earlier tests.

The only thing I suggested was that relying on exact state before
this test makes this test brittle.  I do not necessarily think
"redundant" is bad.  Having belt-and-suspenders sometimes help.

Alternatively, if we wanted to catch a bug where "git tag" exits
with a non-zero status, satisfying test_must_fail, but still creates
the requested tag, then we could do

	git tag -l >tags-before &&
	test_must_fail git tag "" &&
	... random attempts to create with invalid names ...
	test_must_fail git tag "other~tag" &&
	git tag -l >tags-after &&
	test_cmp tags-before tags-after

instead.   And that is a belt-and-suspenders approach.

Having said that, the patch is already an improvement, so let's take
it as is.  Unless there are other things we may want to improve in
this or other patches in the series, that is.

Thanks.

> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
> ---
>  t/t7004-tag.sh | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index faf7d97fc4..6ca5c75b57 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -191,15 +191,11 @@ test_expect_success 'trying to create a tag with the name of one existing should
>  '
>  
>  test_expect_success 'trying to create a tag with a non-valid name should fail' '
> -	git tag -l >actual &&
> -	test_line_count = 1 actual &&
>  	test_must_fail git tag "" &&
>  	test_must_fail git tag .othertag &&
>  	test_must_fail git tag "other tag" &&
>  	test_must_fail git tag "othertag^" &&
> -	test_must_fail git tag "other~tag" &&
> -	git tag -l >actual &&
> -	test_line_count = 1 actual
> +	test_must_fail git tag "other~tag"
>  '
>  
>  test_expect_success 'creating a tag using HEAD directly should succeed' '