Re: [PATCH v3 1/3] tests: Prepare --textconv tests for correctly-failing conversion program
- From
Kirill Smelkov <kirr@landau.phys.spbu.ru>
- Date
- Sep 28, 2010, 12:07 UTC
- Message-ID
- <20100928120722.GA29525@landau.phys.spbu.ru>
- In-Reply-To
- <7vsk0vyriw.fsf@alter.siamese.dyndns.org>
On Mon, Sep 27, 2010 at 11:23:35AM -0700, Junio C Hamano wrote:
> Kirill Smelkov <kirr@landau.phys.spbu.ru> 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.
Show 44 quoted lines
> > 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; }
>
> You are not feeding arguments you think you are to the above "echo":
>
> $ cat >/var/tmp/j.sh <<\EOF
> #!/bin/sh
> e () {
> i=0
> for s
> do
> i=$(( $i + 1 ))
> echo "$i: $s"
> done
> }
> f () {
> e "E: $@ is not binary"
> }
> f 1 "2 3" 4
> EOF
> $ sh /var/tmp/j.sh
> 1: E: 1
> 2: 2 3
> 3: 4 is not binary
>
> Granted, echo is forgiving and will concatenate the arguments it gets with
> a space in between, but you would either want to either:
>
> (1) make it more explicit that helper gets only one argument, by saying
> "$1" instead of "$@", in all places in the helper script; or
>
> (2) if you are planning to make 'helper' capable of handling multiple
> input files, show the error message for the ones that are not binary
> (you would probably need a loop for that).
>
> I think (1) would be sufficient in this case.I too think (1) is right. It was just that originally there was $@ (which I now understand was wrong). So ok to apply the following patch on top of this series? (I assume it's ok, and will repost the whole thing)
---- 8< ----
From: Kirill Smelkov <kirr@mns.spb.ru> Date: Tue, 28 Sep 2010 15:34:48 +0400 Subject: [PATCH 4/4] t4042,t8006,t8007: Textconv converter should take only one argument
Textconv helper in this tests was incorrectly referencing $@, which goes agains textconv "spec". I quote Documentation/gitattributes.txt
""" The `textconv` config option is used to define a program for
performing such a conversion. The program should take a single
argument, the name of a file to convert, and produce the
resulting text on stdout. """So correct textconv helpers to use $1 instead.
Noticed-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> --- t/t4042-diff-textconv-caching.sh | 4 ++-- t/t8006-blame-textconv.sh | 4 ++-- t/t8007-cat-file-textconv.sh | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/t/t4042-diff-textconv-caching.sh b/t/t4042-diff-textconv-caching.sh index 68fee12..6aaa10b 100755 --- a/t/t4042-diff-textconv-caching.sh +++ b/t/t4042-diff-textconv-caching.sh @@ -5,8 +5,8 @@ test_description='test textconv caching' cat >helper <<'EOF' #!/bin/sh -grep -q '^bin: ' "$@" || { echo "E: $@ is not \"binary\" file" 1>&2; exit 1; } -sed 's/^bin: /converted: /' "$@" >helper.out +grep -q '^bin: ' "$1" || { echo "E: $1 is not \"binary\" file" 1>&2; exit 1; } +sed 's/^bin: /converted: /' "$1" >helper.out cat helper.out EOF chmod +x helper diff --git a/t/t8006-blame-textconv.sh b/t/t8006-blame-textconv.sh index a8c311f..dbf623b 100755 --- a/t/t8006-blame-textconv.sh +++ b/t/t8006-blame-textconv.sh @@ -9,8 +9,8 @@ find_blame() { cat >helper <<'EOF' #!/bin/sh -grep -q '^bin: ' "$@" || { echo "E: $@ is not \"binary\" file" 1>&2; exit 1; } -sed 's/^bin: /converted: /' "$@" +grep -q '^bin: ' "$1" || { echo "E: $1 is not \"binary\" file" 1>&2; exit 1; } +sed 's/^bin: /converted: /' "$1" EOF chmod +x helper diff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh index 5e2e0d2..78a0085 100755 --- a/t/t8007-cat-file-textconv.sh +++ b/t/t8007-cat-file-textconv.sh @@ -5,8 +5,8 @@ test_description='git cat-file textconv support' cat >helper <<'EOF' #!/bin/sh -grep -q '^bin: ' "$@" || { echo "E: $@ is not \"binary\" file" 1>&2; exit 1; } -sed 's/^bin: /converted: /' "$@" +grep -q '^bin: ' "$1" || { echo "E: $1 is not \"binary\" file" 1>&2; exit 1; } +sed 's/^bin: /converted: /' "$1" EOF chmod +x helper
-- 1.7.3.19.g3fe0a