| Message ID | patch-01.10-c8040da8e55-20211106T214259Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cat-file: better usage UX & error messages | expand |
On Sat, Nov 6, 2021 at 5:47 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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@gmail.com> > --- > 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...
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 658628375c8..0ad00e1fe73 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -4,6 +4,96 @@ test_description='git cat-file' . ./test-lib.sh +test_cmdmode_usage() { + 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 +' + +test_incompatible_usage() { + test_expect_code 129 "$@" 2>err && + grep -E "^error:.*$switch.*needs" err +} + +for opt in --batch --batch-check +do + test_expect_success "usage: incompatible options: --path with $opt" ' + test_incompatible_usage git cat-file --path=foo $opt + ' +done + +short_modes="-e -p -t -s" +cw_modes="--textconv --filters" + +for opt in $cw_modes +do + test_expect_success "usage: $opt requires another option" ' + test_expect_code 129 git cat-file $opt + ' + + test_expect_failure "usage: incompatible options: --batch-all-objects with $opt" ' + test_incompatible_usage git cat-file --batch-all-objects $opt + ' +done + +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 + +for opt in $short_modes $cw_modes +do + args="one two three" + test_expect_success "usage: too many arguments: $opt $args" ' + test_expect_code 129 git cat-file $opt $args + ' + + for opt2 in --buffer --follow-symlinks + do + test_expect_success "usage: incompatible arguments: $opt with batch option $opt2" ' + test_expect_code 129 git cat-file $opt $opt2 + ' + done +done + +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 + echo_without_newline () { printf '%s' "$*" }
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@gmail.com> --- t/t1006-cat-file.sh | 90 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+)