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

From: Jeff King <[email-masked]>
Date: Sat, 3 Jan 2015 06:41:13 -0500

On Fri, Jan 02, 2015 at 11:01:53PM +0200, Raphael Kubo da Costa wrote:

> In this case, num_ours and num_theirs had undefined values and a call to
> `git for-each-ref --format="%(upstream:track)"` could print spurious
> values such as
> 
>   [behind -111794512]
>   [ahead 38881640, behind 5103867]
> 
> even for repositories with one single commit.
> 
> We now properly verify stat_tracking_info()'s return value and do not
> print anything if it returns -1. This behavior also matches the
> documentation ("has no effect if the ref does not have tracking
> information associated with it").

Thanks, this iteration looks good to me, and this is definitely a bug
worth fixing.

> +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_cmp expected actual
> +'

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
 '

which IMHO makes it a little more obvious what the setup is doing. But I
am OK with it either way.

-Peff