Re: [PATCH 4/5] test-tool: test refspec input/output

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Apr 5, 2021, 17:52 UTC
Message-ID: <CAPig+cTCeED-Q_64+S5MDVFjCJdnCqYXWNS2ZDWKm4Ehjn=VXw@mail.gmail.com>
In-Reply-To: <c8d1de06f84499f2f56b3a06df665630806f94ce.1617627856.git.gitgitgadget@gmail.com>

On Mon, Apr 5, 2021 at 9:04 AM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 14 quoted lines

> Add a new test-helper, 'test-tool refspec', that currently reads stdin
> line-by-line and translates the refspecs using the parsing logic of
> refspec_item_init() and writes them to output.
> [...]
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
> diff --git a/t/helper/test-refspec.c b/t/helper/test-refspec.c
> @@ -0,0 +1,39 @@
> +int cmd__refspec(int argc, const char **argv)
> +{
> +       struct strbuf line = STRBUF_INIT;
> +       [...]
> +       return 0;
> +}

Leaking `strbuf line` here. Yes, I realize that the function is returning and test-tool exiting immediately after this, so not a big deal, but it's easy to do this correctly by releasing the strbuf, thus setting good precedence for people who might use this as a template for new test-tool functions they add in the future.

Show 20 quoted lines

> diff --git a/t/t5511-refspec.sh b/t/t5511-refspec.sh
> @@ -93,4 +93,45 @@ test_refspec fetch "refs/heads/${good}"
> +test_expect_success 'test input/output round trip' '
> +       cat >input <<-\EOF &&
> +               +refs/heads/*:refs/remotes/origin/*
> +               refs/heads/*:refs/remotes/origin/*
> +               refs/heads/main:refs/remotes/frotz/xyzzy
> +               :refs/remotes/frotz/deleteme
> +               ^refs/heads/secrets
> +               refs/heads/secret:refs/heads/translated
> +               refs/heads/secret:heads/translated
> +               refs/heads/secret:remotes/translated
> +               secret:translated
> +               refs/heads/*:remotes/xxy/*
> +               refs/heads*/for-linus:refs/remotes/mine/*
> +               2e36527f23b7f6ae15e6f21ac3b08bf3fed6ee48:refs/heads/fixed
> +               HEAD
> +               @
> +               :
> +       EOF

Over-indented heredoc body. It is customary[1] in this codebase for the body and EOF to have the same indentation as the command which starts the heredoc.

Show 17 quoted lines

> +       cat >expect <<-\EOF &&
> +               +refs/heads/*:refs/remotes/origin/*
> +               refs/heads/*:refs/remotes/origin/*
> +               refs/heads/main:refs/remotes/frotz/xyzzy
> +               :refs/remotes/frotz/deleteme
> +               ^refs/heads/secrets
> +               refs/heads/secret:refs/heads/translated
> +               refs/heads/secret:heads/translated
> +               refs/heads/secret:remotes/translated
> +               secret:translated
> +               refs/heads/*:remotes/xxy/*
> +               refs/heads*/for-linus:refs/remotes/mine/*
> +               2e36527f23b7f6ae15e6f21ac3b08bf3fed6ee48:refs/heads/fixed
> +               HEAD
> +               HEAD
> +               :
> +       EOF

Ditto.

Show 5 quoted lines

> +       test-tool refspec <input >output &&
> +       test_cmp expect output &&
> +       test-tool refspec --fetch <input >output &&
> +       test_cmp expect output
> +'

[1]: https://lore.kernel.org/git/CAPig+cSBVG0AdyqXH2mZp6Ohrcb8_ec1Mm_vGbQM4zWT_7yYxQ@mail.gmail.com/