Re: [PATCH v3] for-each-ref: Always check stat_tracking_info()'s return value.

From: Raphael Kubo da Costa <rakuco@freebsd.org>
Date: Jan 3, 2015, 19:19 UTC
Message-ID: <868uhjzn7o.fsf@FreeBSD.org>
In-Reply-To: <20150103114113.GC27793@peff.net>

Jeff King <peff@peff.net> writes:

Show 23 quoted lines

> I think you could minimize this quite a bit as:
>
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index cba3454..f259c22 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -340,12 +340,11 @@ cat >expected <<EOF
>  EOF
>  
>  test_expect_success 'Check that :track[short] works when upstream is gone' '
> -	git branch --track to_delete master &&
> -	git branch --track parent_gone to_delete &&
> -	test_when_finished "git branch -D parent_gone" &&
> -	git branch -D to_delete &&
> -	git for-each-ref --format="%(upstream:track)" refs/heads/parent_gone >actual &&
> -	git for-each-ref --format="%(upstream:trackshort)" refs/heads/parent_gone >>actual &&
> +	test_when_finished "git config branch.master.merge refs/heads/master" &&
> +	git config branch.master.merge refs/heads/does-not-exist &&
> +	git for-each-ref \
> +		--format="%(upstream:track)$LF%(upstream:trackshort)" \
> +		refs/heads/master >actual &&
>  	test_cmp expected actual
>  '

Thanks Jeff (and Eric!) for the reviews so far. That does look much better (the original test case was a reduction of a failure in a Chromium test case).

One question about this suggestion: this test case actually depends on the remote manipulations done in the setup test to work, as otherwise we don't even test stat_tracking_info() because populate_value() would bail out earlier on branch_get() returning NULL due to the branch's remote not being set. Is it OK to continue assuming that?