On Sat, Sep 29, 2018 at 05:30:04PM +0200, Nguyễn Thái Ngọc Duy wrote:
> 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).
>
> This function has uses outside t1300 as well but I'm not going to
> convert them all. And it will be used in the next commit where
> per-worktree config feature is introduced.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <[email protected]>
> ---
> t/t1300-config.sh | 79 ++++++++++-------------------------------
> t/test-lib-functions.sh | 24 +++++++++++++
> 2 files changed, 43 insertions(+), 60 deletions(-)
>
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index cdf1fed5d1..00c2b0f0eb 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -76,15 +76,11 @@ EOF
> test_expect_success 'unset type specifiers may be reset to conflicting ones'
> '
> - echo 1048576 >expect &&
> - git config --type=bool --no-type --type=int core.big >actual &&
> - test_cmp expect actual
> + test_cmp_config 1048576 --type=bool --no-type --type=int core.big
> '
>
> test_expect_success '--type rejects unknown specifiers' '
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index d82fac9d79..4cd7fb8fdf 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -747,6 +747,30 @@ test_cmp() {
> $GIT_TEST_CMP "$@"
> }
>
> +# similar to test_cmp but $2 is a config key instead of actual value
This doesn't describe very well the function's parameters: $1
specifies the expected value (as opposed to the file containing the
expected value), and the rest can be all kinds of 'git config'
options, not just a second argument with the config key; e.g. see call
in the above hunk.
Perheps only a brief description and a usage string like this would
sufficiently cover everything?
# Check that the given config key has the expected value.
#
# test_cmp_config [-C <dir>] <expected-value>
# [<git-config-options>...] <config-key>
> +# it can also accept -C to read from a different repo, e.g.
> +#
> +# test_cmp_config -C xyz foo core.bar
> +#
> +# is sort of equivalent of
I think this comment should outright say that "is a better alternative
to", because it provides useful output on failure, and because it
doesn't hide 'git config's exit code.
Note that this second point does make a bit of a difference:
test_cmp_config "" nonexisting.variable
would fail, because
$ git config nonexisting.variable ; echo $?
1
and the &&-chain. However,
test "" = "$(git config nonexisting.variable)"
would still succeed, because the non-zero exit code is ignored.
I consider this a benefit, as it will protect us from a typo in the
name of a set but empty variable, even though we won't get any error
message.
> +#
> +# test "foo" = "$(git -C xyz core.bar)"
> +
Nit: unnecessary empty line.
> +test_cmp_config() {
> + if [ "$1" = "-C" ]
> + then
> + shift &&
> + GD="-C $1" &&
> + shift
> + else
> + GD=
> + fi &&
I think you could now safely declare GD as local. The test balloon
01d3a526ad (t0000: check whether the shell supports the "local"
keyword, 2017-10-26) has been out there for 4 releases / almost a
year, and we haven't heard about any issues, and the upcoming hash
translation test helpers use 'local' as well.
> + echo "$1" >expected &&
> + shift &&
> + git $GD config "$@" >actual &&
> + test_cmp expected actual
> +}
> +
> # test_cmp_bin - helper to compare binary files
>
> test_cmp_bin() {
> --
> 2.19.0.341.g3acb95d729
>
