Re: [PATCH 4/7] t7450: test verify_path() handling of gitmodules

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 5, 2020, 08:38 UTC
Message-ID: <20201005083814.GM1166820@google.com>
In-Reply-To: <20201005083004.GC2862927@coredump.intra.peff.net>

Jeff King wrote:

> On Mon, Oct 05, 2020 at 12:53:11AM -0700, Jonathan Nieder wrote:
>> Jeff King wrote:

Show 29 quoted lines

>>> @@ -155,8 +155,14 @@ test_expect_success 'fsck detects symlinked .gitmodules file' '
>>>  		{
>>>  			printf "100644 blob $content\t$tricky\n" &&
>>>  			printf "120000 blob $target\t.gitmodules\n"
>>> -		} >bad-tree &&
>>> -		tree=$(git mktree <bad-tree) &&
>>> +		} >bad-tree
>>> +	) &&
>>> +	tree=$(git -C symlink mktree <symlink/bad-tree)
>>> +'
>>
>> This is super nitpicky, but: test scripts can be hard to maintain when
>> there's this kind of state carried from assertion to assertion without
>> it being made obvious.
>>
>> Can this include "setup" or "set up" in the name to do that?  E.g.
>>
>> 	test_expect_success 'set up repo with symlinked .gitmodules file' '
>> 		...
>> 	'
>
> Hmph. I specifically _tried_ to do that by breaking it into a separate
> test with the name "create" in it, which I thought was one of the
> code-words for "I'm doing stuff that will be used in another test". But
> I guess there's no official rule on that. I dug up:
>
>   https://lore.kernel.org/git/20130826173501.GS4110@google.com/
>
> but I guess I mis-remembered "create" being present there.

I can try to find some time today to introduce a test_setup helper. Having to figure out and rely on this kind of ad hoc convention is not something I really want to ask of patch authors and reviewers.

The reason I find "set up" clearer than "create" is that the latter is something I can easily imagine myself genuinely wanting to test. "Set up for a later test" is more explicit about what the commands are being run for.

Jonathan