Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> Add the missing tests for the --bool-or-str code added in
> dbd8c09bfe (mergetool: allow auto-merge for meld to follow the
> vim-diff behavior, 2020-05-07).
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
> t/t1300-config.sh | 72 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 72 insertions(+)
>
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index e0dd5d65ce..a002ec5644 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -802,6 +802,78 @@ test_expect_success 'get --bool-or-int' '
> test_cmp expect actual
> '
>
> +test_expect_success 'get --bool-or-str' '
> + cat >.git/config <<-\EOF &&
> + [bool]
> + true1
> + true2 = true
> + true3 = TRUE
> + true4 = yes
> + true5 = YES
> + true6 = on
> + true7 = ON
> + false1 =
> + false2 = false
> + false3 = FALSE
> + false4 = no
> + false5 = NO
> + false6 = off
> + false7 = OFF
> + [int]
> + int1 = 0
> + int2 = 1
> + int3 = -1
> + [string]
> + string1 = hello
> + string2 = there you
> + EOF
That's fairly complete set (but misses common permutations like
"Yes"). I am not sure, if we try "true" and "TRUE", if it is worth
to check yes/YES and others, but at the same time, I do not know if
reducing the permutations tested would improve the readability,
runtime and/or maintainability of the test.
> + cat >expect <<-\EOF &&
> + true
> + true
> + true
> + true
> + true
> + true
> + true
> + false
> + false
> + false
> + false
> + false
> + false
> + false
> + false
> + false
> + true
> + true
> + hello
> + there you
> + EOF
The "right answer" is hard to read and maintain. Can we immediately
spot what happened to int.int3 in this output, for example?
Perhaps with something like
inspect_config () {
name=$1
shift
printf "%s %s\n" $(git config "$@" "$name") "$name"
}
we can make these lines to say
int.int1 false
int.int2 true
int.int3 true
string.string1 hello
string.string2 there you
to make them easier to follow? Without such a hint, I would expect
that a failure output from test_cmp at the end would be very hard to
grok, especially with so many permutations tested that produces runs
of "true" and "false".
> + {
> + git config --type=bool-or-str bool.true1 &&
> + git config --bool-or-str bool.true2 &&
This is a bit curious. We do not do full permutation between
--type=bool-or-str and --bool-or-str here. We just check both
would work only once. Feels a bit inconsistent.
My gut-feeling vote is to just try true/TRUE for case insensitivity
and try all the other variants in lowercase, but I can go with the
full permutation if you strongly prefer it.
> ...
> + git config --bool-or-str int.int1 &&
> + git config --bool-or-str int.int2 &&
> + git config --bool-or-str int.int3 &&
> + git config --bool-or-str string.string1 &&
> + git config --bool-or-str string.string2
> + } >actual &&
> + test_cmp expect actual
> +'
Thanks.
