Re: [PATCH v4 08/20] mktag tests: don't create "mytag" twice

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ævar Arnfjörð Bjarmason  <avarab@xxxxxxxxx> writes:

> Change a test added in e0aaf781f6 (mktag.c: improve verification of
> tagger field and tests, 2008-03-27) to not create "mytag", which
> should only be created and verified at the end in an earlier test
> added in 446c6faec6 (New tests and en-passant modifications to mktag.,
> 2006-07-29).

If mktag fails to create a tag, presumably .git/refs/tags/mytag file
would be left empty.  Wouldn't "git tag -l" notice that it is not a
valid ref and omit it from its output?  I suspect if you corrupt the
contents of tag.sig temporarily while the first of the original
tests were running in such a way that mktag notices a bogosity, the
second half of the original would notice.

Having said that, I like the new way much better.

But do we even need to create the tag with update-ref in the first
place?  What does it check?  The fact that "mktag" only creates an
object and never creates a new ref to point at the newly created
object (hence we expect refs/tags/mytag does not yet exist)?

I am not complaining that it runs update-ref there or creates the
mytag tag.  I am just saying I do not understand what the test wants
to check by doing so.

> While we're at it let's prevent a similar logic error from creeping
> into the test by asserting that "mytag" doesn't exist before we create
> it. Let's do this by moving the test to use "update-ref", instead of
> our own homebrew ad-hoc refstore update.

Great.  I really like this part of the change that future-proofs us.

Thanks.

> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
>  t/t3800-mktag.sh | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
> index bbd148618e..b6dcdbebe6 100755
> --- a/t/t3800-mktag.sh
> +++ b/t/t3800-mktag.sh
> @@ -257,7 +257,7 @@ EOF
>  
>  test_expect_success \
>      'allow empty tag email' \
> -    'git mktag <tag.sig >.git/refs/tags/mytag'
> +    'git mktag <tag.sig'
>  
>  ############################################################
>  # 16. disallow spaces in tag email
> @@ -383,16 +383,9 @@ tagger T A Gger <tagger@xxxxxxxxxxx> 1206478233 -0500
>  
>  EOF
>  
> -test_expect_success \
> -    'create valid tag' \
> -    'git mktag <tag.sig >.git/refs/tags/mytag'
> -
> -############################################################
> -# 25. check mytag
> -
> -test_expect_success \
> -    'check mytag' \
> -    'git tag -l | grep mytag'
> -
> +test_expect_success 'create valid tag' '
> +	git mktag <tag.sig >hash &&
> +	git update-ref refs/tags/mytag $(cat hash) $(test_oid zero)
> +'
>  
>  test_done




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux