Re: [PATCH v4 1/9] submodule absorbgitdirs tests: add missing "Migrating git..." tests

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Dec 20, 2022, 10:32 UTC
Message-ID: <221220.86a63ixt24.gmgdl@evledraar.gmail.com>
In-Reply-To: <kl6l7cyse6bi.fsf@chooglen-macbookpro.roam.corp.google.com>

On Thu, Dec 15 2022, Glen Choo wrote:

Show 48 quoted lines

> No comment on the structure of the tests themselves; those look good to
> me.
>
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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.

Show 26 quoted lines

>> @@ -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.