Re: [PATCH 1/3] test-lib: tests can have multiple prerequisites

Jeff King [off-list ref] writes:

I do wonder if it might be less error-prone to have:

  test_expect_success PREREQ1 PREREQ2 'desc' 'test'

instead of

  test_expect_success 'PREREQ1 PREREQ2' 'desc' 'test'

Spot what is wrong with this:

        test_expect_success make sure we have the repo '
                test -d .git
        '

I think "we may or may not have a single prereq word" would be less error
prone.  (you could make sure "test_have_prereq" gets no lowercase letter
to mitigate the issue, though).

I tend to like the comma myself, but quoted, space separated list is fine
by me.

On Mon, Apr 26, 2010 at 05:06:55PM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

I do wonder if it might be less error-prone to have:

  test_expect_success PREREQ1 PREREQ2 'desc' 'test'

instead of

  test_expect_success 'PREREQ1 PREREQ2' 'desc' 'test'

Spot what is wrong with this:

        test_expect_success make sure we have the repo '
                test -d .git
        '

To me it's easy to spot, but there is nothing wrong with it that the
shell will spot, so that is a downside. Whatever we go with, it would
perhaps be less error prone to set "NOPREREQ1" when we don't have
PREREQ1, and check that either $1 or NO$1 is set when checking
prerequisites. But that is probably just over-engineering. This is not
an error we have seen a lot of, and it is not that hard to spot with
code review.

-Peff