Subject: Re: [PATCH 6/7] object tests: add test for unexpected objects in tags

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Sun, 28 Mar 2021 03:35:46 +0200

On Tue, Mar 09 2021, Jeff King wrote:

> On Mon, Mar 08, 2021 at 09:04:25PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> Fix a blind spot in the tests added in 0616617c7e1 (t: introduce tests
>> for unexpected object types, 2019-04-09), there were no meaningful
>> tests for checking how we reported on finding the incorrect object
>> type in a tag, i.e. one that broke the "type" promise in the tag
>> header.
>
> Isn't this covered by tests 16 and 17 ("traverse unexpected non-commit
> tag", both "lone" and "seen")? And likewise the matching "non-tree" and
> "non-blob" variants afterwards?

Barely, those tests are mainly testing that rev-list doesn't die, and
only do a very fuzzy match on the output. E.g. checking `grep "not a
commit" err`, not a full test_cmp that'll check what OID is reported
etc.

> The only thing we don't seem to cover is an unexpected non-tag. I don't
> mind adding that, but why wouldn't we just follow the template of the
> existing tests?

I am following that template to some extent, e.g. using
${commit,tree,blob}. It just didn't seem worth it to refactor an earlier
test in the file just to re-use a single hash-object invocation, those
tests e.g. clobber the $tag variable, so bending over backwards to
re-use anything set up in them would mean some refactoring.

I think it's much clearer just do do all the different kinds of setup in
the new setup function.