From: Junio C Hamano Date: Tue, 28 Sep 2010 15:36:46 GMT Subject: Re: [PATCH v3 1/3] tests: Prepare --textconv tests for correctly-failing conversion program Message-ID: <7vlj6lubg1.fsf@alter.siamese.dyndns.org> In-Reply-To: <20100928120722.GA29525@landau.phys.spbu.ru> Kirill Smelkov writes: > On Mon, Sep 27, 2010 at 11:23:35AM -0700, Junio C Hamano wrote: >> Kirill Smelkov writes: > > >> > +sed 's/^bin:/converted:/' "$@" >helper.out >> >> Minor nit: this is inconsistent with the check done with grep above that >> insists that the colon is followed by a SP. > > Yes, you are right. I'll amend it. > >> > diff --git a/t/t4042-diff-textconv-caching.sh b/t/t4042-diff-textconv-caching.sh >> > index 91f8198..7668099 100755 >> > --- a/t/t4042-diff-textconv-caching.sh >> > +++ b/t/t4042-diff-textconv-caching.sh >> > @@ -5,18 +5,19 @@ test_description='test textconv caching' >> > >> > cat >helper <<'EOF' >> > #!/bin/sh >> > -sed 's/^/converted: /' "$@" >helper.out >> > +grep -q '^bin: ' "$@" || { echo "E: $@ is not \"binary\" file" 1>&2; exit 1; } >> ... > ... > I too think (1) is right. It was just that originally there was $@ > (which I now understand was wrong). Well, the original's use of "$@" is perfectly fine; it would do the right thing with one argument, of course, but it would do the right thing with more than one, too. On the other hand, your use inside "echo" is not.