Re: [GSoC] [PATCH v5 1/1] t9811: Improve test coverage and clarity

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 8, 2025, 21:21 UTC
Message-ID: <xmqqbjt6wdrk.fsf@gitster.g>
In-Reply-To: <20250408114841.58592-2-anthonywang03@icloud.com>

Anthony Wang <anthonywang513@gmail.com> writes:

> Subject: Re: [GSoC] [PATCH v5 1/1] t9811: Improve test coverage and clarity

"Improve" -> "improve".

I am not sure about "clarity", but I agree that the main thrust of this change is no longer "avoid hiding exit status of git behind pipe", but more about "use 'show-ref --verify' to validate the right thing (meaning: grepping just for a string that could be a substring is an unreliable test)". Perhaps that is giving more clarity? coverage to test negative outcome is "while at it", just like we (incidentally) lost pipes that used to hide exit status, so I am not sure if it deserves mention on the commit title.

    t9811: be more precise to check tag creation

    The tests grep tagnames they expect to exist from "git tag"
    output, which can be fooled by false positive if an unexpected
    tag whose name has the expected tagname as its substring.  Fix
    them by using "git show-ref --verify" instead.

    While we are at it, add a negative test to verify that a tag
    that is involved in earlier tests that is not supposed to appear
    in the result does indeed not appear in the resulting
    repository.

    Incidentally, this would also correct the problem the original
    had, which lost the exit status of "git tag" that was placed
    upstream of a pipe.

or something, perhaps?

Also you'd need to sign-off your patch.

Thanks.

Show 35 quoted lines

> Remove the pipe following the `git tag`, ensuring the exit code is not 
> hidden. Add explicit verification to check for expected and unexpected 
> tags, increasing specificity and future-proofing a portion of the test.
>
> ---
>  t/t9811-git-p4-label-import.sh | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh
> index 5ac5383fb7..593de09eb4 100755
> --- a/t/t9811-git-p4-label-import.sh
> +++ b/t/t9811-git-p4-label-import.sh
> @@ -95,9 +95,10 @@ test_expect_success 'two labels on the same changelist' '
>  		cd "$git" &&
>  		git p4 sync --import-labels &&
>  
> -		git tag | grep TAG_F1 &&
> -		git tag | grep -q TAG_F1_1 &&
> -		git tag | grep -q TAG_F1_2 &&
> +		git tag &&
> +		git show-ref --verify refs/tags/TAG_F1_1 &&
> +		git show-ref --verify refs/tags/TAG_F1_2 &&
> +		test_must_fail git show-ref --verify refs/tags/TAG_F1_ONLY &&
>  
>  		cd main &&
>  
> @@ -208,7 +209,7 @@ test_expect_success 'use git config to enable import/export of tags' '
>  		git p4 rebase --verbose &&
>  		git p4 submit --verbose &&
>  		git tag &&
> -		git tag | grep TAG_F1_1
> +		git show-ref --verify refs/tags/TAG_F1_1 &&
>  	) &&
>  	(
>  		cd "$cli" &&