Re: [PATCH v2 1/2] t5702: test fetch with multiple refspecs at a time

[Stefan Beller <sbeller@google.com>]

On Tue, Jun 5, 2018 at 2:40 PM Jonathan Tan <jonathantanmy@google.com> wrote:
>
> Extend the protocol v2 tests to also test fetches with multiple refspecs
> specified. This also covers the previously uncovered cases of fetching
> with prefix matching and fetching by SHA-1.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  t/t5702-protocol-v2.sh | 47 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
> diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
> index a4fe6508b..261e82b0f 100755
> --- a/t/t5702-protocol-v2.sh
> +++ b/t/t5702-protocol-v2.sh
> @@ -359,6 +359,53 @@ test_expect_success 'default refspec is used to filter ref when fetchcing' '
>         grep "ref-prefix refs/tags/" log
>  '
>
> +test_expect_success 'fetch supports various ways of have lines' '
> +       rm -rf server client trace &&

Can we move these deletions to test_when_finished of the previous(?) test
as well as have them here in a test_when_finished line?

> +       git init server &&
> +       test_commit -C server dwim &&
> +       TREE=$(git -C server rev-parse HEAD^{tree}) &&
> +       git -C server tag exact \
> +               $(git -C server commit-tree -m a "$TREE") &&
> +       git -C server tag dwim-unwanted \
> +               $(git -C server commit-tree -m b "$TREE") &&
> +       git -C server tag exact-unwanted \
> +               $(git -C server commit-tree -m c "$TREE") &&
> +       git -C server tag prefix1 \
> +               $(git -C server commit-tree -m d "$TREE") &&
> +       git -C server tag prefix2 \
> +               $(git -C server commit-tree -m e "$TREE") &&
> +       git -C server tag fetch-by-sha1 \
> +               $(git -C server commit-tree -m f "$TREE") &&
> +       git -C server tag completely-unrelated \
> +               $(git -C server commit-tree -m g "$TREE") &&

So you create different commits all with the same tree; no parents;
this seems to convey nicely that this is about testing tags.

> +       git init client &&
> +       GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \
> +               fetch "file://$(pwd)/server" \
> +               dwim \
> +               refs/tags/exact \
> +               refs/tags/prefix*:refs/tags/prefix* \
> +               "$(git -C server rev-parse fetch-by-sha1)" &&
> +
> +       # Ensure that the appropriate prefixes are sent (using a sample)
> +       grep "fetch> ref-prefix dwim" trace &&
> +       grep "fetch> ref-prefix refs/heads/dwim" trace &&
> +       grep "fetch> ref-prefix refs/tags/prefix" trace &&
> +
> +       # Ensure that the correct objects are returned
> +       git -C client cat-file -e $(git -C server rev-parse dwim) &&
> +       git -C client cat-file -e $(git -C server rev-parse exact) &&
> +       git -C client cat-file -e $(git -C server rev-parse prefix1) &&
> +       git -C client cat-file -e $(git -C server rev-parse prefix2) &&
> +       git -C client cat-file -e $(git -C server rev-parse fetch-by-sha1) &&
> +       test_must_fail git -C client cat-file -e \
> +               $(git -C server rev-parse dwim-unwanted) &&
> +       test_must_fail git -C client cat-file -e \
> +               $(git -C server rev-parse exact-unwanted) &&
> +       test_must_fail git -C client cat-file -e \
> +               $(git -C server rev-parse completely-unrelated)
> +'

This test is precise and easy to understand; the patch is
Reviewed-by: Stefan Beller <sbeller@google.com>
(considering the test_when_finished comment as
an optional nit)