Re: [PATCH v3] for-each-ref: Always check stat_tracking_info()'s return value.
- From
Jeff King <peff@peff.net>
- Date
- Jan 3, 2015, 11:41 UTC
- Message-ID
- <20150103114113.GC27793@peff.net>
- In-Reply-To
- <1420232513-13867-1-git-send-email-raphael.kubo.da.costa@intel.com>
On Fri, Jan 02, 2015 at 11:01:53PM +0200, Raphael Kubo da Costa wrote:
Show 13 quoted lines
> 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.
Show 9 quoted lines
> +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