Re: [PATCH v2 3/4] t5704: match tests to modern style

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 23, 2012, 23:36 UTC
Message-ID: <7vzkc915rx.fsf@alter.siamese.dyndns.org>
In-Reply-To: <c6579adc648119fbd1c54e2a1a9a0de86e0e8b57.1329988335.git.trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

> Fix all of them.  There's a catch to the last point: test_commit
> creates a tag.  We still change it to test_commit, and explicitly
> delete the tags, so as to highlight that the test relies on not having
> them.

I do not have an objection to the use of these three test_commits, and I do not have an objection to delete the extra three tags they create so that the set of refs in the repository matches what the later test expects, either.

But I found the explanation a bit iffy.

Show 16 quoted lines

>  test_expect_success 'setup' '
> +	test_commit initial &&
>  	test_tick &&
>  	git tag -m tag tag &&
> +	test_commit second &&
> +	test_commit third &&
> +	git tag -d initial &&
> +	git tag -d second &&
> +	git tag -d third
>  '
>  
>  test_expect_success 'tags can be excluded by rev-list options' '
>  	git bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&
>  	git ls-remote bundle > output &&
>  	! grep tag output
>  '

If you do not delete 'third' tag, which matches the tip of the current branch, the resulting bundle created with "create --all" would end up containing that tag, and you will see it in the ls-remote output.

But the funny thing is that you can leave initial and second in the repository and the resulting bundle still passes the test. 'initial', 'second' and 'tag' are excluded. Exclusion of 'tag' tag (sheesh, it makes this conversation more confusing than necessary) is what this test checks, and the date range given to the rev-list ensures that initial and second are not included.

But the tip of the current branch and the lightweight 'third' tag both point at the same commit object, and that, together with the fact that we ask for '--all', is the reason why it is included in the result, making the test fail.

So perhaps a better fix may be to do something like the attached on top, and rewrite everything after "Fix all of them".

	... Fix all of them.

        Also rename the manually created tag 'tag' that points at a commit
        that is older than the --since threshold a later test uses, to a
        more descriptive 'ancienttag', and update the check that reads
        from the resulting bundle with ls-remote to look for 'ancienttag'.
        The purpose of this test is to make sure that the tag that
        predates the date range is not in the resulting bundle, but
        because these test_commit also will create tags, one of which
        (namely, 'third') points at a commit that is newer than that
        threshold, ls-remote will list it in its output. Looking for 'tag'
        will match refs/tags/third, making the test incorrectly fail.

diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index a51c8b0..3c436e7 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -6,18 +6,15 @@ test_description='some bundle related tests'
 test_expect_success 'setup' '
 	test_commit initial &&
 	test_tick &&
-	git tag -m tag tag &&
+	git tag -m tag antienttag &&
 	test_commit second &&
-	test_commit third &&
-	git tag -d initial &&
-	git tag -d second &&
-	git tag -d third
+	test_commit third
 '
 
 test_expect_success 'tags can be excluded by rev-list options' '
 	git bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&
 	git ls-remote bundle > output &&
-	! grep tag output
+	! grep antienttag output
 '
 
 test_expect_success 'die if bundle file cannot be created' '