Skip to content

Commit

Permalink
tests: disallow the use of abbreviated options (by default)
Browse files Browse the repository at this point in the history
Git's command-line parsers support uniquely abbreviated options, e.g.
`git init --ba` would automatically expand `--ba` to `--bare`.

This is a very convenient feature in every day life for Git users, in
particular when tab completion is not available.

However, it is not a good idea to rely on that in Git's test suite, as
something that is a unique abbreviation of a command line option today
might no longer be a unique abbreviation tomorrow.

For example, if a future contribution added a new mode
`git init --babyproofing` and a previously-introduced test case used the
fact that `git init --ba` expanded to `git init --bare`, that future
contribution would now have to touch seemingly unrelated tests just to
keep the test suite from failing.

So let's disallow abbreviated options in the test suite by default.

Note: for ease of implementation, this patch really only touches the
`parse-options` machinery: more and more hand-rolled option parsers are
converted to use that internal API, and more and more scripts are
converted to built-ins (naturally using the parse-options API, too), so
in practice this catches most issues, and is definitely the biggest bang
for the buck.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
dscho authored and gitster committed Apr 15, 2019
1 parent effc2ba commit b02e7d5
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 1 deletion.
9 changes: 9 additions & 0 deletions parse-options.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include "color.h"
#include "utf8.h"

static int disallow_abbreviated_options;

#define OPT_SHORT 1
#define OPT_UNSET 2

Expand Down Expand Up @@ -344,6 +346,10 @@ static enum parse_opt_result parse_long_opt(
return get_value(p, options, all_opts, flags ^ opt_flags);
}

if (disallow_abbreviated_options && (ambiguous_option || abbrev_option))
die("disallowed abbreviated or ambiguous option '%.*s'",
(int)(arg_end - arg), arg);

if (ambiguous_option) {
error(_("ambiguous option: %s "
"(could be --%s%s or --%s%s)"),
Expand Down Expand Up @@ -708,6 +714,9 @@ int parse_options(int argc, const char **argv, const char *prefix,
{
struct parse_opt_ctx_t ctx;

disallow_abbreviated_options =
git_env_bool("GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS", 0);

parse_options_start(&ctx, argc, argv, prefix, options, flags);
switch (parse_options_step(&ctx, options, usagestr)) {
case PARSE_OPT_HELP:
Expand Down
4 changes: 4 additions & 0 deletions t/README
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,10 @@ GIT_TEST_SIDEBAND_ALL=<boolean>, when true, overrides the
fetch-pack to not request sideband-all (even if the server advertises
sideband-all).

GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=<boolean>, when true (which is
the default when running tests), errors out when an abbreviated option
is used.

Naming Tests
------------

Expand Down
14 changes: 13 additions & 1 deletion t/t0040-parse-options.sh
Original file line number Diff line number Diff line change
Expand Up @@ -203,20 +203,24 @@ file: (not set)
EOF

test_expect_success 'unambiguously abbreviated option' '
GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
test-tool parse-options --int 2 --boolean --no-bo >output 2>output.err &&
test_must_be_empty output.err &&
test_cmp expect output
'

test_expect_success 'unambiguously abbreviated option with "="' '
GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
test-tool parse-options --expect="integer: 2" --int=2
'

test_expect_success 'ambiguously abbreviated option' '
test_expect_code 129 test-tool parse-options --strin 123
test_expect_code 129 env GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
test-tool parse-options --strin 123
'

test_expect_success 'non ambiguous option (after two options it abbreviates)' '
GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
test-tool parse-options --expect="string: 123" --st 123
'

Expand Down Expand Up @@ -325,6 +329,7 @@ file: (not set)
EOF

test_expect_success 'negation of OPT_NONEG flags is not ambiguous' '
GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
test-tool parse-options --no-ambig >output 2>output.err &&
test_must_be_empty output.err &&
test_cmp expect output
Expand Down Expand Up @@ -370,4 +375,11 @@ test_expect_success '--no-verbose resets multiple verbose to 0' '
test-tool parse-options --expect="verbose: 0" -v -v -v --no-verbose
'

test_expect_success 'GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS works' '
GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
test-tool parse-options --ye &&
test_must_fail env GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=true \
test-tool parse-options --ye
'

test_done
7 changes: 7 additions & 0 deletions t/test-lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ fi
. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
export PERL_PATH SHELL_PATH

# Disallow the use of abbreviated options in the test suite by default
if test -z "${GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS}"
then
GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=true
export GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS
fi

################################################################
# It appears that people try to run tests without building...
"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X" >/dev/null
Expand Down

0 comments on commit b02e7d5

Please sign in to comment.