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 <dir>" 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 <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 325d8212cb8f236216b0275a53849c80d9e76632..038e24c401451d61e9c93fb5c233d132a3d51acd 100755 (executable)
--- 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