On Tue, Mar 09 2021, Jeff King wrote:
On Mon, Mar 08, 2021 at 09:04:25PM +0100, Ævar Arnfjörð Bjarmason wrote:
quoted
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.