On Sat, Nov 6, 2021 at 5:47 PM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote:
> Stress test the usage emitted when options are combined in ways that
> isn't supported. Let's test various option combinations, some of these
> we buggily allow right now.
>
> E.g. this reveals a bug in 321459439e1 (cat-file: support
> --textconv/--filters in batch mode, 2016-09-09) that we'll fix in a
> subsequent commit. We're supposed to be emitting a relevant message
> when --batch-all-objects is combined with --textconv or --filters, but
> we don't.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
> @@ -4,6 +4,96 @@ test_description='git cat-file'
> +test_cmdmode_usage() {
Style nit: add space before ()
> + test_expect_code 129 "$@" 2>err &&
> + grep "^error:.*is incompatible with" err
> +}
> +
> +test_expect_success 'usage: cmdmode' '
> + test_cmdmode_usage git cat-file -e -p &&
> + test_cmdmode_usage git cat-file -p -t &&
> + test_cmdmode_usage git cat-file -t -s &&
> + test_cmdmode_usage git cat-file -s --textconv &&
> + test_cmdmode_usage git cat-file --textconv --filters
> +'
A minor observation: I usually avoid combining tests into a
conglomerate since it makes it harder to discover at a glance
the problematic test if one does start failing. I'd probably have used a
separate test_expect_success() invocation for each allowed switch
combination (in other words, five distinct tests instead of all five
cases stuffed into a single test). Not a big deal.
> +test_incompatible_usage() {
Style nit: add space before ()
> + test_expect_code 129 "$@" 2>err &&
> + grep -E "^error:.*$switch.*needs" err
> +}
What is `$switch`? There doesn't seem to be any such variable defined
which means the regex is really:
^error:.*.*needs
thus matches "by accident".
> +for opt in $short_modes
> +do
> + test_expect_success "usage: $opt requires another option" '
> + test_expect_code 129 git cat-file $opt
> + '
> +
> + for opt2 in --batch \
> + --batch-check \
> + --follow-symlinks
> + do
> + test_expect_failure "usage: incompatible options: $opt and $opt2" '
> + test_incompatible_usage git cat-file $opt $opt2
> + '
> + done
> +
> + opt2="--path=foo HEAD:some-path.txt"
> + test_expect_success "usage: incompatible options: $opt and $opt2" '
> + test_incompatible_usage git cat-file $opt $opt2
> + '
> +done
So, the only reason the final `opt2` is not part of the for-loop:
for opt2 in --batch \
--batch-check \
--follow-symlinks \
"--path=foo HEAD:some-path.txt"
is that it succeeds but the others fail?
> +for opt in --buffer \
> + --follow-symlinks \
> + --batch-all-objects
> +do
> + status=success
> + if test $opt = "--buffer"
> + then
> + status=failure
> + fi
> + test_expect_$status "usage: bad option combination: $opt without batch mode" '
> + test_expect_code 129 git cat-file $opt &&
> + test_expect_code 129 git cat-file $opt commit HEAD
> + '
> +done
In this case, `status` differentiates between success and failure...
