From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sat, 07 Aug 2010 00:21:34 GMT
Subject: Re: [PATCH v2] test-lib: user-friendly alternatives to test [!] [-d|-f]
Message-ID: <AANLkTimiSJQPcZRZ06BamJPkd8PBkm7CaMcsKRSdEeP_@mail.gmail.com>
In-Reply-To: <20100806225705.GA2534@burratino>

On Fri, Aug 6, 2010 at 22:57, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Matthieu Moy wrote:
>        - test_file_exists <name> [<diagnosis>]
>        - test_dir_exists <name> [<diagnosis>]
>
>          Check that <name> exists and is a file or directory,
>          printing a diagnostic if it does not.  The <diagnosis>
>          if present will be used to give some added context to
>          the diagnostic.
>
>        - test_does_not_exist <name> [<diagnosis>]
>
>          Check that <name> does not exist, printing a
>          diagnostic if it does.  The <diagnosis> will be
>          printed on failure as added context if present.
>
> I think the ..._must_exist names put the emphasis in the
> wrong place, and they look funny in "if" statements.

Personally I'd prefer something where I can just think in the "test"
bultin terms and still get debug info, something like (pseduocode)

    gittest() {
        test "$#" = 3 && { bool='!'; shift; } || bool=
        test=$1; shift
        args="$@"; shift
        case "$test" in
        -f)
            # Handle the common case of -f with a custom message
        ;;
        -d)
            # Same for -d
        ;;
        *)
            # Just pass a switch to test and say "test with the -X
switch", or something
esac
    }

    gittest ! -f ~/.gitconfig
    gittest -f ~/.gitconfig
    gittest -d /tmp
    gittest ! -d /tmp
    gittest -s /tmp

I'll never be able to fit more test_* functions in my brain, and I
wrote the docs :)

> Style nitpick: if statementss in the test-lib have tended to look like
>
>  if [ foo ]
>  then
>        bar
>  fi
>
> so far.  Here the whole function is a glorified "test -f", so I wonder
> if
>
>        [ -f "$1" ] ||
>        {
>                echo >&2 "file $1 doesn't exist. $*"
>                false
>        }
>
> would not be clearer.  I dunno.

This is the style we usually use:

    if ! test -f "$1"
    then
        echo >&2 "file $1 doesn't exist. $*"
        false
    fi

> I have often run into silent test failures of the sort your patch
> is designed to avoid.  Thanks for tackling it.

Yeah, having more intra-test progress is definitely good. Right now I
just remove things from the tests in an ad-hoc fashion until they
start passing if they fail when I debug them.

I mentioned that we could emit these test progress reports as TAP in a
previous E-Mail. Here's how that could look like:

    $ perl -MTest::More=no_plan -E '
        subtest "A git test" => sub {
            pass("doing test -f file");
            pass("git commit ...");
            pass("test_tick...");
            done_testing();
        } for 1 .. 2
    '
        ok 1 - doing test -f file
        ok 2 - git commit ...
        ok 3 - test_tick...
        1..3
    ok 1 - A git test
        ok 1 - doing test -f file
        ok 2 - git commit ...
        ok 3 - test_tick...
        1..3
    ok 2 - A git test
    1..2

I.e. we could make these intra-test progress reports machine readable.

As the example shows the obvious next step would be to make other
utility functions like test_commit() emit a progress status as well.