[PATCH 1/4] t5510: make confusing config cleanup more explicit
- From
Jeff King <peff@peff.net>
- Date
- Aug 19, 2025, 19:24 UTC
- Message-ID
- <20250819192455.GA1059295@coredump.intra.peff.net>
- In-Reply-To
- <20250819192004.GA1058857@coredump.intra.peff.net>
Several tests set a config variable in a sub-repo we chdir into via a subshell, like this:
( cd "$D" && cd two && git config foo.bar baz )
But they also clean up the variable with a when_finished directive outside of the subshell, like this:
test_when_finished "git config unset foo.bar"
At first glance, this shouldn't work! The cleanup clause cannot be run from the subshell (since environment changes there are lost by the time the test snippet finishes). But since the cleanup command runs outside the subshell, our working directory will not have been switched into "two".
But it does work. Why?
The answer is that an earlier test does a "cd two" that moves the whole test's working directory out of $TRASH_DIRECTORY and into "two". So the subshell is a bit of a red herring; we are already in the right directory! That's why we need the "cd $D" at the top of the shell, to put us back to a known spot.
Let's make this cleanup code more explicitly specify where we expect the config command to run. That makes the script more robust against running a subset of the tests, and ultimately will make it easier to refactor the script to avoid these top-level chdirs.
Signed-off-by: Jeff King <peff@peff.net> --- t/t5510-fetch.sh | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index ebc696546b..64fea9f4a5 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -119,7 +119,7 @@ test_expect_success "fetch test remote HEAD change" ' test "z$head" = "z$branch"' test_expect_success "fetch test followRemoteHEAD never" ' - test_when_finished "git config unset remote.origin.followRemoteHEAD" && + test_when_finished "git -C \"$D/two\" config unset remote.origin.followRemoteHEAD" && ( cd "$D" && cd two && @@ -134,7 +134,7 @@ test_expect_success "fetch test followRemoteHEAD never" ' ' test_expect_success "fetch test followRemoteHEAD warn no change" ' - test_when_finished "git config unset remote.origin.followRemoteHEAD" && + test_when_finished "git -C \"$D/two\" config unset remote.origin.followRemoteHEAD" && ( cd "$D" && cd two && @@ -154,7 +154,7 @@ test_expect_success "fetch test followRemoteHEAD warn no change" ' ' test_expect_success "fetch test followRemoteHEAD warn create" ' - test_when_finished "git config unset remote.origin.followRemoteHEAD" && + test_when_finished "git -C \"$D/two\" config unset remote.origin.followRemoteHEAD" && ( cd "$D" && cd two && @@ -170,7 +170,7 @@ test_expect_success "fetch test followRemoteHEAD warn create" ' ' test_expect_success "fetch test followRemoteHEAD warn detached" ' - test_when_finished "git config unset remote.origin.followRemoteHEAD" && + test_when_finished "git -C \"$D/two\" config unset remote.origin.followRemoteHEAD" && ( cd "$D" && cd two && @@ -187,7 +187,7 @@ test_expect_success "fetch test followRemoteHEAD warn detached" ' ' test_expect_success "fetch test followRemoteHEAD warn quiet" ' - test_when_finished "git config unset remote.origin.followRemoteHEAD" && + test_when_finished "git -C \"$D/two\" config unset remote.origin.followRemoteHEAD" && ( cd "$D" && cd two && @@ -205,7 +205,7 @@ test_expect_success "fetch test followRemoteHEAD warn quiet" ' ' test_expect_success "fetch test followRemoteHEAD warn-if-not-branch branch is same" ' - test_when_finished "git config unset remote.origin.followRemoteHEAD" && + test_when_finished "git -C \"$D/two\" config unset remote.origin.followRemoteHEAD" && ( cd "$D" && cd two && @@ -223,7 +223,7 @@ test_expect_success "fetch test followRemoteHEAD warn-if-not-branch branch is sa ' test_expect_success "fetch test followRemoteHEAD warn-if-not-branch branch is different" ' - test_when_finished "git config unset remote.origin.followRemoteHEAD" && + test_when_finished "git -C \"$D/two\" config unset remote.origin.followRemoteHEAD" && ( cd "$D" && cd two && @@ -243,7 +243,7 @@ test_expect_success "fetch test followRemoteHEAD warn-if-not-branch branch is di ' test_expect_success "fetch test followRemoteHEAD always" ' - test_when_finished "git config unset remote.origin.followRemoteHEAD" && + test_when_finished "git -C \"$D/two\" config unset remote.origin.followRemoteHEAD" && ( cd "$D" && cd two && @@ -260,7 +260,7 @@ test_expect_success "fetch test followRemoteHEAD always" ' ' test_expect_success 'followRemoteHEAD does not kick in with refspecs' ' - test_when_finished "git config unset remote.origin.followRemoteHEAD" && + test_when_finished "git -C \"$D/two\" config unset remote.origin.followRemoteHEAD" && ( cd "$D" && cd two &&
-- 2.51.0.326.gecbb38d78e