Re: [PATCH 2/7] t: introduce tests for unexpected object types

From: Jeff King <peff@peff.net>
Date: Apr 5, 2019, 18:24 UTC
Message-ID: <20190405182412.GC2284@sigill.intra.peff.net>
In-Reply-To: <20190405105033.GT32732@szeder.dev>

On Fri, Apr 05, 2019 at 12:50:33PM +0200, SZEDER Gábor wrote:

> > +test_expect_failure 'traverse unexpected non-tree entry (seen)' '
> > +	test_must_fail git rev-list --objects $blob $broken_tree >output 2>&1
> 
> This test saves standard output and error, but doesn't look at them.

I think we want to be checking for "not a tree" here, which is later added with the fix. But either we should have the test_i18ngrep here initially, or we should add both the redirect and the grep with the fix.

Show 7 quoted lines

> > +test_expect_success 'setup unexpected non-commit parent' '
> > +	git cat-file commit $commit |
> > +		perl -lpe "/^author/ && print q(parent $blob)" \
> > +		>broken-commit &&
> 
> Don't run git commands upstream of a pipe, because the pipe hides
> their exit code.  This applies to several other tests below as well.

I disagree with that rule here. We're not testing "cat-file" in any meaningful way, but just getting some stock output from a known-good commit.

> Wouldn't a 'sed' one-liner suffice, so we won't have yet another perl
> dependency?

Heh, this was actually the subject of much discussion before the patches hit the list. If you can write such a one-liner that is both readable and portable, please share it. I got disgusted with sed and suggested this perl.

-Peff