Re: [PATCH 01/10] cat-file tests: test bad usage

On Sat, Nov 6, 2021 at 5:47 PM Ævar Arnfjörð Bjarmason [off-list ref] wrote:

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...