Re: [PATCH] t/t4018: avoid two unnecessary sub-shell invocations

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sep 10, 2010, 17:25 UTC
Message-ID: <AANLkTindP1KiyFjF+7EVhKDciBr-g_cg3jfHCRB0Qfxo@mail.gmail.com>
In-Reply-To: <rl_Fe3GvwRHMfzdMCqwnBhdCJhaaE21fdWY64ux0ATvXGt5hFA80h-4iMktgWtjAQ_54uD09KJc@cipher.nrlssc.navy.mil>

On Fri, Sep 10, 2010 at 16:13, Brandon Casey <casey@nrlssc.navy.mil> wrote:

Show 32 quoted lines

> From: Brandon Casey <drafnel@gmail.com>
>
>
> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---
> Thanks again Jeff for your, as always, very valuable review.
>
>
>  t/t4018-diff-funcname.sh |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
> index 620cd02..c8e1937 100755
> --- a/t/t4018-diff-funcname.sh
> +++ b/t/t4018-diff-funcname.sh
> @@ -37,13 +37,13 @@ for p in $builtin_patterns
>  do
>        test_expect_success "builtin $p pattern compiles" '
>                echo "*.java diff=$p" > .gitattributes &&
> -               ! ( git diff --no-index Beer.java Beer-correct.java 2>&1 |
> -                       grep "fatal" > /dev/null )
> +               ! { git diff --no-index Beer.java Beer-correct.java 2>&1 |
> +                       grep "fatal" > /dev/null; }
>        '
>        test_expect_success "builtin $p wordRegex pattern compiles" '
> -               ! ( git diff --no-index --word-diff \
> +               ! { git diff --no-index --word-diff \
>                        Beer.java Beer-correct.java 2>&1 |
> -                       grep "fatal" > /dev/null )
> +                       grep "fatal" > /dev/null; }
>        '
>  done

FWIW (but don't think you need to change 'em) I'd do these as (untested):

test_expect_success "builtin $p wordRegex pattern compiles" '
    git diff --no-index --word-diff >out 2>err &&
    # Or whatever out should be..
    ! test -s out &&
    grep fatal err
'

It's much easier to debug tests that use intermediate files with --immediate --debug when they break, because you can just check out what out and err contain. You can't do that if the output was forgotten in some pipe.

Maybe I should add a bit into t/README about this ...