From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: [PATCH 11/14] test: completion: tests for __gitcomp regression
Date: Wed, 3 Jul 2019 19:49:09 +0200 [thread overview]
Message-ID: <20190703174909.GU21574@szeder.dev> (raw)
In-Reply-To: <20190621223107.8022-12-felipe.contreras@gmail.com>
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@gmail.com>
> ---
> 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
>
next prev parent reply other threads:[~2019-07-03 17:49 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
2019-06-21 22:30 ` [PATCH 01/14] completion: zsh: fix __gitcomp_direct() Felipe Contreras
2019-06-22 15:03 ` Felipe Contreras
2019-06-21 22:30 ` [PATCH 02/14] completion: zsh: fix for directories with spaces Felipe Contreras
2019-06-21 22:30 ` [PATCH 03/14] completion: remove zsh hack Felipe Contreras
2019-06-21 22:30 ` [PATCH 04/14] completion: zsh: improve main function selection Felipe Contreras
2019-06-21 22:30 ` [PATCH 05/14] completion: prompt: fix color for Zsh Felipe Contreras
2019-06-21 22:30 ` [PATCH 06/14] completion: bash: cleanup cygwin check Felipe Contreras
2019-06-21 22:31 ` [PATCH 07/14] completion: zsh: update installation instructions Felipe Contreras
2019-06-21 22:31 ` [PATCH 08/14] completion: bash: remove old compat wrappers Felipe Contreras
2019-06-21 22:31 ` [PATCH 09/14] completion: bash: remove zsh wrapper Felipe Contreras
2019-06-21 22:31 ` [PATCH 10/14] completion: zsh: trivial cleanups Felipe Contreras
2019-06-21 22:31 ` [PATCH 11/14] test: completion: tests for __gitcomp regression Felipe Contreras
2019-07-03 17:38 ` Junio C Hamano
2019-07-03 17:49 ` SZEDER Gábor [this message]
2019-06-21 22:31 ` [PATCH 12/14] test: completion: use global config Felipe Contreras
2019-07-03 17:22 ` Junio C Hamano
2019-06-21 22:31 ` [PATCH 13/14] completion: add default options Felipe Contreras
2019-06-22 3:01 ` Duy Nguyen
2019-06-22 4:36 ` Felipe Contreras
2019-06-24 17:22 ` Junio C Hamano
2019-06-25 1:38 ` Felipe Contreras
2019-06-25 3:32 ` Duy Nguyen
2019-06-21 22:31 ` [PATCH 14/14] completion: add default merge strategies Felipe Contreras
2019-06-24 17:23 ` Junio C Hamano
2019-06-25 1:11 ` Felipe Contreras
2019-06-25 1:43 ` Junio C Hamano
2019-07-03 17:14 ` SZEDER Gábor
2019-07-03 17:50 ` [PATCH 00/14] completion: a bunch of updates Junio C Hamano
2019-07-03 19:06 ` SZEDER Gábor
2020-10-25 3:51 ` Felipe Contreras
2020-10-25 3:46 ` Felipe Contreras
2020-10-27 20:23 ` Junio C Hamano
2020-10-27 22:19 ` Felipe Contreras
2020-10-27 23:32 ` Junio C Hamano
2020-10-28 0:06 ` Felipe Contreras
2020-10-28 9:09 ` Stefan Haller
2020-10-28 16:31 ` Felipe Contreras
2020-10-28 17:34 ` Stefan Haller
2020-10-29 17:16 ` Junio C Hamano
2020-10-29 17:27 ` Junio C Hamano
2020-11-02 20:18 ` Felipe Contreras
2020-11-03 1:49 ` Junio C Hamano
2020-11-04 0:09 ` Felipe Contreras
2020-11-04 18:08 ` Junio C Hamano
2020-11-05 1:09 ` Felipe Contreras
2020-11-05 22:09 ` Junio C Hamano
2020-10-30 8:01 ` Stefan Haller
2020-10-30 17:19 ` Junio C Hamano
2020-11-02 20:29 ` Felipe Contreras
2020-11-02 23:05 ` Aaron Schrab
2020-11-03 1:35 ` Junio C Hamano
2020-11-03 23:46 ` Felipe Contreras
2020-11-03 22:37 ` Felipe Contreras
2020-11-03 9:59 ` Stefan Haller
2020-11-03 17:12 ` Junio C Hamano
2020-11-03 20:07 ` Stefan Haller
2020-11-02 19:18 ` Felipe Contreras
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190703174909.GU21574@szeder.dev \
--to=szeder.dev@gmail.com \
--cc=felipe.contreras@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=pclouds@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).