Skip to content

Commit

Permalink
t: use and remove single test directory
Browse files Browse the repository at this point in the history
In commit e1c2c8a of PR git-lfs#1107 we
revised our test suite 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
prefix pattern of "git-lfs_TEMP".  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 the variable
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 if the KEEPTRASH variable was empty or unset.
This ensured that, in the normal case, we removed all of our
test artifacts after completing our test scripts.

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 commit, the "test" target's recipe in 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() made prior to running "prove".  Finally, the "test"
Makefile target recpie 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, the 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 that handled the deletion
of the directory specified by the GIT_LFS_TEST_DIR variable.

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.

In our CI jobs, we run our 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 now what is stored in
our GIT_LFS_TEST_DIR environment 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 tests in our t/t-env.sh and
t/t-workflow.sh test scripts are expecting a Unix-style path beginning
with /tmp.

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 environment 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.
  • Loading branch information
chrisd8088 committed Oct 5, 2024
1 parent 9d98037 commit 4130136
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 7 deletions.
6 changes: 3 additions & 3 deletions t/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ test-commands : $(TEST_CMDS)

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

.PHONY : $(TEST_SRCS)
$(TEST_SRCS) : $(TEST_CMDS)
Expand Down
4 changes: 4 additions & 0 deletions t/t-env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ envInitConfig='git config filter.lfs.process = "git-lfs filter-process"
git config filter.lfs.smudge = "git-lfs smudge -- %f"
git config filter.lfs.clean = "git-lfs clean -- %f"'

if [ "$IS_WINDOWS" -eq 1 ]; then
export MSYS2_ENV_CONV_EXCL="GIT_LFS_TEST_DIR"
fi

begin_test "env with no remote"
(
set -e
Expand Down
4 changes: 4 additions & 0 deletions t/t-worktree.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ envInitConfig='git config filter.lfs.process = "git-lfs filter-process"
git config filter.lfs.smudge = "git-lfs smudge -- %f"
git config filter.lfs.clean = "git-lfs clean -- %f"'

if [ "$IS_WINDOWS" -eq 1 ]; then
export MSYS2_ENV_CONV_EXCL="GIT_LFS_TEST_DIR"
fi

begin_test "git worktree"
(
set -e
Expand Down
13 changes: 9 additions & 4 deletions t/testenv.sh
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ resolve_symlink() {
else
readlink -f "$arg"
fi

}

# The root directory for the git-lfs repository by default.
Expand All @@ -84,11 +83,17 @@ else
fi

if [ -z "$GIT_LFS_TEST_DIR" ]; then
GIT_LFS_TEST_DIR=$(mktemp -d -t "$TEMPDIR_PREFIX")
GIT_LFS_TEST_DIR=$(resolve_symlink $GIT_LFS_TEST_DIR)
GIT_LFS_TEST_DIR="$(mktemp -d -t "$TEMPDIR_PREFIX")"
GIT_LFS_TEST_DIR="$(resolve_symlink "$GIT_LFS_TEST_DIR")"
# cleanup either after single test or at end of integration (except on fail)
RM_GIT_LFS_TEST_DIR=yes
RM_GIT_LFS_TEST_DIR="yes"
fi

# Make these variables available to all test files run in the same shell,
# particularly when setup() is run first by itself to start a single
# common lfstest-gitserver instance.
export GIT_LFS_TEST_DIR RM_GIT_LFS_TEST_DIR

# create a temporary work space
TMPDIR=$GIT_LFS_TEST_DIR

Expand Down
5 changes: 5 additions & 0 deletions t/testhelpers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,11 @@ shutdown() {
LFSTEST_DIR="$REMOTEDIR" \
LFS_URL_FILE="$LFS_URL_FILE" \
lfstest-count-tests decrement

# delete entire lfs test root if we created it (double check pattern)
if [ -z "$KEEPTRASH" ] && [ "$RM_GIT_LFS_TEST_DIR" = "yes" ] && [[ $GIT_LFS_TEST_DIR == *"$TEMPDIR_PREFIX"* ]]; then
rm -rf "$GIT_LFS_TEST_DIR"
fi
}

tap_show_plan() {
Expand Down

0 comments on commit 4130136

Please sign in to comment.