Re: [PATCH v2 1/2] t1300: extract and use test_cmp_config()

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 30, 2018, 04:05 UTC
Message-ID: <CAPig+cSTvewR51Vj-4MKGJKVzqFMhf4nz5yy_PdFP+EF6qMKrA@mail.gmail.com>
In-Reply-To: <20180929153005.10599-2-pclouds@gmail.com>

On Sat, Sep 29, 2018 at 11:30 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:

Show 11 quoted lines

> In many config-related tests it's common to check if a config variable
> has expected value and we want to print the differences when the test
> fails. Doing it the normal way is three lines of shell code. Let's add
> a function do to all this (and a little more).
> [...]
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> @@ -747,6 +747,30 @@ test_cmp() {
> +# similar to test_cmp but $2 is a config key instead of actual value
> +# it can also accept -C to read from a different repo, e.g.

Minor: maybe say that "-C <dir>" changes to <dir> for the git-config invocation.

Show 5 quoted lines

> +#     test_cmp_config -C xyz foo core.bar
> +#
> +# is sort of equivalent of
> +#
> +#     test "foo" = "$(git -C xyz core.bar)"

Should be: $(git -C xyz config core.bar)

> +test_cmp_config() {
> +       if [ "$1" = "-C" ]
> +       then

Style:

    if test "$1" = "-C"
    then
        ...

Show 7 quoted lines

> +               shift &&
> +               GD="-C $1" &&
> +               shift
> +       else
> +               GD=
> +       fi &&
> +       echo "$1" >expected &&

If $1 starts with a hyphen, 'echo' might try interpreting it as an option. Use printf instead:

    printf "%s\n" "$1" &&

> +       shift &&
> +       git $GD config "$@" >actual &&
> +       test_cmp expected actual

Please choose names other than "actual" and "expected" since those are likely to clobber files of the same name which the test has set up itself. (Or, at the very least, document that this function clobbers those files -- but using better filenames is preferable.)

> +}