Re: [PATCH 2/7] t: introduce tests for unexpected object types
- Date: Fri, 5 Apr 2019 20:42:29 +0200
- From: SZEDER Gábor <szeder.dev@xxxxxxxxx>
- Subject: Re: [PATCH 2/7] t: introduce tests for unexpected object types
On Fri, Apr 05, 2019 at 02:24:12PM -0400, Jeff King wrote:
> 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.
> > > +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
It makes auditing harder and sets bad example.
> > 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.
Hm, so the trivial s/// with '\n' in the replacement part is not
portable, then? Oh, well.