* [PATCH 01/12] t/Makefile: stop setting GIT_CONFIG
2014-03-20 23:11 ` [PATCH 0/12] GIT_CONFIG in the test suite Jeff King
@ 2014-03-20 23:13 ` Jeff King
2014-03-20 23:13 ` [PATCH 02/12] t/test-lib: drop redundant unset of GIT_CONFIG Jeff King
` (10 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Jeff King @ 2014-03-20 23:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Tran, git
Once upon a time, the setting of GIT_CONFIG in the
environment could affect how tests ran. Commit 9c3796f (Fix
setting config variables with an alternative GIT_CONFIG,
2006-06-20) unconditionally set GIT_CONFIG in the Makefile
when running tests to give us a known starting point.
This is insufficient for running the tests outside of the
Makefile, however, and 8565d2d (Make tests independent of
global config files, 2007-02-15) later set GIT_CONFIG
directly in test-lib.sh. At that point the Makefile setting
was redundant, but we never removed it. Let's do so now.
Signed-off-by: Jeff King <peff@peff.net>
---
t/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/Makefile b/t/Makefile
index 2373a04..8fd1a72 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -36,11 +36,11 @@ test: pre-clean $(TEST_LINT)
$(MAKE) aggregate-results-and-cleanup
prove: pre-clean $(TEST_LINT)
- @echo "*** prove ***"; GIT_CONFIG=.git/config $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
+ @echo "*** prove ***"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
$(MAKE) clean-except-prove-cache
$(T):
- @echo "*** $@ ***"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
+ @echo "*** $@ ***"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
pre-clean:
$(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)'
--
1.9.0.560.g01ceb46
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 02/12] t/test-lib: drop redundant unset of GIT_CONFIG
2014-03-20 23:11 ` [PATCH 0/12] GIT_CONFIG in the test suite Jeff King
2014-03-20 23:13 ` [PATCH 01/12] t/Makefile: stop setting GIT_CONFIG Jeff King
@ 2014-03-20 23:13 ` Jeff King
2014-03-20 23:14 ` [PATCH 03/12] t: drop useless sane_unset GIT_* calls Jeff King
` (9 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Jeff King @ 2014-03-20 23:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Tran, git
This is already handled by the mass GIT_* unsetting added by
95a1d12 (tests: scrub environment of GIT_* variables,
2011-03-15).
Signed-off-by: Jeff King <peff@peff.net>
---
t/test-lib.sh | 1 -
1 file changed, 1 deletion(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..625f06e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -649,7 +649,6 @@ else # normal case, use ../bin-wrappers only unless $with_dashes:
fi
fi
GIT_TEMPLATE_DIR="$GIT_BUILD_DIR"/templates/blt
-unset GIT_CONFIG
GIT_CONFIG_NOSYSTEM=1
GIT_ATTR_NOSYSTEM=1
export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_ATTR_NOSYSTEM
--
1.9.0.560.g01ceb46
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 03/12] t: drop useless sane_unset GIT_* calls
2014-03-20 23:11 ` [PATCH 0/12] GIT_CONFIG in the test suite Jeff King
2014-03-20 23:13 ` [PATCH 01/12] t/Makefile: stop setting GIT_CONFIG Jeff King
2014-03-20 23:13 ` [PATCH 02/12] t/test-lib: drop redundant unset of GIT_CONFIG Jeff King
@ 2014-03-20 23:14 ` Jeff King
2014-03-21 21:24 ` Junio C Hamano
2014-03-20 23:15 ` [PATCH 04/12] t: stop using GIT_CONFIG to cross repo boundaries Jeff King
` (8 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: Jeff King @ 2014-03-20 23:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Tran, git
Several test scripts manually unset GIT_CONFIG and other
GIT_* variables. These are generally taken care of for us by
test-lib.sh already.
Unsetting these is not only useless, but can be confusing to
a reader, who may wonder why some tests in a script unset
them and others do not (t0001 is particularly guilty of this
inconsistency, probably because many of its tests predate
the test-lib.sh environment-cleansing).
Note that we cannot always get rid of such unsetting. For
example, t9130 can drop the GIT_CONFIG unset, but not the
GIT_DIR one, because lib-git-svn.sh sets the latter. And in
t1000, we unset GIT_TEMPLATE_DIR, which is explicitly
initialized by test-lib.sh.
Signed-off-by: Jeff King <peff@peff.net>
---
I suppose one could make an argument that test-lib.sh may later change
the set of variables it clears, and these unsets are documenting an
explicit need of each test. I'd find that more compelling if it were
actually applied consistently.
t/t0001-init.sh | 15 ---------------
t/t9130-git-svn-authors-file.sh | 1 -
t/t9400-git-cvsserver-server.sh | 1 -
3 files changed, 17 deletions(-)
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 9fb582b..ddc8160 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -25,7 +25,6 @@ check_config () {
test_expect_success 'plain' '
(
- sane_unset GIT_DIR GIT_WORK_TREE &&
mkdir plain &&
cd plain &&
git init
@@ -35,7 +34,6 @@ test_expect_success 'plain' '
test_expect_success 'plain nested in bare' '
(
- sane_unset GIT_DIR GIT_WORK_TREE &&
git init --bare bare-ancestor.git &&
cd bare-ancestor.git &&
mkdir plain-nested &&
@@ -47,7 +45,6 @@ test_expect_success 'plain nested in bare' '
test_expect_success 'plain through aliased command, outside any git repo' '
(
- sane_unset GIT_DIR GIT_WORK_TREE &&
HOME=$(pwd)/alias-config &&
export HOME &&
mkdir alias-config &&
@@ -65,7 +62,6 @@ test_expect_success 'plain through aliased command, outside any git repo' '
test_expect_failure 'plain nested through aliased command' '
(
- sane_unset GIT_DIR GIT_WORK_TREE &&
git init plain-ancestor-aliased &&
cd plain-ancestor-aliased &&
echo "[alias] aliasedinit = init" >>.git/config &&
@@ -78,7 +74,6 @@ test_expect_failure 'plain nested through aliased command' '
test_expect_failure 'plain nested in bare through aliased command' '
(
- sane_unset GIT_DIR GIT_WORK_TREE &&
git init --bare bare-ancestor-aliased.git &&
cd bare-ancestor-aliased.git &&
echo "[alias] aliasedinit = init" >>config &&
@@ -91,7 +86,6 @@ test_expect_failure 'plain nested in bare through aliased command' '
test_expect_success 'plain with GIT_WORK_TREE' '
if (
- sane_unset GIT_DIR &&
mkdir plain-wt &&
cd plain-wt &&
GIT_WORK_TREE=$(pwd) git init
@@ -104,7 +98,6 @@ test_expect_success 'plain with GIT_WORK_TREE' '
test_expect_success 'plain bare' '
(
- sane_unset GIT_DIR GIT_WORK_TREE GIT_CONFIG &&
mkdir plain-bare-1 &&
cd plain-bare-1 &&
git --bare init
@@ -114,7 +107,6 @@ test_expect_success 'plain bare' '
test_expect_success 'plain bare with GIT_WORK_TREE' '
if (
- sane_unset GIT_DIR GIT_CONFIG &&
mkdir plain-bare-2 &&
cd plain-bare-2 &&
GIT_WORK_TREE=$(pwd) git --bare init
@@ -128,7 +120,6 @@ test_expect_success 'plain bare with GIT_WORK_TREE' '
test_expect_success 'GIT_DIR bare' '
(
- sane_unset GIT_CONFIG &&
mkdir git-dir-bare.git &&
GIT_DIR=git-dir-bare.git git init
) &&
@@ -138,7 +129,6 @@ test_expect_success 'GIT_DIR bare' '
test_expect_success 'init --bare' '
(
- sane_unset GIT_DIR GIT_WORK_TREE GIT_CONFIG &&
mkdir init-bare.git &&
cd init-bare.git &&
git init --bare
@@ -149,7 +139,6 @@ test_expect_success 'init --bare' '
test_expect_success 'GIT_DIR non-bare' '
(
- sane_unset GIT_CONFIG &&
mkdir non-bare &&
cd non-bare &&
GIT_DIR=.git git init
@@ -160,7 +149,6 @@ test_expect_success 'GIT_DIR non-bare' '
test_expect_success 'GIT_DIR & GIT_WORK_TREE (1)' '
(
- sane_unset GIT_CONFIG &&
mkdir git-dir-wt-1.git &&
GIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-1.git git init
) &&
@@ -170,7 +158,6 @@ test_expect_success 'GIT_DIR & GIT_WORK_TREE (1)' '
test_expect_success 'GIT_DIR & GIT_WORK_TREE (2)' '
if (
- sane_unset GIT_CONFIG &&
mkdir git-dir-wt-2.git &&
GIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-2.git git --bare init
)
@@ -183,8 +170,6 @@ test_expect_success 'GIT_DIR & GIT_WORK_TREE (2)' '
test_expect_success 'reinit' '
(
- sane_unset GIT_CONFIG GIT_WORK_TREE GIT_CONFIG &&
-
mkdir again &&
cd again &&
git init >out1 2>err1 &&
diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh
index c3443ce..a812783 100755
--- a/t/t9130-git-svn-authors-file.sh
+++ b/t/t9130-git-svn-authors-file.sh
@@ -97,7 +97,6 @@ test_expect_success 'fresh clone with svn.authors-file in config' '
test x = x"$(git config svn.authorsfile)" &&
test_config="$HOME"/.gitconfig &&
sane_unset GIT_DIR &&
- sane_unset GIT_CONFIG &&
git config --global \
svn.authorsfile "$HOME"/svn-authors &&
test x"$HOME"/svn-authors = x"$(git config svn.authorsfile)" &&
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 3edc408..ed98e64 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -25,7 +25,6 @@ perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
test_done
}
-unset GIT_DIR GIT_CONFIG
WORKDIR=$(pwd)
SERVERDIR=$(pwd)/gitcvs.git
git_config="$SERVERDIR/config"
--
1.9.0.560.g01ceb46
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 03/12] t: drop useless sane_unset GIT_* calls
2014-03-20 23:14 ` [PATCH 03/12] t: drop useless sane_unset GIT_* calls Jeff King
@ 2014-03-21 21:24 ` Junio C Hamano
2014-03-24 21:56 ` Jeff King
0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2014-03-21 21:24 UTC (permalink / raw)
To: Jeff King; +Cc: David Tran, git
Jeff King <peff@peff.net> writes:
> Several test scripts manually unset GIT_CONFIG and other
> GIT_* variables. These are generally taken care of for us by
> test-lib.sh already.
>
> Unsetting these is not only useless, but can be confusing to
> a reader, who may wonder why some tests in a script unset
> them and others do not (t0001 is particularly guilty of this
> inconsistency, probably because many of its tests predate
> the test-lib.sh environment-cleansing).
> Note that we cannot always get rid of such unsetting. For
> example, t9130 can drop the GIT_CONFIG unset, but not the
> GIT_DIR one, because lib-git-svn.sh sets the latter. And in
> t1000, we unset GIT_TEMPLATE_DIR, which is explicitly
> initialized by test-lib.sh.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I suppose one could make an argument that test-lib.sh may later change
> the set of variables it clears, and these unsets are documenting an
> explicit need of each test. I'd find that more compelling if it were
> actually applied consistently.
Hmph. I am looking at "git show HEAD^:t/t0001-init.sh" after
applying this patch, and it does look consistently done with
GIT_CONFIG and GIT_DIR (I am not sure about GIT_WORK_TREE but from a
cursory read it is done consistently for tests on non-bare
repositories).
So I would actually agree with your alternative interpretation
"Unsetting these is useless, but it does serve documentation
purpose---without having to see what the state of the environment
when the subprocess is started, the reader can understand what is
being tested", rather than the one in the log message.
Having said that, I am perfectly OK with the change to t0001 in this
patch, if we added at the very beginning of the test sequence a
comment that says:
Below, creation and use of repositories are tested with various
combinations of environment settings and command line flags.
They are done inside subshells to avoid leaking temporary
environment settings to later tests *and* assumes that the
initial environment does not have have GIT_DIR, GIT_CONFIG, and
GIT_WORK_TREE defined.
or something.
> t/t0001-init.sh | 15 ---------------
> t/t9130-git-svn-authors-file.sh | 1 -
> t/t9400-git-cvsserver-server.sh | 1 -
> 3 files changed, 17 deletions(-)
>
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index 9fb582b..ddc8160 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -25,7 +25,6 @@ check_config () {
>
> test_expect_success 'plain' '
> (
> - sane_unset GIT_DIR GIT_WORK_TREE &&
> mkdir plain &&
> cd plain &&
> git init
> @@ -35,7 +34,6 @@ test_expect_success 'plain' '
>
> test_expect_success 'plain nested in bare' '
> (
> - sane_unset GIT_DIR GIT_WORK_TREE &&
> git init --bare bare-ancestor.git &&
> cd bare-ancestor.git &&
> mkdir plain-nested &&
> @@ -47,7 +45,6 @@ test_expect_success 'plain nested in bare' '
>
> test_expect_success 'plain through aliased command, outside any git repo' '
> (
> - sane_unset GIT_DIR GIT_WORK_TREE &&
> HOME=$(pwd)/alias-config &&
> export HOME &&
> mkdir alias-config &&
> @@ -65,7 +62,6 @@ test_expect_success 'plain through aliased command, outside any git repo' '
>
> test_expect_failure 'plain nested through aliased command' '
> (
> - sane_unset GIT_DIR GIT_WORK_TREE &&
> git init plain-ancestor-aliased &&
> cd plain-ancestor-aliased &&
> echo "[alias] aliasedinit = init" >>.git/config &&
> @@ -78,7 +74,6 @@ test_expect_failure 'plain nested through aliased command' '
>
> test_expect_failure 'plain nested in bare through aliased command' '
> (
> - sane_unset GIT_DIR GIT_WORK_TREE &&
> git init --bare bare-ancestor-aliased.git &&
> cd bare-ancestor-aliased.git &&
> echo "[alias] aliasedinit = init" >>config &&
> @@ -91,7 +86,6 @@ test_expect_failure 'plain nested in bare through aliased command' '
>
> test_expect_success 'plain with GIT_WORK_TREE' '
> if (
> - sane_unset GIT_DIR &&
> mkdir plain-wt &&
> cd plain-wt &&
> GIT_WORK_TREE=$(pwd) git init
> @@ -104,7 +98,6 @@ test_expect_success 'plain with GIT_WORK_TREE' '
>
> test_expect_success 'plain bare' '
> (
> - sane_unset GIT_DIR GIT_WORK_TREE GIT_CONFIG &&
> mkdir plain-bare-1 &&
> cd plain-bare-1 &&
> git --bare init
> @@ -114,7 +107,6 @@ test_expect_success 'plain bare' '
>
> test_expect_success 'plain bare with GIT_WORK_TREE' '
> if (
> - sane_unset GIT_DIR GIT_CONFIG &&
> mkdir plain-bare-2 &&
> cd plain-bare-2 &&
> GIT_WORK_TREE=$(pwd) git --bare init
> @@ -128,7 +120,6 @@ test_expect_success 'plain bare with GIT_WORK_TREE' '
> test_expect_success 'GIT_DIR bare' '
>
> (
> - sane_unset GIT_CONFIG &&
> mkdir git-dir-bare.git &&
> GIT_DIR=git-dir-bare.git git init
> ) &&
> @@ -138,7 +129,6 @@ test_expect_success 'GIT_DIR bare' '
> test_expect_success 'init --bare' '
>
> (
> - sane_unset GIT_DIR GIT_WORK_TREE GIT_CONFIG &&
> mkdir init-bare.git &&
> cd init-bare.git &&
> git init --bare
> @@ -149,7 +139,6 @@ test_expect_success 'init --bare' '
> test_expect_success 'GIT_DIR non-bare' '
>
> (
> - sane_unset GIT_CONFIG &&
> mkdir non-bare &&
> cd non-bare &&
> GIT_DIR=.git git init
> @@ -160,7 +149,6 @@ test_expect_success 'GIT_DIR non-bare' '
> test_expect_success 'GIT_DIR & GIT_WORK_TREE (1)' '
>
> (
> - sane_unset GIT_CONFIG &&
> mkdir git-dir-wt-1.git &&
> GIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-1.git git init
> ) &&
> @@ -170,7 +158,6 @@ test_expect_success 'GIT_DIR & GIT_WORK_TREE (1)' '
> test_expect_success 'GIT_DIR & GIT_WORK_TREE (2)' '
>
> if (
> - sane_unset GIT_CONFIG &&
> mkdir git-dir-wt-2.git &&
> GIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-2.git git --bare init
> )
> @@ -183,8 +170,6 @@ test_expect_success 'GIT_DIR & GIT_WORK_TREE (2)' '
> test_expect_success 'reinit' '
>
> (
> - sane_unset GIT_CONFIG GIT_WORK_TREE GIT_CONFIG &&
> -
> mkdir again &&
> cd again &&
> git init >out1 2>err1 &&
> diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh
> index c3443ce..a812783 100755
> --- a/t/t9130-git-svn-authors-file.sh
> +++ b/t/t9130-git-svn-authors-file.sh
> @@ -97,7 +97,6 @@ test_expect_success 'fresh clone with svn.authors-file in config' '
> test x = x"$(git config svn.authorsfile)" &&
> test_config="$HOME"/.gitconfig &&
> sane_unset GIT_DIR &&
> - sane_unset GIT_CONFIG &&
> git config --global \
> svn.authorsfile "$HOME"/svn-authors &&
> test x"$HOME"/svn-authors = x"$(git config svn.authorsfile)" &&
> diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
> index 3edc408..ed98e64 100755
> --- a/t/t9400-git-cvsserver-server.sh
> +++ b/t/t9400-git-cvsserver-server.sh
> @@ -25,7 +25,6 @@ perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
> test_done
> }
>
> -unset GIT_DIR GIT_CONFIG
> WORKDIR=$(pwd)
> SERVERDIR=$(pwd)/gitcvs.git
> git_config="$SERVERDIR/config"
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 03/12] t: drop useless sane_unset GIT_* calls
2014-03-21 21:24 ` Junio C Hamano
@ 2014-03-24 21:56 ` Jeff King
2014-03-24 22:06 ` Junio C Hamano
2014-03-25 4:56 ` Junio C Hamano
0 siblings, 2 replies; 28+ messages in thread
From: Jeff King @ 2014-03-24 21:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Tran, git
On Fri, Mar 21, 2014 at 02:24:31PM -0700, Junio C Hamano wrote:
> > Unsetting these is not only useless, but can be confusing to
> > a reader, who may wonder why some tests in a script unset
> > them and others do not (t0001 is particularly guilty of this
> > inconsistency, probably because many of its tests predate
> > the test-lib.sh environment-cleansing).
> [...]
> > I suppose one could make an argument that test-lib.sh may later change
> > the set of variables it clears, and these unsets are documenting an
> > explicit need of each test. I'd find that more compelling if it were
> > actually applied consistently.
>
> Hmph. I am looking at "git show HEAD^:t/t0001-init.sh" after
> applying this patch, and it does look consistently done with
> GIT_CONFIG and GIT_DIR (I am not sure about GIT_WORK_TREE but from a
> cursory read it is done consistently for tests on non-bare
> repositories).
I don't understand why we stop bothering with the unsets starting with
"init with --template". Are those variables not important to the outcome
of that and later tests, or did the author simply not bother because
they are noops?
> So I would actually agree with your alternative interpretation
> "Unsetting these is useless, but it does serve documentation
> purpose---without having to see what the state of the environment
> when the subprocess is started, the reader can understand what is
> being tested", rather than the one in the log message.
I'd agree with that if I were convinced that the presence of them there
versus the absence of them later was meaningful.
> Having said that, I am perfectly OK with the change to t0001 in this
> patch, if we added at the very beginning of the test sequence a
> comment that says:
>
> Below, creation and use of repositories are tested with various
> combinations of environment settings and command line flags.
> They are done inside subshells to avoid leaking temporary
> environment settings to later tests *and* assumes that the
> initial environment does not have have GIT_DIR, GIT_CONFIG, and
> GIT_WORK_TREE defined.
>
> or something.
I do not have a problem with that, as it implicitly covers all of the
tests following it. I do not think it is particularly necessary, though.
Assuming we start with a known test environment and avoiding polluting
it for further tests are basic principles of _all_ test scripts.
-Peff
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] t: drop useless sane_unset GIT_* calls
2014-03-24 21:56 ` Jeff King
@ 2014-03-24 22:06 ` Junio C Hamano
2014-03-25 4:56 ` Junio C Hamano
1 sibling, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2014-03-24 22:06 UTC (permalink / raw)
To: Jeff King; +Cc: David Tran, git
Jeff King <peff@peff.net> writes:
> I do not have a problem with that, as it implicitly covers all of the
> tests following it. I do not think it is particularly necessary, though.
> Assuming we start with a known test environment and avoiding polluting
> it for further tests are basic principles of _all_ test scripts.
They should be, but I suspect majority of tests, especially the
older ones, do have dependencies on earlier test pieces X-<.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] t: drop useless sane_unset GIT_* calls
2014-03-24 21:56 ` Jeff King
2014-03-24 22:06 ` Junio C Hamano
@ 2014-03-25 4:56 ` Junio C Hamano
1 sibling, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2014-03-25 4:56 UTC (permalink / raw)
To: Jeff King; +Cc: David Tran, git
Jeff King <peff@peff.net> writes:
>> Hmph. I am looking at "git show HEAD^:t/t0001-init.sh" after
>> applying this patch, and it does look consistently done with
>> GIT_CONFIG and GIT_DIR (I am not sure about GIT_WORK_TREE but from a
>> cursory read it is done consistently for tests on non-bare
>> repositories).
>
> I don't understand why we stop bothering with the unsets starting with
> "init with --template". Are those variables not important to the outcome
> of that and later tests, or did the author simply not bother because
> they are noops?
If I had to guess (without running "blame", so it may well turn out
that "the author" may turn out to be me ;-), it is simply the author
not being careful.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 04/12] t: stop using GIT_CONFIG to cross repo boundaries
2014-03-20 23:11 ` [PATCH 0/12] GIT_CONFIG in the test suite Jeff King
` (2 preceding siblings ...)
2014-03-20 23:14 ` [PATCH 03/12] t: drop useless sane_unset GIT_* calls Jeff King
@ 2014-03-20 23:15 ` Jeff King
2014-03-21 21:26 ` Junio C Hamano
2014-03-20 23:15 ` [PATCH 05/12] t: prefer "git config --file" to GIT_CONFIG with test_must_fail Jeff King
` (7 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: Jeff King @ 2014-03-20 23:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Tran, git
Some tests want to check or set config in another
repository. E.g., t1000 creates repositories and makes sure
that their core.bare and core.worktree settings are what we
expect. We can do this with:
GIT_CONFIG=$repo/.git/config git config ...
but it better shows the intent to just enter the repository
and let "git config" do the normal lookups:
(cd $repo && git config ...)
In theory, this would cause us to use an extra subshell, but
in all such cases, we are actually already in a subshell.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0001-init.sh | 4 ++--
t/t5701-clone-local.sh | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index ddc8160..9b05fdf 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -12,8 +12,8 @@ check_config () {
echo "expected a directory $1, a file $1/config and $1/refs"
return 1
fi
- bare=$(GIT_CONFIG="$1/config" git config --bool core.bare)
- worktree=$(GIT_CONFIG="$1/config" git config core.worktree) ||
+ bare=$(cd "$1" && git config --bool core.bare)
+ worktree=$(cd "$1" && git config core.worktree) ||
worktree=unset
test "$bare" = "$2" && test "$worktree" = "$3" || {
diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
index c490368..3c087e9 100755
--- a/t/t5701-clone-local.sh
+++ b/t/t5701-clone-local.sh
@@ -12,8 +12,8 @@ test_expect_success 'preparing origin repository' '
: >file && git add . && git commit -m1 &&
git clone --bare . a.git &&
git clone --bare . x &&
- test "$(GIT_CONFIG=a.git/config git config --bool core.bare)" = true &&
- test "$(GIT_CONFIG=x/config git config --bool core.bare)" = true &&
+ test "$(cd a.git && git config --bool core.bare)" = true &&
+ test "$(cd x && git config --bool core.bare)" = true &&
git bundle create b1.bundle --all &&
git bundle create b2.bundle master &&
mkdir dir &&
@@ -24,7 +24,7 @@ test_expect_success 'preparing origin repository' '
test_expect_success 'local clone without .git suffix' '
git clone -l -s a b &&
(cd b &&
- test "$(GIT_CONFIG=.git/config git config --bool core.bare)" = false &&
+ test "$(git config --bool core.bare)" = false &&
git fetch)
'
--
1.9.0.560.g01ceb46
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 04/12] t: stop using GIT_CONFIG to cross repo boundaries
2014-03-20 23:15 ` [PATCH 04/12] t: stop using GIT_CONFIG to cross repo boundaries Jeff King
@ 2014-03-21 21:26 ` Junio C Hamano
2014-03-24 22:00 ` Jeff King
0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2014-03-21 21:26 UTC (permalink / raw)
To: Jeff King; +Cc: David Tran, git
Jeff King <peff@peff.net> writes:
> Some tests want to check or set config in another
> repository. E.g., t1000 creates repositories and makes sure
> that their core.bare and core.worktree settings are what we
> expect. We can do this with:
>
> GIT_CONFIG=$repo/.git/config git config ...
>
> but it better shows the intent to just enter the repository
> and let "git config" do the normal lookups:
>
> (cd $repo && git config ...)
>
> In theory, this would cause us to use an extra subshell, but
> in all such cases, we are actually already in a subshell.
Sure; alternatively we could use "git -C $there", but this rewrite
is fine by me.
Thanks.
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> t/t0001-init.sh | 4 ++--
> t/t5701-clone-local.sh | 6 +++---
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index ddc8160..9b05fdf 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -12,8 +12,8 @@ check_config () {
> echo "expected a directory $1, a file $1/config and $1/refs"
> return 1
> fi
> - bare=$(GIT_CONFIG="$1/config" git config --bool core.bare)
> - worktree=$(GIT_CONFIG="$1/config" git config core.worktree) ||
> + bare=$(cd "$1" && git config --bool core.bare)
> + worktree=$(cd "$1" && git config core.worktree) ||
> worktree=unset
>
> test "$bare" = "$2" && test "$worktree" = "$3" || {
> diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
> index c490368..3c087e9 100755
> --- a/t/t5701-clone-local.sh
> +++ b/t/t5701-clone-local.sh
> @@ -12,8 +12,8 @@ test_expect_success 'preparing origin repository' '
> : >file && git add . && git commit -m1 &&
> git clone --bare . a.git &&
> git clone --bare . x &&
> - test "$(GIT_CONFIG=a.git/config git config --bool core.bare)" = true &&
> - test "$(GIT_CONFIG=x/config git config --bool core.bare)" = true &&
> + test "$(cd a.git && git config --bool core.bare)" = true &&
> + test "$(cd x && git config --bool core.bare)" = true &&
> git bundle create b1.bundle --all &&
> git bundle create b2.bundle master &&
> mkdir dir &&
> @@ -24,7 +24,7 @@ test_expect_success 'preparing origin repository' '
> test_expect_success 'local clone without .git suffix' '
> git clone -l -s a b &&
> (cd b &&
> - test "$(GIT_CONFIG=.git/config git config --bool core.bare)" = false &&
> + test "$(git config --bool core.bare)" = false &&
> git fetch)
> '
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 04/12] t: stop using GIT_CONFIG to cross repo boundaries
2014-03-21 21:26 ` Junio C Hamano
@ 2014-03-24 22:00 ` Jeff King
0 siblings, 0 replies; 28+ messages in thread
From: Jeff King @ 2014-03-24 22:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Tran, git
On Fri, Mar 21, 2014 at 02:26:02PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Some tests want to check or set config in another
> > repository. E.g., t1000 creates repositories and makes sure
> > that their core.bare and core.worktree settings are what we
> > expect. We can do this with:
> >
> > GIT_CONFIG=$repo/.git/config git config ...
> >
> > but it better shows the intent to just enter the repository
> > and let "git config" do the normal lookups:
> >
> > (cd $repo && git config ...)
> >
> > In theory, this would cause us to use an extra subshell, but
> > in all such cases, we are actually already in a subshell.
>
> Sure; alternatively we could use "git -C $there", but this rewrite
> is fine by me.
The existing callers all pass actual $GIT_DIRs, so I initially wrote it
as "git --git-dir=$repo config ...". Doing it as "-C" is perhaps nicer,
as callers could potentially pass a shorter string to the repo root,
and not bother with adding "/.git". However, t0001 needs the actual
$GIT_DIR (because it looks for things like the refs/ directory in the
same function), and the other callers are just passing bare repos.
So I'm fine with any of them. Feel free to mark it up if you have a
preference.
-Peff
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 05/12] t: prefer "git config --file" to GIT_CONFIG with test_must_fail
2014-03-20 23:11 ` [PATCH 0/12] GIT_CONFIG in the test suite Jeff King
` (3 preceding siblings ...)
2014-03-20 23:15 ` [PATCH 04/12] t: stop using GIT_CONFIG to cross repo boundaries Jeff King
@ 2014-03-20 23:15 ` Jeff King
2014-03-20 23:17 ` [PATCH 06/12] t: prefer "git config --file" to GIT_CONFIG Jeff King
` (6 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Jeff King @ 2014-03-20 23:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Tran, git
This lets us get rid of an extra "env" invocation in the
middle, and is slightly more readable.
Signed-off-by: Jeff King <peff@peff.net>
---
The case that started this all...
This is also the only reason this series needs to go on top of David's
patch.
t/t1300-repo-config.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index cd23d07..e355aa1 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -961,15 +961,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '
'
test_expect_success 'nonexistent configuration' '
- test_must_fail env GIT_CONFIG=doesnotexist git config --list &&
- test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy
+ test_must_fail git config --file=doesnotexist --list &&
+ test_must_fail git config --file=doesnotexist test.xyzzy
'
test_expect_success SYMLINKS 'symlink to nonexistent configuration' '
ln -s doesnotexist linktonada &&
ln -s linktonada linktolinktonada &&
- test_must_fail env GIT_CONFIG=linktonada git config --list &&
- test_must_fail env GIT_CONFIG=linktolinktonada git config --list
+ test_must_fail git config --file=linktonada --list &&
+ test_must_fail git config --file=linktolinktonada --list
'
test_expect_success 'check split_cmdline return' "
--
1.9.0.560.g01ceb46
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 06/12] t: prefer "git config --file" to GIT_CONFIG
2014-03-20 23:11 ` [PATCH 0/12] GIT_CONFIG in the test suite Jeff King
` (4 preceding siblings ...)
2014-03-20 23:15 ` [PATCH 05/12] t: prefer "git config --file" to GIT_CONFIG with test_must_fail Jeff King
@ 2014-03-20 23:17 ` Jeff King
2014-03-20 23:17 ` [PATCH 07/12] t0001: make symlink reinit test more careful Jeff King
` (5 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Jeff King @ 2014-03-20 23:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Tran, git
Doing:
GIT_CONFIG=foo git config ...
is equivalent to:
git config --file=foo ...
The latter is easier to read and slightly less error-prone,
because of issues with one-shot variables and shell
functions (e.g., you cannot use the former with
test_must_fail).
Note that we explicitly leave one case in t1300 which checks
the same operation on both GIT_CONFIG and "git config
--file". They are equivalent in the code these days, but
this will make sure it remains so.
Signed-off-by: Jeff King <peff@peff.net>
---
Unlike the last patch, this one has no tangible benefits besides "Peff
thinks it looks better". I also tend to think that GIT_CONFIG is
something that it would be nice to get rid of in the long run, but I
don't have any immediate plans to do so.
t/t1300-repo-config.sh | 20 ++++++++++----------
t/t1302-repo-version.sh | 2 +-
t/t7400-submodule-basic.sh | 5 ++---
t/t9130-git-svn-authors-file.sh | 2 +-
t/t9154-git-svn-fancy-glob.sh | 6 +++---
5 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index e355aa1..85c6637 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -461,7 +461,7 @@ test_expect_success 'new variable inserts into proper section' '
test_cmp expect .git/config
'
-test_expect_success 'alternative GIT_CONFIG (non-existing file should fail)' '
+test_expect_success 'alternative --file (non-existing file should fail)' '
test_must_fail git config --file non-existing-config -l
'
@@ -495,10 +495,10 @@ test_expect_success 'refer config from subdirectory' '
'
-test_expect_success 'refer config from subdirectory via GIT_CONFIG' '
+test_expect_success 'refer config from subdirectory via --file' '
(
cd x &&
- GIT_CONFIG=../other-config git config --get ein.bahn >actual &&
+ git config --file=../other-config --get ein.bahn >actual &&
test_cmp expect actual
)
'
@@ -510,8 +510,8 @@ cat > expect << EOF
park = ausweis
EOF
-test_expect_success '--set in alternative GIT_CONFIG' '
- GIT_CONFIG=other-config git config anwohner.park ausweis &&
+test_expect_success '--set in alternative file' '
+ git config --file=other-config anwohner.park ausweis &&
test_cmp expect other-config
'
@@ -942,11 +942,11 @@ test_expect_success 'inner whitespace kept verbatim' '
test_expect_success SYMLINKS 'symlinked configuration' '
ln -s notyet myconfig &&
- GIT_CONFIG=myconfig git config test.frotz nitfol &&
+ git config --file=myconfig test.frotz nitfol &&
test -h myconfig &&
test -f notyet &&
- test "z$(GIT_CONFIG=notyet git config test.frotz)" = znitfol &&
- GIT_CONFIG=myconfig git config test.xyzzy rezrov &&
+ test "z$(git config --file=notyet test.frotz)" = znitfol &&
+ git config --file=myconfig test.xyzzy rezrov &&
test -h myconfig &&
test -f notyet &&
cat >expect <<-\EOF &&
@@ -954,8 +954,8 @@ test_expect_success SYMLINKS 'symlinked configuration' '
rezrov
EOF
{
- GIT_CONFIG=notyet git config test.frotz &&
- GIT_CONFIG=notyet git config test.xyzzy
+ git config --file=notyet test.frotz &&
+ git config --file=notyet test.xyzzy
} >actual &&
test_cmp expect actual
'
diff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh
index 0e47662..0d9388a 100755
--- a/t/t1302-repo-version.sh
+++ b/t/t1302-repo-version.sh
@@ -19,7 +19,7 @@ test_expect_success 'setup' '
test_create_repo "test" &&
test_create_repo "test2" &&
- GIT_CONFIG=test2/.git/config git config core.repositoryformatversion 99
+ git config --file=test2/.git/config core.repositoryformatversion 99
'
test_expect_success 'gitdir selection on normal repos' '
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index c28e8d8..7c88245 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -249,8 +249,7 @@ test_expect_success 'submodule add in subdirectory with relative path should fai
'
test_expect_success 'setup - add an example entry to .gitmodules' '
- GIT_CONFIG=.gitmodules \
- git config submodule.example.url git://example.com/init.git
+ git config --file=.gitmodules submodule.example.url git://example.com/init.git
'
test_expect_success 'status should fail for unmapped paths' '
@@ -264,7 +263,7 @@ test_expect_success 'setup - map path in .gitmodules' '
path = init
EOF
- GIT_CONFIG=.gitmodules git config submodule.example.path init &&
+ git config --file=.gitmodules submodule.example.path init &&
test_cmp expect .gitmodules
'
diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh
index a812783..c44de26 100755
--- a/t/t9130-git-svn-authors-file.sh
+++ b/t/t9130-git-svn-authors-file.sh
@@ -67,7 +67,7 @@ test_expect_success 'fetch fails on ee' '
'
tmp_config_get () {
- GIT_CONFIG=.git/svn/.metadata git config --get "$1"
+ git config --file=.git/svn/.metadata --get "$1"
}
test_expect_success 'failure happened without negative side effects' '
diff --git a/t/t9154-git-svn-fancy-glob.sh b/t/t9154-git-svn-fancy-glob.sh
index b780e0e..a0150f0 100755
--- a/t/t9154-git-svn-fancy-glob.sh
+++ b/t/t9154-git-svn-fancy-glob.sh
@@ -22,7 +22,7 @@ test_expect_success 'add red branch' "
"
test_expect_success 'add gre branch' "
- GIT_CONFIG=.git/svn/.metadata git config --unset svn-remote.svn.branches-maxRev &&
+ git config --file=.git/svn/.metadata --unset svn-remote.svn.branches-maxRev &&
git config svn-remote.svn.branches 'branches/{red,gre}:refs/remotes/*' &&
git svn fetch &&
git rev-parse refs/remotes/red &&
@@ -31,7 +31,7 @@ test_expect_success 'add gre branch' "
"
test_expect_success 'add green branch' "
- GIT_CONFIG=.git/svn/.metadata git config --unset svn-remote.svn.branches-maxRev &&
+ git config --file=.git/svn/.metadata --unset svn-remote.svn.branches-maxRev &&
git config svn-remote.svn.branches 'branches/{red,green}:refs/remotes/*' &&
git svn fetch &&
git rev-parse refs/remotes/red &&
@@ -40,7 +40,7 @@ test_expect_success 'add green branch' "
"
test_expect_success 'add all branches' "
- GIT_CONFIG=.git/svn/.metadata git config --unset svn-remote.svn.branches-maxRev &&
+ git config --file=.git/svn/.metadata --unset svn-remote.svn.branches-maxRev &&
git config svn-remote.svn.branches 'branches/*:refs/remotes/*' &&
git svn fetch &&
git rev-parse refs/remotes/red &&
--
1.9.0.560.g01ceb46
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 07/12] t0001: make symlink reinit test more careful
2014-03-20 23:11 ` [PATCH 0/12] GIT_CONFIG in the test suite Jeff King
` (5 preceding siblings ...)
2014-03-20 23:17 ` [PATCH 06/12] t: prefer "git config --file" to GIT_CONFIG Jeff King
@ 2014-03-20 23:17 ` Jeff King
2014-03-20 23:17 ` [PATCH 08/12] t0001: use test_path_is_* Jeff King
` (4 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Jeff King @ 2014-03-20 23:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Tran, git
In the final test of t0001, we have a repo whose .git is a
symlink to a directory "here", and we use
"--separate-git-dir" to migrate that to a .git file pointing
to a different directory. We check that the data is migrated
to the new directory and that .git looks like a git-file.
We also check that "here" is not a directory, which is
slightly misleading. It should not be a directory, but
neither should it be gone. It is the actual resting place of
the git-file, and .git remains a symlink to it.
Let's check that more explicitly, both to make our test more
robust, and to make further cleanups in this area more
obvious.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0001-init.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 9b05fdf..5245711 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -402,8 +402,8 @@ test_expect_success SYMLINKS 're-init to move gitdir symlink' '
) &&
echo "gitdir: `pwd`/realgitdir" >expected &&
test_cmp expected newdir/.git &&
- test -d realgitdir/refs &&
- ! test -d newdir/here
+ test_cmp expected newdir/here &&
+ test -d realgitdir/refs
'
test_done
--
1.9.0.560.g01ceb46
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 08/12] t0001: use test_path_is_*
2014-03-20 23:11 ` [PATCH 0/12] GIT_CONFIG in the test suite Jeff King
` (6 preceding siblings ...)
2014-03-20 23:17 ` [PATCH 07/12] t0001: make symlink reinit test more careful Jeff King
@ 2014-03-20 23:17 ` Jeff King
2014-03-20 23:18 ` [PATCH 09/12] t0001: use test_config_global Jeff King
` (3 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Jeff King @ 2014-03-20 23:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Tran, git
t0001 predates the test_path_is_* helpers, and uses "test
-f" and "test -d" directly. Using the helpers provides
better debugging output, and are a little more robust.
As opposed to "! test -d", test_path_is_missing will
actually makes sure the path does not exist at all.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0001-init.sh | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 5245711..fdcf4b3 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -199,13 +199,13 @@ test_expect_success 'init with --template (blank)' '
cd template-plain &&
git init
) &&
- test -f template-plain/.git/info/exclude &&
+ test_path_is_file template-plain/.git/info/exclude &&
(
mkdir template-blank &&
cd template-blank &&
git init --template=
) &&
- ! test -f template-blank/.git/info/exclude
+ test_path_is_missing template-blank/.git/info/exclude
'
test_expect_success 'init with init.templatedir set' '
@@ -263,7 +263,7 @@ test_expect_success 'init creates a new directory' '
rm -fr newdir &&
(
git init newdir &&
- test -d newdir/.git/refs
+ test_path_is_dir newdir/.git/refs
)
'
@@ -271,7 +271,7 @@ test_expect_success 'init creates a new bare directory' '
rm -fr newdir &&
(
git init --bare newdir &&
- test -d newdir/refs
+ test_path_is_dir newdir/refs
)
'
@@ -280,7 +280,7 @@ test_expect_success 'init recreates a directory' '
(
mkdir newdir &&
git init newdir &&
- test -d newdir/.git/refs
+ test_path_is_dir newdir/.git/refs
)
'
@@ -289,14 +289,14 @@ test_expect_success 'init recreates a new bare directory' '
(
mkdir newdir &&
git init --bare newdir &&
- test -d newdir/refs
+ test_path_is_dir newdir/refs
)
'
test_expect_success 'init creates a new deep directory' '
rm -fr newdir &&
git init newdir/a/b/c &&
- test -d newdir/a/b/c/.git/refs
+ test_path_is_dir newdir/a/b/c/.git/refs
'
test_expect_success POSIXPERM 'init creates a new deep directory (umask vs. shared)' '
@@ -306,7 +306,7 @@ test_expect_success POSIXPERM 'init creates a new deep directory (umask vs. shar
# the repository itself should follow "shared"
umask 002 &&
git init --bare --shared=0660 newdir/a/b/c &&
- test -d newdir/a/b/c/refs &&
+ test_path_is_dir newdir/a/b/c/refs &&
ls -ld newdir/a newdir/a/b > lsab.out &&
! grep -v "^drwxrw[sx]r-x" lsab.out &&
ls -ld newdir/a/b/c > lsc.out &&
@@ -319,7 +319,7 @@ test_expect_success 'init notices EEXIST (1)' '
(
>newdir &&
test_must_fail git init newdir &&
- test -f newdir
+ test_path_is_file newdir
)
'
@@ -329,7 +329,7 @@ test_expect_success 'init notices EEXIST (2)' '
mkdir newdir &&
>newdir/a
test_must_fail git init newdir/a/b &&
- test -f newdir/a
+ test_path_is_file newdir/a
)
'
@@ -345,15 +345,15 @@ test_expect_success POSIXPERM,SANITY 'init notices EPERM' '
test_expect_success 'init creates a new bare directory with global --bare' '
rm -rf newdir &&
git --bare init newdir &&
- test -d newdir/refs
+ test_path_is_dir newdir/refs
'
test_expect_success 'init prefers command line to GIT_DIR' '
rm -rf newdir &&
mkdir otherdir &&
GIT_DIR=otherdir git --bare init newdir &&
- test -d newdir/refs &&
- ! test -d otherdir/refs
+ test_path_is_dir newdir/refs &&
+ test_path_is_missing otherdir/refs
'
test_expect_success 'init with separate gitdir' '
@@ -361,7 +361,7 @@ test_expect_success 'init with separate gitdir' '
git init --separate-git-dir realgitdir newdir &&
echo "gitdir: `pwd`/realgitdir" >expected &&
test_cmp expected newdir/.git &&
- test -d realgitdir/refs
+ test_path_is_dir realgitdir/refs
'
test_expect_success 're-init on .git file' '
@@ -375,8 +375,8 @@ test_expect_success 're-init to update git link' '
) &&
echo "gitdir: `pwd`/surrealgitdir" >expected &&
test_cmp expected newdir/.git &&
- test -d surrealgitdir/refs &&
- ! test -d realgitdir/refs
+ test_path_is_dir surrealgitdir/refs &&
+ test_path_is_missing realgitdir/refs
'
test_expect_success 're-init to move gitdir' '
@@ -388,7 +388,7 @@ test_expect_success 're-init to move gitdir' '
) &&
echo "gitdir: `pwd`/realgitdir" >expected &&
test_cmp expected newdir/.git &&
- test -d realgitdir/refs
+ test_path_is_dir realgitdir/refs
'
test_expect_success SYMLINKS 're-init to move gitdir symlink' '
@@ -403,7 +403,7 @@ test_expect_success SYMLINKS 're-init to move gitdir symlink' '
echo "gitdir: `pwd`/realgitdir" >expected &&
test_cmp expected newdir/.git &&
test_cmp expected newdir/here &&
- test -d realgitdir/refs
+ test_path_is_dir realgitdir/refs
'
test_done
--
1.9.0.560.g01ceb46
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 09/12] t0001: use test_config_global
2014-03-20 23:11 ` [PATCH 0/12] GIT_CONFIG in the test suite Jeff King
` (7 preceding siblings ...)
2014-03-20 23:17 ` [PATCH 08/12] t0001: use test_path_is_* Jeff King
@ 2014-03-20 23:18 ` Jeff King
2014-03-20 23:19 ` [PATCH 10/12] t0001: use test_must_fail Jeff King
` (2 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Jeff King @ 2014-03-20 23:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Tran, git
We hand-set several config options using :
git config -f $HOME/.gitconfig ...
Instead, we can use "test_config_global". Not only is this
more readable, but it cleans up for us so that subsequent
tests aren't polluted by our settings.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0001-init.sh | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index fdcf4b3..9515da3 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -211,9 +211,8 @@ test_expect_success 'init with --template (blank)' '
test_expect_success 'init with init.templatedir set' '
mkdir templatedir-source &&
echo Content >templatedir-source/file &&
+ test_config_global init.templatedir "${HOME}/templatedir-source" &&
(
- test_config="${HOME}/.gitconfig" &&
- git config -f "$test_config" init.templatedir "${HOME}/templatedir-source" &&
mkdir templatedir-set &&
cd templatedir-set &&
sane_unset GIT_TEMPLATE_DIR &&
@@ -225,10 +224,9 @@ test_expect_success 'init with init.templatedir set' '
'
test_expect_success 'init --bare/--shared overrides system/global config' '
+ test_config_global core.bare false &&
+ test_config_global core.sharedRepository 0640 &&
(
- test_config="$HOME"/.gitconfig &&
- git config -f "$test_config" core.bare false &&
- git config -f "$test_config" core.sharedRepository 0640 &&
mkdir init-bare-shared-override &&
cd init-bare-shared-override &&
git init --bare --shared=0666
@@ -239,9 +237,8 @@ test_expect_success 'init --bare/--shared overrides system/global config' '
'
test_expect_success 'init honors global core.sharedRepository' '
+ test_config_global core.sharedRepository 0666 &&
(
- test_config="$HOME"/.gitconfig &&
- git config -f "$test_config" core.sharedRepository 0666 &&
mkdir shared-honor-global &&
cd shared-honor-global &&
git init
--
1.9.0.560.g01ceb46
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 10/12] t0001: use test_must_fail
2014-03-20 23:11 ` [PATCH 0/12] GIT_CONFIG in the test suite Jeff King
` (8 preceding siblings ...)
2014-03-20 23:18 ` [PATCH 09/12] t0001: use test_config_global Jeff King
@ 2014-03-20 23:19 ` Jeff King
2014-03-20 23:21 ` [PATCH 11/12] t0001: drop useless subshells Jeff King
2014-03-20 23:23 ` [PATCH 12/12] t0001: drop subshells just for "cd" Jeff King
11 siblings, 0 replies; 28+ messages in thread
From: Jeff King @ 2014-03-20 23:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Tran, git
We've hand-rolled several "if" statements looking for
failures. We can use test_must_fail here, which is shorter
and more robust.
Note that we modify the commands slightly (to use "git init
foo" rather than "cd foo && git init") to avoid dealing with
a subshell, but this should not affect the outcome.
Signed-off-by: Jeff King <peff@peff.net>
---
I'm pretty sure we can actually drop the "mkdir" in each of
these cases, too, but I was trying to leave things as close
to the original as possible.
t/t0001-init.sh | 38 +++++++++++---------------------------
1 file changed, 11 insertions(+), 27 deletions(-)
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 9515da3..4560bba 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -85,15 +85,8 @@ test_expect_failure 'plain nested in bare through aliased command' '
'
test_expect_success 'plain with GIT_WORK_TREE' '
- if (
- mkdir plain-wt &&
- cd plain-wt &&
- GIT_WORK_TREE=$(pwd) git init
- )
- then
- echo Should have failed -- GIT_WORK_TREE should not be used
- false
- fi
+ mkdir plain-wt &&
+ test_must_fail env GIT_WORK_TREE="$(pwd)/plain-wt" git init plain-wt
'
test_expect_success 'plain bare' '
@@ -106,15 +99,10 @@ test_expect_success 'plain bare' '
'
test_expect_success 'plain bare with GIT_WORK_TREE' '
- if (
- mkdir plain-bare-2 &&
- cd plain-bare-2 &&
- GIT_WORK_TREE=$(pwd) git --bare init
- )
- then
- echo Should have failed -- GIT_WORK_TREE should not be used
- false
- fi
+ mkdir plain-bare-2 &&
+ test_must_fail \
+ env GIT_WORK_TREE="$(pwd)/plain-bare-2" \
+ git --bare init plain-bare-2
'
test_expect_success 'GIT_DIR bare' '
@@ -156,15 +144,11 @@ test_expect_success 'GIT_DIR & GIT_WORK_TREE (1)' '
'
test_expect_success 'GIT_DIR & GIT_WORK_TREE (2)' '
-
- if (
- mkdir git-dir-wt-2.git &&
- GIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-2.git git --bare init
- )
- then
- echo Should have failed -- --bare should not be used
- false
- fi
+ mkdir git-dir-wt-2.git &&
+ test_must_fail env \
+ GIT_WORK_TREE="$(pwd)" \
+ GIT_DIR=git-dir-wt-2.git \
+ git --bare init
'
test_expect_success 'reinit' '
--
1.9.0.560.g01ceb46
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 11/12] t0001: drop useless subshells
2014-03-20 23:11 ` [PATCH 0/12] GIT_CONFIG in the test suite Jeff King
` (9 preceding siblings ...)
2014-03-20 23:19 ` [PATCH 10/12] t0001: use test_must_fail Jeff King
@ 2014-03-20 23:21 ` Jeff King
2014-03-21 20:27 ` Eric Sunshine
2014-03-20 23:23 ` [PATCH 12/12] t0001: drop subshells just for "cd" Jeff King
11 siblings, 1 reply; 28+ messages in thread
From: Jeff King @ 2014-03-20 23:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Tran, git
Many tests use subshells, but don't actually change the
shell environment. They were probably cargo-culted from
earlier tests which did need subshells. Drop the useless
ones.
Signed-off-by: Jeff King <peff@peff.net>
---
These ones should produce no behavior change at all; they're purely
mechanical "(foo && bar)" to "foo && bar" (though of course I did them
by hand, because you need to know that "foo" and "bar" do not affect the
environment).
t/t0001-init.sh | 61 +++++++++++++++++++++------------------------------------
1 file changed, 22 insertions(+), 39 deletions(-)
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 4560bba..55a68bc 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -106,11 +106,8 @@ test_expect_success 'plain bare with GIT_WORK_TREE' '
'
test_expect_success 'GIT_DIR bare' '
-
- (
- mkdir git-dir-bare.git &&
- GIT_DIR=git-dir-bare.git git init
- ) &&
+ mkdir git-dir-bare.git &&
+ GIT_DIR=git-dir-bare.git git init &&
check_config git-dir-bare.git true unset
'
@@ -242,36 +239,28 @@ test_expect_success 'init rejects insanely long --template' '
test_expect_success 'init creates a new directory' '
rm -fr newdir &&
- (
- git init newdir &&
- test_path_is_dir newdir/.git/refs
- )
+ git init newdir &&
+ test_path_is_dir newdir/.git/refs
'
test_expect_success 'init creates a new bare directory' '
rm -fr newdir &&
- (
- git init --bare newdir &&
- test_path_is_dir newdir/refs
- )
+ git init --bare newdir &&
+ test_path_is_dir newdir/refs
'
test_expect_success 'init recreates a directory' '
rm -fr newdir &&
- (
- mkdir newdir &&
- git init newdir &&
- test_path_is_dir newdir/.git/refs
- )
+ mkdir newdir &&
+ git init newdir &&
+ test_path_is_dir newdir/.git/refs
'
test_expect_success 'init recreates a new bare directory' '
rm -fr newdir &&
- (
- mkdir newdir &&
- git init --bare newdir &&
- test_path_is_dir newdir/refs
- )
+ mkdir newdir &&
+ git init --bare newdir &&
+ test_path_is_dir newdir/refs
'
test_expect_success 'init creates a new deep directory' '
@@ -297,30 +286,24 @@ test_expect_success POSIXPERM 'init creates a new deep directory (umask vs. shar
test_expect_success 'init notices EEXIST (1)' '
rm -fr newdir &&
- (
- >newdir &&
- test_must_fail git init newdir &&
- test_path_is_file newdir
- )
+ >newdir &&
+ test_must_fail git init newdir &&
+ test_path_is_file newdir
'
test_expect_success 'init notices EEXIST (2)' '
rm -fr newdir &&
- (
- mkdir newdir &&
- >newdir/a
- test_must_fail git init newdir/a/b &&
- test_path_is_file newdir/a
- )
+ mkdir newdir &&
+ >newdir/a
+ test_must_fail git init newdir/a/b &&
+ test_path_is_file newdir/a
'
test_expect_success POSIXPERM,SANITY 'init notices EPERM' '
rm -fr newdir &&
- (
- mkdir newdir &&
- chmod -w newdir &&
- test_must_fail git init newdir/a/b
- )
+ mkdir newdir &&
+ chmod -w newdir &&
+ test_must_fail git init newdir/a/b
'
test_expect_success 'init creates a new bare directory with global --bare' '
--
1.9.0.560.g01ceb46
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 11/12] t0001: drop useless subshells
2014-03-20 23:21 ` [PATCH 11/12] t0001: drop useless subshells Jeff King
@ 2014-03-21 20:27 ` Eric Sunshine
0 siblings, 0 replies; 28+ messages in thread
From: Eric Sunshine @ 2014-03-21 20:27 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, David Tran, Git List
On Thu, Mar 20, 2014 at 7:21 PM, Jeff King <peff@peff.net> wrote:
> Many tests use subshells, but don't actually change the
> shell environment. They were probably cargo-culted from
> earlier tests which did need subshells. Drop the useless
> ones.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> These ones should produce no behavior change at all; they're purely
> mechanical "(foo && bar)" to "foo && bar" (though of course I did them
> by hand, because you need to know that "foo" and "bar" do not affect the
> environment).
>
> t/t0001-init.sh | 61 +++++++++++++++++++++------------------------------------
> 1 file changed, 22 insertions(+), 39 deletions(-)
>
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index 4560bba..55a68bc 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -297,30 +286,24 @@ test_expect_success POSIXPERM 'init creates a new deep directory (umask vs. shar
>
> test_expect_success 'init notices EEXIST (2)' '
> rm -fr newdir &&
> - (
> - mkdir newdir &&
> - >newdir/a
> - test_must_fail git init newdir/a/b &&
> - test_path_is_file newdir/a
> - )
> + mkdir newdir &&
> + >newdir/a
Broken &&-chain (though, not introduced by this patch).
> + test_must_fail git init newdir/a/b &&
> + test_path_is_file newdir/a
> '
>
> test_expect_success POSIXPERM,SANITY 'init notices EPERM' '
> rm -fr newdir &&
> - (
> - mkdir newdir &&
> - chmod -w newdir &&
> - test_must_fail git init newdir/a/b
> - )
> + mkdir newdir &&
> + chmod -w newdir &&
> + test_must_fail git init newdir/a/b
> '
>
> test_expect_success 'init creates a new bare directory with global --bare' '
> --
> 1.9.0.560.g01ceb46
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 12/12] t0001: drop subshells just for "cd"
2014-03-20 23:11 ` [PATCH 0/12] GIT_CONFIG in the test suite Jeff King
` (10 preceding siblings ...)
2014-03-20 23:21 ` [PATCH 11/12] t0001: drop useless subshells Jeff King
@ 2014-03-20 23:23 ` Jeff King
11 siblings, 0 replies; 28+ messages in thread
From: Jeff King @ 2014-03-20 23:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Tran, git
Many tests do something like:
(
mkdir foo &&
cd foo &&
git init
)
You can do the same these days with "git init foo", which
makes the tests shorter and simpler to read.
Signed-off-by: Jeff King <peff@peff.net>
---
Unlike the last patch, this one _could_ have an affect. I made the
assumption that "git init foo" would behave sanely, but that other
complex things should be left alone. E.g., ones that set GIT_DIR in the
environment to a relative path might be affected based on when git does
the "chdir".
t/t0001-init.sh | 56 +++++++++-----------------------------------------------
1 file changed, 9 insertions(+), 47 deletions(-)
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 55a68bc..68549d1 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -24,11 +24,7 @@ check_config () {
}
test_expect_success 'plain' '
- (
- mkdir plain &&
- cd plain &&
- git init
- ) &&
+ git init plain &&
check_config plain/.git false unset
'
@@ -90,11 +86,7 @@ test_expect_success 'plain with GIT_WORK_TREE' '
'
test_expect_success 'plain bare' '
- (
- mkdir plain-bare-1 &&
- cd plain-bare-1 &&
- git --bare init
- ) &&
+ git --bare init plain-bare-1 &&
check_config plain-bare-1 true unset
'
@@ -112,12 +104,7 @@ test_expect_success 'GIT_DIR bare' '
'
test_expect_success 'init --bare' '
-
- (
- mkdir init-bare.git &&
- cd init-bare.git &&
- git init --bare
- ) &&
+ git init --bare init-bare.git &&
check_config init-bare.git true unset
'
@@ -166,26 +153,14 @@ test_expect_success 'reinit' '
test_expect_success 'init with --template' '
mkdir template-source &&
echo content >template-source/file &&
- (
- mkdir template-custom &&
- cd template-custom &&
- git init --template=../template-source
- ) &&
+ git init --template=../template-source template-custom &&
test_cmp template-source/file template-custom/.git/file
'
test_expect_success 'init with --template (blank)' '
- (
- mkdir template-plain &&
- cd template-plain &&
- git init
- ) &&
+ git init template-plain &&
test_path_is_file template-plain/.git/info/exclude &&
- (
- mkdir template-blank &&
- cd template-blank &&
- git init --template=
- ) &&
+ git init --template= template-blank &&
test_path_is_missing template-blank/.git/info/exclude
'
@@ -207,11 +182,7 @@ test_expect_success 'init with init.templatedir set' '
test_expect_success 'init --bare/--shared overrides system/global config' '
test_config_global core.bare false &&
test_config_global core.sharedRepository 0640 &&
- (
- mkdir init-bare-shared-override &&
- cd init-bare-shared-override &&
- git init --bare --shared=0666
- ) &&
+ git init --bare --shared=0666 init-bare-shared-override &&
check_config init-bare-shared-override true unset &&
test x0666 = \
x`git config -f init-bare-shared-override/config core.sharedRepository`
@@ -219,22 +190,13 @@ test_expect_success 'init --bare/--shared overrides system/global config' '
test_expect_success 'init honors global core.sharedRepository' '
test_config_global core.sharedRepository 0666 &&
- (
- mkdir shared-honor-global &&
- cd shared-honor-global &&
- git init
- ) &&
+ git init shared-honor-global &&
test x0666 = \
x`git config -f shared-honor-global/.git/config core.sharedRepository`
'
test_expect_success 'init rejects insanely long --template' '
- (
- insane=$(printf "x%09999dx" 1) &&
- mkdir test &&
- cd test &&
- test_must_fail git init --template=$insane
- )
+ test_must_fail git init --template=$(printf "x%09999dx" 1) test
'
test_expect_success 'init creates a new directory' '
--
1.9.0.560.g01ceb46
^ permalink raw reply related [flat|nested] 28+ messages in thread