Re: [PATCH] ref namespaces: tests

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 14, 2011, 23:13 UTC
Message-ID: <7v1uxs3177.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110714205055.GA26956@leaf>

Josh Triplett <josh@joshtriplett.org> writes:

Show 5 quoted lines

> Test pushing, pulling, and mirroring of repositories with ref
> namespaces.
>
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> Signed-off-by: Jamey Sharp <jamey@minilop.net>

Thanks.

>  create mode 100755 t/t5502-fetch-push-namespaces.sh

Isn't 5502 used already?

Show 9 quoted lines

> +test_expect_success setup '
> +	test_tick &&
> +	git init original &&
> +	(
> +		cd original &&
> +		i=0 &&
> +		while [ "$i" -lt 2 ]
> +		do
> +			echo "$i" > count &&

This is just style, but the test scripts prefer to spell these like this:

	while test "$i" -lt 2
	do
        	echo "$i" >count &&
                ...

to favor "test" over "[ ... ]", and omit SP between ">" redirection (or "<" for that matter) and the filename.

> +		git remote add pushee-namespaced "ext::git --namespace=namespace %s ../pushee" &&

Nice ;-).

Show 6 quoted lines

> +test_expect_success 'pushing into a repository using a ref namespace' '
> +	(
> +		cd original &&
> +		git push pushee-namespaced master &&
> +		git ls-remote pushee-namespaced > actual &&
> +		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba\trefs/heads/master\n" > expected &&

Could you avoid hardcoding the exact object names here? Your script knows what object should appear at refs/heads/master at "pushee-namespaced" (as you have pushed from the repository "original" you are in), so it may be something like:

	printf "%s\trefs/heads/mater\n" $(git rev-parse master) >expect

Same comment applies for all the other hardcoded object names.

Show 11 quoted lines

> +test_expect_success 'mirroring a repository using a ref namespace' '
> +	git clone --mirror pushee mirror &&
> +	(
> +		cd mirror &&
> +		git for-each-ref refs/ > actual &&
> +		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba commit\trefs/namespaces/namespace/refs/heads/master\n" > expected &&
> +		printf "fbdf4310c71b916568f04753f603fb24a0544227 commit\trefs/namespaces/namespace/refs/tags/0\n" >> expected &&
> +		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba commit\trefs/namespaces/namespace/refs/tags/1\n" >> expected &&
> +		test_cmp expected actual
> +	)
> +'

I am not sure what you are trying to test. "pushee" is pretending to be a hosting site that uses the namespace feature to house refs pushed from original in refs/namespaces/namespace/ so it is expected to have these refs under there. You didn't make any "git remote" configuration in either mirror nor pushee, so it is natural with or without the namespace feature that "git clone --mirror" would find them at the same place.

What hasn't been tested in the above is to see what actual refs pushee has with (cd pushee && git for-each-ref), and you could argue that this test is a proxy for that, but then you are assuming that "clone --mirror" is not broken, which means it would make debugging harder when this test does start failing---is it the basic namespace feature, or is it mirror cloning that acquired a bug to break this test?

Also I would have expected that some configuration that affects the central repository side (i.e. "pushee") to cause unsuspecting clients (e.g. "original" that pushes into it) to see only a subset of refs "pushee" has, which would be the real motivation for this feature. To show that off, we may also need tests that house two or more namespaces hosted in the same repository.