On Fri, Jun 21, 2019 at 05:31:04PM -0500, Felipe Contreras wrote:
> There's a regression in the completion since the introduction of
> __gitcomp.
>
> Go to any directory that doesn't contain a git repository, like /tmp.
> Then type the following:
>
> git checkout --<tab>
>
> You will see nothing. That's because
> `git checkout --git-completion-helper` fails when you run it outside a
> git repository.
>
> You might change to a directory that has a git repository, but it's too
> late, because the empty options have been cached.
This will get outdated rather soonish, as soon as 69702523af
(completion: do not cache if --git-completion-helper fails,
2019-06-12) graduates to master.
> It's unclear how many commands are affected, but this patch attempts to
> at least detect some already in the testing framework.
It seems that several changes in this patch modify tests in a way that
defeats the purpose of the given test, e.g. the tests
'completion.commands removes multiple commands' or 'sourcing the
completion script clears cached merge strategies'
I would rather see tests specifically focusing on the
__gitcomp_builtin() helper function, including test cases when it's
excersized outside of a repository and when it gets additional
parameters to include and exclude some options.
> Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> ---
> t/t9902-completion.sh | 37 ++++++++++++++++++++++++++++---------
> 1 file changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 43cf313a1c..7bef41eaf5 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -122,6 +122,15 @@ test_gitcomp_nl ()
> test_cmp expected out
> }
>
> +offgit ()
> +{
> + GIT_CEILING_DIRECTORIES="$ROOT" &&
> + export GIT_CEILING_DIRECTORIES &&
> + test_when_finished "ROOT='$ROOT'; cd '$TRASH_DIRECTORY'; unset GIT_CEILING_DIRECTORIES" &&
> + ROOT="$ROOT"/non-repo &&
> + cd "$ROOT"
I think fiddling with $ROOT is unnecessary here.
> +}
> +
> invalid_variable_name='${foo.bar}'
>
> actual="$TRASH_DIRECTORY/actual"
> @@ -358,10 +367,8 @@ test_expect_success SYMLINKS '__git_find_repo_path - resulting path avoids symli
> '
>
> test_expect_success '__git_find_repo_path - not a git repository' '
> + offgit &&
> (
> - cd non-repo &&
> - GIT_CEILING_DIRECTORIES="$ROOT" &&
> - export GIT_CEILING_DIRECTORIES &&
> test_must_fail __git_find_repo_path &&
> printf "$__git_repo_path" >"$actual"
> ) &&
> @@ -1388,6 +1395,7 @@ test_expect_success '__git_pretty_aliases' '
> '
>
> test_expect_success 'basic' '
> + offgit &&
> run_completion "git " &&
> # built-in
> grep -q "^add \$" out &&
> @@ -1401,6 +1409,7 @@ test_expect_success 'basic' '
> '
>
> test_expect_success 'double dash "git" itself' '
> + offgit &&
> test_completion "git --" <<-\EOF
> --paginate Z
> --no-pager Z
> @@ -1419,7 +1428,8 @@ test_expect_success 'double dash "git" itself' '
> EOF
> '
>
> -test_expect_success 'double dash "git checkout"' '
> +test_expect_failure 'double dash "git checkout"' '
> + offgit &&
> test_completion "git checkout --" <<-\EOF
> --quiet Z
> --detach Z
> @@ -1442,6 +1452,7 @@ test_expect_success 'double dash "git checkout"' '
> '
>
> test_expect_success 'general options' '
> + offgit &&
> test_completion "git --ver" "--version " &&
> test_completion "git --hel" "--help " &&
> test_completion "git --exe" <<-\EOF &&
> @@ -1460,6 +1471,7 @@ test_expect_success 'general options' '
> '
>
> test_expect_success 'general options plus command' '
> + offgit &&
> test_completion "git --version check" "checkout " &&
> test_completion "git --paginate check" "checkout " &&
> test_completion "git --git-dir=foo check" "checkout " &&
> @@ -1480,11 +1492,13 @@ test_expect_success 'general options plus command' '
> '
>
> test_expect_success 'git --help completion' '
> + offgit &&
> test_completion "git --help ad" "add " &&
> test_completion "git --help core" "core-tutorial "
> '
>
> -test_expect_success 'completion.commands removes multiple commands' '
> +test_expect_failure 'completion.commands removes multiple commands' '
> + offgit &&
> test_config completion.commands "-cherry -mergetool" &&
> git --list-cmds=list-mainporcelain,list-complete,config >out &&
> ! grep -E "^(cherry|mergetool)$" out
> @@ -1547,9 +1561,10 @@ test_expect_success 'complete tree filename with metacharacters' '
> EOF
> '
>
> -test_expect_success PERL 'send-email' '
> - test_completion "git send-email --cov" "--cover-letter " &&
> - test_completion "git send-email ma" "master "
> +test_expect_failure PERL 'send-email' '
> + test_completion "git send-email ma" "master " &&
> + offgit &&
> + test_completion "git send-email --cov" "--cover-letter "
> '
>
> test_expect_success 'complete files' '
> @@ -1649,6 +1664,7 @@ test_expect_success 'completion used <cmd> completion for alias: !f() { : git <c
> '
>
> test_expect_success 'completion without explicit _git_xxx function' '
> + offgit &&
> test_completion "git version --" <<-\EOF
> --build-options Z
> --no-build-options Z
> @@ -1699,13 +1715,15 @@ do
> done
>
> test_expect_success 'sourcing the completion script clears cached commands' '
> + offgit &&
> __git_compute_all_commands &&
> verbose test -n "$__git_all_commands" &&
> . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&
> verbose test -z "$__git_all_commands"
> '
>
> -test_expect_success 'sourcing the completion script clears cached merge strategies' '
> +test_expect_failure 'sourcing the completion script clears cached merge strategies' '
> + offgit &&
> GIT_TEST_GETTEXT_POISON= &&
> __git_compute_merge_strategies &&
> verbose test -n "$__git_merge_strategies" &&
> @@ -1714,6 +1732,7 @@ test_expect_success 'sourcing the completion script clears cached merge strategi
> '
>
> test_expect_success 'sourcing the completion script clears cached --options' '
> + offgit &&
> __gitcomp_builtin checkout &&
> verbose test -n "$__gitcomp_builtin_checkout" &&
> __gitcomp_builtin notes_edit &&
> --
> 2.22.0
>
