Re: [PATCHv2] write_index: optionally allow broken null sha1s

From: Jeff King <peff@peff.net>
Date: Aug 26, 2013, 14:27 UTC
Message-ID: <20130826142704.GA14858@sigill.intra.peff.net>
In-Reply-To: <20130825195412.GA2752@elie.Belkin>

On Sun, Aug 25, 2013 at 12:54:12PM -0700, Jonathan Nieder wrote:

Show 6 quoted lines

> [setup split across three tests]
>
> This is kind of an old-fashioned test, since each step of the setup is
> treated as a separate test assertion.  I don't really mind until we
> get better automation to make it easy to skip or rearrange tests.
> Just for reference, I think the usual way to do this now is

I don't see that splitting it up more hurts this. If we wanted more automatic rearranging or skipping of tests, we would need tests to declare dependencies on their setup. And we would need to be able to declare dependencies on multiple tests, since having a single setup test does not work in all cases (e.g., sometimes you are testing each step, and the final step relies on earlier steps).

And I do think splitting it up has more granularity. For example, in the sequence here:

Show 14 quoted lines

> 	test_expect_success 'setup' '
> 		# create base commits
> 		...
> 
> 		# create a commit with bogus null sha1 in the tree
> 		...
> 
> 		# We have to make one more commit on top removing the broken
> 		# entry, since otherwise our index does not match HEAD and
> 		# filter-branch will complain. We could make the index match
> 		# HEAD, but doing so would involve writing a null sha1 into
> 		# the index.
> 		...
> 	'

Right now it is not hard to do step 2. But I could easily imagine a world in which git-mktree learns to complain about such bogus trees. And the sequence would become:

  1. create base commits

  2. check that mktree barfs

  3. check that we can override mktree to create a broken tree

  4. clean up tip state

in which case you really want to have individual tests.

I do not care _that_ much, since mktree does not behave that way now, and the setup is otherwise not that likely to fail. But I do not think more granularity actually hurts us, and it can sometimes help (as described above, but also when something fails, the test output more clearly pinpoints what happened).

-Peff