Skip to content

Commit e0daac0

Browse files
committed
t: use and remove single test directory
In commit e1c2c8a of PR git-lfs#1107 we revised the test to use a separate test directory, either a temporary one we create or one specified in advance in a GIT_LFS_TEST_DIR environment variable. If that variable is empty or unset, we populate it with the output of a "mktemp -d" command with a directory name pattern of "git-lfs_TEMP". In either case, we then create all of our test repositories in subdirectories of the location given by GIT_LFS_TEST_DIR. If we create a directory because GIT_LFS_TEST_DIR is empty or unset, we also set the RM_GIT_LFS_TEST_DIR variable with a flag value of "yes". As of commit e1c2c8a, we would then check that variable in the shutdown() shell function, and if it was set to "yes", we would then remove the directory specified by the GIT_LFS_TEST_DIR variable if it matched the pattern we use for temporary directories and the KEEPTRASH variable was empty or unset. This ensured that, in the normal case, we removed all of our test artifacts after completing the test suite. In commit 1926e94 of PR git-lfs#3125 we then migrated our test suite to use the "prove" command, and introduced the lfstest-count-tests utility to help control a common instance of our lfstest-gitserver program, which can be shared and reused by all the individual test scripts. In that PR we also moved our test suite into the "t" directory, and added a Makefile with a "test" target that runs the "prove" command over all the individual test scripts; this is still the framework we use today. In that commmit, the "test" target of the t/Makefile file was written so it first loads the t/testenv.sh script and then runs the setup() shell function, which calls "lfstest-count-tests increment", which starts the lfstest-gitserver program because it increments the test count from zero to one. Every test script run by the "prove" command then calls setup() again, which further increments the test count, and then as it exits it calls the shutdown() shell function, which runs the "lfstest-count-tests decrement" command. That never decrements the test count below one, however, because of the initial call to setup() prior to running "prove". Finally, the "test" Makefile target runs shutdown() after "prove" is complete, which decrements the test count to zero, at which point lfstest-count-tests sends a shutdown POST HTTP request to the running lfstest-gitserver instance. In this design, though, each individual test script loads the t/testenv.sh script, and the script is directly loaded twice by the "test" Makefile target's recipe. In each case, it script runs in a separate shell session, and so does not find a GIT_LFS_TEST_DIR environment variable (unless one is set in the user's shell session when running "make test"). Thus each invocation of t/testenv.sh results in the creation of a new temporary test directory. Commit 1926e94 also removed the logic from the shutdown() shell function which handled the deletion of the directory specified by the GIT_LFS_TEST_DIR variable, if the RM_GIT_LFS_TEST_DIR variable was set to "yes" and we had clearly created a temporary directory to populate GIT_LFS_TEST_DIR. The consequence is that when we run an individual test script by itself, it leaves a single temporary test directory behind, and when we run the "make test" Makefile target, every t/t-*.sh script leaves a test directory behind, plus two additional ones created when the recipe loads the t/testenv.sh script directly. We can improve this situation by restoring the logic to clean up the directory specified by the GIT_LFS_TEST_DIR variable (if the RM_GIT_LFS_TEST_DIR variable contains "yes" and the other conditions described above are met), and then adjusting our "test" target's recipe so it runs all of its key steps in a single Bash shell session. We also make sure to export the GIT_LFS_TEST_DIR and RM_GIT_LFS_TEST_DIR variables into the environment, so that when t/testenv.sh is loaded by the recipe the first time, it creates a temporary directory and exports those variables, which ensures they are then available to all the subsequent test scripts and the recipe's final call to shutdown(). By making these changes, we ensure that in the normal case, when GIT_LFS_TEST_DIR is empty or unset and we run "make test", we create only a single temporary test directory, use it in all the individual test scripts, and then remove it completely afterwards. In our t/t-env.sh and t/t-workflow.sh test scripts, we check the output of the "git lfs env" command in a number of tests, comparing it to the values we expect based on the test conditions, including the values of all environment variables whose names begin with the "GIT_" prefix. As we now export the GIT_LFS_TEST_DIR variable into the environment, it is among these environment variables, which causes the tests to fail on Windows unless we make one additional change. We run our CI test suite on Windows using the Git Bash environment provided by the Git for Windows project, which is in turn based on the MSYS2 environment. In this context, the mktemp(1) command returns a path beginning with /tmp, so that is what is stored in our GIT_LFS_TEST_DIR variable. However, MSYS2 translates such Unix-style paths into actual Windows filesystem paths when it passes command-line arguments and environment variables to any programs it executes, including our Git LFS client. Thus the "git lfs env" command returns a Windows-style path for the GIT_LFS_TEST_DIR variable, while our Bash test scripts are expecting a Unix-style path beginning with /tmp, and so many of our tests in the t/t-env.sh and t/t-workflow.sh test scripts fail. To resolve this, when running on Windows, we set the MSYS2_ENV_CONV_EXCL environment variable at the top of these scripts to contain the name of the GIT_LFS_TEST_DIR variable. As described in the MSYS2 documentation, this ensures MSYS2 does not alter the value of the GIT_LFS_TEST_DIR variable: https://www.msys2.org/docs/filesystem-paths/#environment-variables Finally, we take the opportunity to ensure that the command substitutions we use to set the GIT_LFS_TEST_DIR environment variable are fully quoted, and to clean up some nearby excess whitespace.
1 parent 42ed74d commit e0daac0

File tree

5 files changed

+25
-8
lines changed

5 files changed

+25
-8
lines changed

t/Makefile

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@ test-commands : $(TEST_CMDS)
4545

4646
test : test-commands
4747
$(RM) -r remote test_count{,.lock}
48-
@bash -c '. ./testenv.sh && setup'
49-
$(PROVE) $(PROVE_EXTRA_ARGS) ./t-*.sh
50-
@bash -c '. ./testenv.sh && shutdown'
48+
@bash -c ". ./testenv.sh && setup && cd t && \
49+
RM_GIT_LFS_TEST_DIR=no $(PROVE) $(PROVE_EXTRA_ARGS) t-*.sh && \
50+
shutdown"
5151

5252
.PHONY : $(TEST_SRCS)
5353
$(TEST_SRCS) : $(TEST_CMDS)

t/t-env.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ envInitConfig='git config filter.lfs.process = "git-lfs filter-process"
66
git config filter.lfs.smudge = "git-lfs smudge -- %f"
77
git config filter.lfs.clean = "git-lfs clean -- %f"'
88

9+
if [ $IS_WINDOWS -eq 1 ]; then
10+
export MSYS2_ENV_CONV_EXCL="GIT_LFS_TEST_DIR"
11+
fi
12+
913
begin_test "env with no remote"
1014
(
1115
set -e

t/t-worktree.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ envInitConfig='git config filter.lfs.process = "git-lfs filter-process"
77
git config filter.lfs.smudge = "git-lfs smudge -- %f"
88
git config filter.lfs.clean = "git-lfs clean -- %f"'
99

10+
if [ $IS_WINDOWS -eq 1 ]; then
11+
export MSYS2_ENV_CONV_EXCL="GIT_LFS_TEST_DIR"
12+
fi
13+
1014
begin_test "git worktree"
1115
(
1216
set -e

t/testenv.sh

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ resolve_symlink() {
6262
else
6363
readlink -f "$arg"
6464
fi
65-
6665
}
6766

6867
# The root directory for the git-lfs repository by default.
@@ -83,12 +82,17 @@ else
8382
TEMPDIR_PREFIX="git-lfs_TEMP.XXXXXX"
8483
fi
8584
if [ -z "$GIT_LFS_TEST_DIR" ]; then
86-
87-
GIT_LFS_TEST_DIR=$(mktemp -d -t "$TEMPDIR_PREFIX")
88-
GIT_LFS_TEST_DIR=$(resolve_symlink $GIT_LFS_TEST_DIR)
85+
GIT_LFS_TEST_DIR="$(mktemp -d -t "$TEMPDIR_PREFIX")"
86+
GIT_LFS_TEST_DIR="$(resolve_symlink "$GIT_LFS_TEST_DIR")"
8987
# cleanup either after single test or at end of integration (except on fail)
90-
RM_GIT_LFS_TEST_DIR=yes
88+
RM_GIT_LFS_TEST_DIR="yes"
9189
fi
90+
91+
# Make these variables available to all test files run in the same shell,
92+
# particularly when setup() is run first by itself to start a single
93+
# common lfstest-gitserver instance.
94+
export GIT_LFS_TEST_DIR RM_GIT_LFS_TEST_DIR
95+
9296
# create a temporary work space
9397
TMPDIR=$GIT_LFS_TEST_DIR
9498

t/testhelpers.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,11 @@ shutdown() {
647647
LFSTEST_DIR="$REMOTEDIR" \
648648
LFS_URL_FILE="$LFS_URL_FILE" \
649649
lfstest-count-tests decrement
650+
651+
# delete entire lfs test root if we created it (double check pattern)
652+
if [ -z "$KEEPTRASH" ] && [ "$RM_GIT_LFS_TEST_DIR" = "yes" ] && [[ $GIT_LFS_TEST_DIR == *"$TEMPDIR_PREFIX"* ]]; then
653+
rm -rf "$GIT_LFS_TEST_DIR"
654+
fi
650655
}
651656

652657
tap_show_plan() {

0 commit comments

Comments
 (0)