Re: [PATCH v5 8/8] fetch-pack: implement ref-in-want

From: Brandon Williams <bmwill@google.com>
Date: Jun 27, 2018, 18:18 UTC
Message-ID: <20180627181809.GJ19910@google.com>
In-Reply-To: <20180627180943.35472-1-jonathantanmy@google.com>

On 06/27, Jonathan Tan wrote:

Show 5 quoted lines

> > +test_expect_success 'setup repos for change-while-negotiating test' '
> 
> The tests that follow are basic ref-in-want tests, not tests on a repo
> that changes during negotiation - this would be just "setup repos for
> fetch tests".

That looks like a copy-paste error.

Show 14 quoted lines

> 
> > +test_expect_success 'fetching with exact OID' '
> > +	rm -rf local &&
> > +	cp -r "$LOCAL_PRISTINE" local &&
> > +	git -C local fetch origin $(git -C "$REPO" rev-parse d):refs/heads/actual &&
> > +
> > +	git -C "$REPO" rev-parse "d" >expected &&
> > +	git -C local rev-parse refs/heads/actual >actual &&
> > +	test_cmp expected actual
> > +'
> 
> Also verify that "want-ref refs/tags/d" is being sent over the wire, and
> not any "want ...". (If not we can't distinguish these from the usual
> non-want-ref behavior.) Same comment for the other tests.

I think your mistaken on how what this test is looking for. no want-ref line is going to be sent because we're requesting an exact OID here, not a ref. But I can add checks for want-ref in the tests that should be sending want-ref.

> 
> Other than that (and my other comments), this patch series looks good.

-- 
Brandon Williams