From: Jonathan Nieder <jrnieder@gmail.com>
Date: Sun, 07 Aug 2011 13:39:39 GMT
Subject: Re: [PATCH 2/3] bisect: add tests for bisection on bare repositories
Message-ID: <20110807133939.GA10748@elie.gateway.2wire.net>
In-Reply-To: <1312714240-23647-3-git-send-email-jon.seymour@gmail.com>

Jon Seymour wrote:

> We add a test to check that bisection works on bare repositories
> both when --no-checkout is specified explicitly and when it
> is defaulted.

Yay!

> --- a/t/t6030-bisect-porcelain.sh
> +++ b/t/t6030-bisect-porcelain.sh
> @@ -592,6 +592,34 @@ test_expect_success 'erroring out when using bad path parameters' '
>  	grep "bad path parameters" error.txt
>  '
>  
> +test_expect_success 'create bare repo' '
> +	git clone --bare . bare
> +'

I'd prefer to see separate clones for the two tests, so if one
catastrophically fails it does not affect the other.

> +
> +test_expect_success 'test bisection on bare repo - --no-checkout specified' '
> +	test_when_finished "cd .." &&
> +	cd bare &&
> +	git bisect start --no-checkout &&
> +	git bisect good $HASH1 &&
> +	git bisect bad $HASH4 &&
> +	git bisect run sh -c \
> +		"test \$(git rev-list BISECT_HEAD ^$HASH2 --max-count=1 | wc -l) = 0" \
> +		>../my_bisect_log.txt &&
> +	grep "$HASH3 is the first bad commit" ../my_bisect_log.txt &&
> +	git bisect reset'

I worry that "sh" might not actually be a POSIX shell on some systems.
Maybe a subshell could also simplify this.  Like so:

	git clone --bare . bare.nocheckout &&
	(
		cd bare &&
		git bisect start --no-checkout &&
		git bisect good $HASH1 &&
		git bisect bad $HASH4 &&
		git bisect run \
			eval "test \$(... " \
			>../log.nocheckout &&
		git bisect reset
	) &&
	grep "$HASH3 is the first bad commit" log.nocheckout

> +test_expect_success 'test bisection on bare repo - --no-checkout defaulted' '

Perhaps these could share code by using a function defined at the top
of the file.

With or without the changes described above, I like it.  Thanks.