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

From: Jeff King <peff@peff.net>
Date: Oct 5, 2020, 08:30 UTC
Message-ID: <20201005083004.GC2862927@coredump.intra.peff.net>
In-Reply-To: <20201005075311.GG1166820@google.com>

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

Show 20 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.

-Peff