Re: [PATCH 6/7] t1303 (config): style tweaks

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 7, 2010, 04:52 UTC
Message-ID: <7vtym2p34e.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100907043050.GA13291@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 22 quoted lines

> ... But I really don't want to be too draconian
> about "everything must go into test_expect_success". Sure, if you are
> executing commands that might have output, or might be of interest to
> the user, put them there. But I find this a lot more readable:
>
> cat >expect <<'EOF'
> ... some expected output ...
> EOF
> test_expect_success 'frob it' '
>         git frob &&
>         test_cmp expect actual
> '
>
> than:
>
> test_expect_success 'frob it' '
>         cat >expect <<"EOF" &&
> ... some expected output ...
> EOF
>         git frob &&
>         test_cmp expect actual
> '

I agree.

My gut feeling has been that any set-up that uses "git" should never be outside test_expect_success, but things like cat <<HERE / echo to prepare test vector are not expected to fail (we are not in the business of testing the build platform) and can be at the top of the script between the inclusion of test-lib.sh and the first test_expect_success.

When you throw in tools that are platform dependent to the mix, however, things get muddier. Setup that use stuff like "ln -s this that" and "setacl" is easier to handle inside test_expect_success guarded by an appropriate prerequisite. So even that "anything git should be inside, everything else can go outside if it wants to" rule of thumb is not so black and white.