On Thu, Dec 15 2022, Glen Choo wrote: > No comment on the structure of the tests themselves; those look good to > me. > > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >> diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh >> index 2859695c6d2..d556342ea57 100755 >> --- a/t/t7412-submodule-absorbgitdirs.sh >> +++ b/t/t7412-submodule-absorbgitdirs.sh >> @@ -10,6 +10,7 @@ TEST_PASSES_SANITIZE_LEAK=true >> . ./test-lib.sh >> >> test_expect_success 'setup a real submodule' ' >> + cwd="$(pwd)" && >> git init sub1 && >> test_commit -C sub1 first && >> git submodule add ./sub1 && > > [...] > >> @@ -18,13 +19,21 @@ test_expect_success 'setup a real submodule' ' >> ' >> >> test_expect_success 'absorb the git dir' ' >> + >expect && >> + >actual && >> >expect.1 && >> >expect.2 && >> >actual.1 && >> >actual.2 && >> git status >expect.1 && >> git -C sub1 rev-parse HEAD >expect.2 && >> - git submodule absorbgitdirs && >> + cat >expect <<-EOF && >> + Migrating git directory of '\''sub1'\'' from >> + '\''$cwd/sub1/.git'\'' to >> + '\''$cwd/.git/modules/sub1'\'' >> + EOF >> + git submodule absorbgitdirs 2>actual && >> + test_cmp expect actual && >> git fsck && >> test -f sub1/.git && >> test -d .git/modules/sub1 && > > I thought that we typically avoid setting environment variables in the > test cases themselves, so when we set environment variables to be read > in later tests, we typically set them outside of the test case (e.g. > t/t5526-fetch-submodules.sh). We could do it either way, but no, I think the preferred style is to do such setup/assignment in a test_expect_success, we don't run our tests as "set -e", so we'd miss any errors (however unlikely in this case) from the commands outside test bodies. See e.g. t0002-gitfile.sh for the same pattern, i.e. setting the "$REAL" variable in the setup "test_expect_success", then using it later. >> @@ -97,6 +119,27 @@ test_expect_success 'absorb the git dir in a nested submodule' ' >> test_cmp expect.2 actual.2 >> ' >> >> +test_expect_success 'absorb the git dir outside of primary worktree' ' >> + test_when_finished "rm -rf repo-bare.git" && >> + git clone --bare . repo-bare.git && >> + test_when_finished "rm -rf repo-wt" && >> + git -C repo-bare.git worktree add ../repo-wt && >> + >> + test_when_finished "rm -f .gitconfig" && >> + test_config_global protocol.file.allow always && >> + git -C repo-wt submodule update --init && >> + git init repo-wt/sub2 && >> + test_commit -C repo-wt/sub2 A && >> + git -C repo-wt submodule add ./sub2 sub2 && >> + cat >expect <<-EOF && >> + Migrating git directory of '\''sub2'\'' from >> + '\''$cwd/repo-wt/sub2/.git'\'' to >> + '\''$cwd/repo-bare.git/worktrees/repo-wt/modules/sub2'\'' >> + EOF >> + DO_IT=1 git -C repo-wt submodule absorbgitdirs 2>actual && > > DO_IT is a leftover from dev? > > (I'm also curious as to what it does :)). Oops, will fix! It was just something for ad-hoc getenv()-debugging that escaped the lab.
