From d66f68ff985f494f3122ab05b532c6e41884ad64 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 17 May 2016 15:36:26 -0400 Subject: [PATCH 1/5] t1500: be considerate to future potential tests The final batch of git-rev-parse tests work against a non-local object database named repo.git. This is done by renaming .git to repo.git and pointing GIT_DIR at it, but the name is never restored to .git at the end of the script, which can be problematic for tests added in the future. Be more friendly by instead making repo.git a copy of .git. Furthermore, make it clear that tests in repo.git will be independent from the results of earlier tests done in .git by initializing repo.git earlier in the test sequence. Likewise, bundle remaining preparation (such as directory creation) into a common setup test consistent with modern practice. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/t1500-rev-parse.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh index 48ee07779d..0194f54012 100755 --- a/t/t1500-rev-parse.sh +++ b/t/t1500-rev-parse.sh @@ -37,6 +37,11 @@ test_rev_parse() { ROOT=$(pwd) +test_expect_success 'setup' ' + mkdir -p sub/dir work && + cp -R .git repo.git +' + test_rev_parse toplevel false false true '' .git cd .git || exit 1 @@ -45,7 +50,6 @@ cd objects || exit 1 test_rev_parse .git/objects/ false true false '' "$ROOT/.git" cd ../.. || exit 1 -mkdir -p sub/dir || exit 1 cd sub/dir || exit 1 test_rev_parse subdirectory false false true sub/dir/ "$ROOT/.git" cd ../.. || exit 1 @@ -56,7 +60,6 @@ test_rev_parse 'core.bare = true' true false false git config --unset core.bare test_rev_parse 'core.bare undefined' false false true -mkdir work || exit 1 cd work || exit 1 GIT_DIR=../.git GIT_CONFIG="$(pwd)"/../.git/config @@ -71,7 +74,6 @@ test_rev_parse 'GIT_DIR=../.git, core.bare = true' true false false '' git config --unset core.bare test_rev_parse 'GIT_DIR=../.git, core.bare undefined' false false true '' -mv ../.git ../repo.git || exit 1 GIT_DIR=../repo.git GIT_CONFIG="$(pwd)"/../repo.git/config From 12f7526c66547ced1b670f5c0faeec5120fc9c7a Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Wed, 18 May 2016 16:15:42 -0400 Subject: [PATCH 2/5] t1500: test_rev_parse: facilitate future test enhancements Tests run by test_rev_parse() are nearly identical; each invokes git-rev-parse with a single option and compares the result against an expected value. Such duplication makes it onerous to extend the tests since any change needs to be repeated in each test. Avoid the duplication by parameterizing the test and driving it via a for-loop. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/t1500-rev-parse.sh | 44 +++++++++++++++++--------------------------- 1 file changed, 17 insertions(+), 27 deletions(-) diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh index 0194f54012..ecc575ba1d 100755 --- a/t/t1500-rev-parse.sh +++ b/t/t1500-rev-parse.sh @@ -3,38 +3,28 @@ test_description='test git rev-parse' . ./test-lib.sh -test_rev_parse() { +# usage: label is-bare is-inside-git is-inside-work prefix git-dir +test_rev_parse () { name=$1 shift - test_expect_success "$name: is-bare-repository" \ - "test '$1' = \"\$(git rev-parse --is-bare-repository)\"" - shift - [ $# -eq 0 ] && return - - test_expect_success "$name: is-inside-git-dir" \ - "test '$1' = \"\$(git rev-parse --is-inside-git-dir)\"" - shift - [ $# -eq 0 ] && return - - test_expect_success "$name: is-inside-work-tree" \ - "test '$1' = \"\$(git rev-parse --is-inside-work-tree)\"" - shift - [ $# -eq 0 ] && return - - test_expect_success "$name: prefix" \ - "test '$1' = \"\$(git rev-parse --show-prefix)\"" - shift - [ $# -eq 0 ] && return - - test_expect_success "$name: git-dir" \ - "test '$1' = \"\$(git rev-parse --git-dir)\"" - shift - [ $# -eq 0 ] && return + for o in --is-bare-repository \ + --is-inside-git-dir \ + --is-inside-work-tree \ + --show-prefix \ + --git-dir + do + test $# -eq 0 && break + expect="$1" + test_expect_success "$name: $o" ' + echo "$expect" >expect && + git rev-parse $o >actual && + test_cmp expect actual + ' + shift + done } -# label is-bare is-inside-git is-inside-work prefix git-dir - ROOT=$(pwd) test_expect_success 'setup' ' From 1e043cff7815786b3d1a4c07bac63b3d8e1e30ef Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Wed, 18 May 2016 16:15:43 -0400 Subject: [PATCH 3/5] t1500: avoid changing working directory outside of tests Ideally, each test should be responsible for setting up state it needs rather than relying upon transient global state. Toward this end, teach test_rev_parse() to accept a "-C " option to allow callers to instruct it explicitly in which directory its tests should be run. Take advantage of this new option to avoid changing the working directory outside of tests. Implementation note: test_rev_parse() passes "-C " along to git-rev-parse with properly quoted. The natural and POSIX way to do so is via ${dir:+-C "$dir"}, however, with some older broken shells, this expression evaluates incorrectly to a single argument ("-C ") rather than the expected two (-C and ""). Work around this problem with the slightly ungainly expression: ${dir:+-C} ${dir:+"$dir"} Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/t1500-rev-parse.sh | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh index ecc575ba1d..d73a52bc38 100755 --- a/t/t1500-rev-parse.sh +++ b/t/t1500-rev-parse.sh @@ -3,8 +3,18 @@ test_description='test git rev-parse' . ./test-lib.sh -# usage: label is-bare is-inside-git is-inside-work prefix git-dir +# usage: [options] label is-bare is-inside-git is-inside-work prefix git-dir test_rev_parse () { + d= + while : + do + case "$1" in + -C) d="$2"; shift; shift ;; + -*) error "test_rev_parse: unrecognized option '$1'" ;; + *) break ;; + esac + done + name=$1 shift @@ -18,7 +28,7 @@ test_rev_parse () { expect="$1" test_expect_success "$name: $o" ' echo "$expect" >expect && - git rev-parse $o >actual && + git ${d:+-C} ${d:+"$d"} rev-parse $o >actual && test_cmp expect actual ' shift @@ -34,15 +44,10 @@ test_expect_success 'setup' ' test_rev_parse toplevel false false true '' .git -cd .git || exit 1 -test_rev_parse .git/ false true false '' . -cd objects || exit 1 -test_rev_parse .git/objects/ false true false '' "$ROOT/.git" -cd ../.. || exit 1 +test_rev_parse -C .git .git/ false true false '' . +test_rev_parse -C .git/objects .git/objects/ false true false '' "$ROOT/.git" -cd sub/dir || exit 1 -test_rev_parse subdirectory false false true sub/dir/ "$ROOT/.git" -cd ../.. || exit 1 +test_rev_parse -C sub/dir subdirectory false false true sub/dir/ "$ROOT/.git" git config core.bare true test_rev_parse 'core.bare = true' true false false @@ -50,30 +55,29 @@ test_rev_parse 'core.bare = true' true false false git config --unset core.bare test_rev_parse 'core.bare undefined' false false true -cd work || exit 1 GIT_DIR=../.git -GIT_CONFIG="$(pwd)"/../.git/config +GIT_CONFIG="$(pwd)/work/../.git/config" export GIT_DIR GIT_CONFIG git config core.bare false -test_rev_parse 'GIT_DIR=../.git, core.bare = false' false false true '' +test_rev_parse -C work 'GIT_DIR=../.git, core.bare = false' false false true '' git config core.bare true -test_rev_parse 'GIT_DIR=../.git, core.bare = true' true false false '' +test_rev_parse -C work 'GIT_DIR=../.git, core.bare = true' true false false '' git config --unset core.bare -test_rev_parse 'GIT_DIR=../.git, core.bare undefined' false false true '' +test_rev_parse -C work 'GIT_DIR=../.git, core.bare undefined' false false true '' GIT_DIR=../repo.git -GIT_CONFIG="$(pwd)"/../repo.git/config +GIT_CONFIG="$(pwd)/work/../repo.git/config" git config core.bare false -test_rev_parse 'GIT_DIR=../repo.git, core.bare = false' false false true '' +test_rev_parse -C work 'GIT_DIR=../repo.git, core.bare = false' false false true '' git config core.bare true -test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' true false false '' +test_rev_parse -C work 'GIT_DIR=../repo.git, core.bare = true' true false false '' git config --unset core.bare -test_rev_parse 'GIT_DIR=../repo.git, core.bare undefined' false false true '' +test_rev_parse -C work 'GIT_DIR=../repo.git, core.bare undefined' false false true '' test_done From 1dea0dc9e0965e2581cdf64fc9eb072e8d6a88d3 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Wed, 18 May 2016 16:15:44 -0400 Subject: [PATCH 4/5] t1500: avoid setting configuration options outside of tests Ideally, each test should be responsible for setting up state it needs rather than relying upon transient global state. Toward this end, teach test_rev_parse() to accept a "-b " option to allow callers to set "core.bare" explicitly or undefine it. Take advantage of this new option to avoid setting "core.bare" outside of tests. Under the hood, "-b " invokes "test_config -C " (or "test_unconfig -C "), thus git-config knows explicitly where to find its configuration file. Consequently, the global GIT_CONFIG environment variable required by the manual git-config invocations outside of tests is no longer needed, and is thus dropped. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/t1500-rev-parse.sh | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh index d73a52bc38..325d8212cb 100755 --- a/t/t1500-rev-parse.sh +++ b/t/t1500-rev-parse.sh @@ -6,10 +6,15 @@ test_description='test git rev-parse' # usage: [options] label is-bare is-inside-git is-inside-work prefix git-dir test_rev_parse () { d= + bare= while : do case "$1" in -C) d="$2"; shift; shift ;; + -b) case "$2" in + [tfu]*) bare="$2"; shift; shift ;; + *) error "test_rev_parse: bogus core.bare value '$2'" ;; + esac ;; -*) error "test_rev_parse: unrecognized option '$1'" ;; *) break ;; esac @@ -27,6 +32,12 @@ test_rev_parse () { test $# -eq 0 && break expect="$1" test_expect_success "$name: $o" ' + case "$bare" in + t*) test_config ${d:+-C} ${d:+"$d"} core.bare true ;; + f*) test_config ${d:+-C} ${d:+"$d"} core.bare false ;; + u*) test_unconfig ${d:+-C} ${d:+"$d"} core.bare ;; + esac && + echo "$expect" >expect && git ${d:+-C} ${d:+"$d"} rev-parse $o >actual && test_cmp expect actual @@ -49,35 +60,25 @@ test_rev_parse -C .git/objects .git/objects/ false true false '' "$ROOT/.git" test_rev_parse -C sub/dir subdirectory false false true sub/dir/ "$ROOT/.git" -git config core.bare true -test_rev_parse 'core.bare = true' true false false +test_rev_parse -b t 'core.bare = true' true false false -git config --unset core.bare -test_rev_parse 'core.bare undefined' false false true +test_rev_parse -b u 'core.bare undefined' false false true GIT_DIR=../.git -GIT_CONFIG="$(pwd)/work/../.git/config" -export GIT_DIR GIT_CONFIG +export GIT_DIR -git config core.bare false -test_rev_parse -C work 'GIT_DIR=../.git, core.bare = false' false false true '' +test_rev_parse -C work -b f 'GIT_DIR=../.git, core.bare = false' false false true '' -git config core.bare true -test_rev_parse -C work 'GIT_DIR=../.git, core.bare = true' true false false '' +test_rev_parse -C work -b t 'GIT_DIR=../.git, core.bare = true' true false false '' -git config --unset core.bare -test_rev_parse -C work 'GIT_DIR=../.git, core.bare undefined' false false true '' +test_rev_parse -C work -b u 'GIT_DIR=../.git, core.bare undefined' false false true '' GIT_DIR=../repo.git -GIT_CONFIG="$(pwd)/work/../repo.git/config" -git config core.bare false -test_rev_parse -C work 'GIT_DIR=../repo.git, core.bare = false' false false true '' +test_rev_parse -C work -b f 'GIT_DIR=../repo.git, core.bare = false' false false true '' -git config core.bare true -test_rev_parse -C work 'GIT_DIR=../repo.git, core.bare = true' true false false '' +test_rev_parse -C work -b t 'GIT_DIR=../repo.git, core.bare = true' true false false '' -git config --unset core.bare -test_rev_parse -C work 'GIT_DIR=../repo.git, core.bare undefined' false false true '' +test_rev_parse -C work -b u 'GIT_DIR=../repo.git, core.bare undefined' false false true '' test_done From e6273f4da51287363137a24200dd43b87c801b3d Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Wed, 18 May 2016 16:15:45 -0400 Subject: [PATCH 5/5] t1500: avoid setting environment variables outside of tests Ideally, each test should be responsible for setting up state it needs rather than relying upon transient global state. Toward this end, teach test_rev_parse() to accept a "-g " option to allow callers to specify the value of the GIT_DIR environment variable explicitly. Take advantage of this new option to avoid polluting the global scope with GIT_DIR assignments. Implementation note: Typically, tests avoid polluting the global state by wrapping transient environment variable assignments within a subshell, however, this technique doesn't work here since test_config() and test_unconfig() need to know GIT_DIR, as well, but neither function can be used within a subshell. Consequently, GIT_DIR is instead cleared manually via test_when_finished(). Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/t1500-rev-parse.sh | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh index 325d8212cb..038e24c401 100755 --- a/t/t1500-rev-parse.sh +++ b/t/t1500-rev-parse.sh @@ -7,6 +7,7 @@ test_description='test git rev-parse' test_rev_parse () { d= bare= + gitdir= while : do case "$1" in @@ -15,6 +16,7 @@ test_rev_parse () { [tfu]*) bare="$2"; shift; shift ;; *) error "test_rev_parse: bogus core.bare value '$2'" ;; esac ;; + -g) gitdir="$2"; shift; shift ;; -*) error "test_rev_parse: unrecognized option '$1'" ;; *) break ;; esac @@ -32,6 +34,13 @@ test_rev_parse () { test $# -eq 0 && break expect="$1" test_expect_success "$name: $o" ' + if test -n "$gitdir" + then + test_when_finished "unset GIT_DIR" && + GIT_DIR="$gitdir" && + export GIT_DIR + fi && + case "$bare" in t*) test_config ${d:+-C} ${d:+"$d"} core.bare true ;; f*) test_config ${d:+-C} ${d:+"$d"} core.bare false ;; @@ -64,21 +73,18 @@ test_rev_parse -b t 'core.bare = true' true false false test_rev_parse -b u 'core.bare undefined' false false true -GIT_DIR=../.git -export GIT_DIR -test_rev_parse -C work -b f 'GIT_DIR=../.git, core.bare = false' false false true '' +test_rev_parse -C work -g ../.git -b f 'GIT_DIR=../.git, core.bare = false' false false true '' -test_rev_parse -C work -b t 'GIT_DIR=../.git, core.bare = true' true false false '' +test_rev_parse -C work -g ../.git -b t 'GIT_DIR=../.git, core.bare = true' true false false '' -test_rev_parse -C work -b u 'GIT_DIR=../.git, core.bare undefined' false false true '' +test_rev_parse -C work -g ../.git -b u 'GIT_DIR=../.git, core.bare undefined' false false true '' -GIT_DIR=../repo.git -test_rev_parse -C work -b f 'GIT_DIR=../repo.git, core.bare = false' false false true '' +test_rev_parse -C work -g ../repo.git -b f 'GIT_DIR=../repo.git, core.bare = false' false false true '' -test_rev_parse -C work -b t 'GIT_DIR=../repo.git, core.bare = true' true false false '' +test_rev_parse -C work -g ../repo.git -b t 'GIT_DIR=../repo.git, core.bare = true' true false false '' -test_rev_parse -C work -b u 'GIT_DIR=../repo.git, core.bare undefined' false false true '' +test_rev_parse -C work -g ../repo.git -b u 'GIT_DIR=../repo.git, core.bare undefined' false false true '' test_done