Re: [RFC PATCH 1/2] t5412: test receive-pack connectivity check
- From
Justin Tobler <jltobler@gmail.com>
- Date
- May 19, 2025, 21:08 UTC
- Message-ID
- <g2sthtaz5u4zj56bc7clqpxkgcn6leqbbim3kvsvqsaxe2io5o@kmzit5vi7trh>
- In-Reply-To
- <aBtgCYYKPuowEeNQ@pks.im>
On 25/05/07 03:28PM, Patrick Steinhardt wrote:
Show 10 quoted lines
> On Tue, May 06, 2025 at 10:02:48PM -0500, Justin Tobler wrote: > > @@ -630,6 +630,7 @@ integration_tests = [ > > 't5409-colorize-remote-messages.sh', > > 't5410-receive-pack-alternates.sh', > > 't5411-proc-receive-hook.sh', > > + 't5412-receive-pack.sh', > > Instead of creating a new test file, do we maybe want to generalize > "t5410-receive-pack-alternates.sh"? Just a suggestion, this is not a > strong requirement from my side.
That makes sense to me. I'll generalize "t5410" and add the test there.
Show 5 quoted lines
> > + # Capture git-send-pack(1) output sent to git-receive-pack(1). > > + git -C repo send-pack ../setup.git --all \ > > The `-C repo` shouldn't be necessary at all, should it? The repository > in which it runs is specified via the first parameter.
The repository specified by the first parameter is the repository the pack is being sent to. We still need to define the repository on the sending side.
Show 6 quoted lines
> > + test_grep "fatal: Failed to traverse parents" actual && > > + test_must_fail git -C remote.git cat-file -e $(git -C repo rev-parse HEAD) > > I'm a bit surprised by the error message though. First, why is it on > stdout? Second, shouldn't there be some hint that the connectivity check > has failed in the error message?
From my understanding, git-receive-pack(1) also writes the errors it encounters on stdout using sideband. On stderr, git-receive-pack(1) does print the git-rev-list(1) errors it encounters also so we could just capture that and use it in the assertions instead.
When the connectivity check fails, the command error string is set to "missing necessary objects" and outputted on stdout. I will also add a check for this too.
-Justin