Re: [PATCH v2 5/5] remote-curl: use post_rpc() for protocol v2 also

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 22, 2019, 13:18 UTC
Message-ID: <CAPig+cQNg26zHm=ns3PsmPHEsNo4M7hTSG8=oO_uKqEa4wf3Vw@mail.gmail.com>
In-Reply-To: <b0ff17d324d46822da9db898d187dcc5fb0467ca.1550780213.git.jonathantanmy@google.com>

On Thu, Feb 21, 2019 at 3:25 PM Jonathan Tan <jonathantanmy@google.com> wrote:

Show 18 quoted lines

> diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
> @@ -542,7 +542,38 @@ test_expect_success 'clone with http:// using protocol v2' '
> +test_expect_success 'clone big repository with http:// using protocol v2' '
> +       test_when_finished "rm -f log" &&
> +
> +       git init "$HTTPD_DOCUMENT_ROOT_PATH/big" &&
> +       # Ensure that the list of wants is greater than http.postbuffer below
> +       for i in $(test_seq 1 1500)
> +       do
> +               # do not use here-doc, because it requires a process
> +               # per loop iteration
> +               echo "commit refs/heads/too-many-refs-$i" &&
> +               echo "committer git <git@example.com> $i +0000" &&
> +               echo "data 0" &&
> +               echo "M 644 inline bla.txt" &&
> +               echo "data 4" &&
> +               echo "bla"
> +       done | git -C "$HTTPD_DOCUMENT_ROOT_PATH/big" fast-import &&

Although not outright wrong, maintaining an &&-chain in the loop body is a bit misleading here for a couple reasons. First, without "|| exit 1" at the end of the body, neither the loop nor the test itself will abort early if some command within the body fails. Second, the exit status of the loop is lost anyhow due to being upstream of a pipe.

By the way, I see that this same script already contains a couple tests with loops neglecting the trailing "|| exit 1", hence they too fail to abort early if something goes wrong inside the body (which, in both cases, contains a test_commit() invocation). Fixing those might be fodder as a follow-up patch for someone.