Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
> +offgit ()
> +{
Style: opening brace comes on the same line, like
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"
> +}
All of these means that anytime some test uses offgit outside a
subshell, all the subsequent test will start from outside a
repository, with nonstandard GIT_CEILING_DIRECTORIES settings.
The test should avoid using this outside a subshell when able (and
if it apparently cannot easily, we should try to find a way).
> @@ -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 " &&
Adding "offgit" everywhere like this patch does means that this
"basic" test, for example, no longer is performed in the condition
we have been testing the completion script for, doesn't it? If so,
the patch is trading test coverage outside repo with coverage inside
repo, which is not a very good tradeoff.
