Re: [PATCH 3/3] test-lib: '--run' to run only specific tests

[Ilya Bobyr <ilya.bobyr@gmail.com>]

On 4/23/2014 12:51 PM, Eric Sunshine wrote:
> On Tue, Apr 22, 2014 at 4:19 AM, Ilya Bobyr <ilya.bobyr@gmail.com> wrote:
>> Allow better control of the set of tests that will be executed for a
>> single test suite.  Mostly useful while debugging or developing as it
>> allows to focus on a specific test.
>>
>> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
>> ---
>> diff --git a/t/README b/t/README
>> index 6b93aca..2dac619 100644
>> --- a/t/README
>> +++ b/t/README
>> +As noted above, the test set is built going though items left to
>> +right, so this:
>> +
>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
>> +
>> +will run tests 1, 2, and 4.
>> +
>> +You may use negation with ranges.  The following will run all
>> +test as a test suite except from 7 upto 11:
> s/upto/up to/
> ...or...
> s/upto/through/

Fixed.  Thanks.

>> +    $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
>> +
>> +Some tests in a test suite rely on the previous tests performing
>> +certain actions, specifically some tests are designated as
>> +"setup" test, so you cannot _arbitrarily_ disable one test and
>> +expect the rest to function correctly.
>> +--run is mostly useful when you want to focus on a specific test
>> +and know what you are doing.  Or when you want to run up to a
>> +certain test.
>>
>>
>>  Naming Tests
>> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
>> index ae8874e..e2589cc 100755
>> --- a/t/t0000-basic.sh
>> +++ b/t/t0000-basic.sh
>> @@ -84,6 +97,18 @@ check_sub_test_lib_test () {
>>         )
>>  }
>>
>> +check_sub_test_lib_test_err () {
>> +       name="$1" # stdin is the expected output output from the test
>> +       # expecte error output is in descriptor 3
> s/expecte/expected/

Fixed.

>> +       (
>> +               cd "$name" &&
>> +               sed -e 's/^> //' -e 's/Z$//' >expect.out &&
>> +               test_cmp expect.out out &&
>> +               sed -e 's/^> //' -e 's/Z$//' <&3 >expect.err &&
>> +               test_cmp expect.err err
>> +       )
>> +}
>> +
>>  test_expect_success 'pretend we have a fully passing test suite' "
>>         run_sub_test_lib_test full-pass '3 passing tests' <<-\\EOF &&
>>         for i in 1 2 3
>> @@ -333,6 +358,329 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' "
>> +test_expect_success '--run invalid range start' "
>> +       run_sub_test_lib_test_err run-inv-range-start \
>> +               '--run invalid range start' \
>> +               --run='a-5' <<-\\EOF &&
>> +       test_expect_success \"passing test #1\" 'true'
>> +       test_done
>> +       EOF
>> +       check_sub_test_lib_test_err run-inv-range-start \
>> +               <<-\\EOF_OUT 3<<-\\EOF_ERR
>> +       > FATAL: Unexpected exit with code 1
>> +       EOF_OUT
>> +       > error: --run: range start should contain only digits: 'a-5'
> This reads rather strangely, as if it's attempting to give an example
> (after the colon) of a valid digit range, but then shows something
> that is not valid. Rewording it slightly can eliminate the ambiguity:
>
>     error: --run: invalid non-numeric range start: 'a-5'

Changed.

>> +       EOF_ERR
>> +"
>> +
>> +test_expect_success '--run invalid range end' "
>> +       run_sub_test_lib_test_err run-inv-range-end \
>> +               '--run invalid range end' \
>> +               --run='1-z' <<-\\EOF &&
>> +       test_expect_success \"passing test #1\" 'true'
>> +       test_done
>> +       EOF
>> +       check_sub_test_lib_test_err run-inv-range-end \
>> +               <<-\\EOF_OUT 3<<-\\EOF_ERR
>> +       > FATAL: Unexpected exit with code 1
>> +       EOF_OUT
>> +       > error: --run: range end should contain only digits: '1-z'
> Ditto.

Fixed.

>> +       EOF_ERR
>> +"
>> +
>> +test_expect_success '--run invalid selector' "
>> +       run_sub_test_lib_test_err run-inv-selector \
>> +               '--run invalid selector' \
>> +               --run='1?' <<-\\EOF &&
>> +       test_expect_success \"passing test #1\" 'true'
>> +       test_done
>> +       EOF
>> +       check_sub_test_lib_test_err run-inv-selector \
>> +               <<-\\EOF_OUT 3<<-\\EOF_ERR
>> +       > FATAL: Unexpected exit with code 1
>> +       EOF_OUT
>> +       > error: --run: test selector should contain only digits: '1?'
> And here:
>
>     error: --run: invalid non-digit in range selector: '1?'
>
> or something.

Changed to "invalid non-digit in test selector".  This one is only shown
if it does not have a "-", so it is probably not a range.

>> +       EOF_ERR
>> +"
>> +
>> +
>>  test_set_prereq HAVEIT
>>  haveit=no
>>  test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index e7d9c51..46ba513 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -366,6 +374,100 @@ match_pattern_list () {
>>         return 1
>>  }
>>
>> +match_test_selector_list () {
>> +       title="$1"
>> +       shift
>> +       arg="$1"
>> +       shift
>> +       test -z "$1" && return 0
>> +
>> +       # Both commas and spaces are accepted as separators
>> +       OLDIFS=$IFS
>> +       IFS='   ,'
> The comment mentions only space and comma, but the actual assigned IFS
> value also treats tabs as separators. Perhaps update the comment to
> say "commas and whitespace".

I thought that tab is a space character =)  Changed it.

>> +       set -- $1
>> +       IFS=$OLDIFS
>> +
>> +       # If the first selector is negative we include by default.
>> +       include=
>> +       case "$1" in
>> +               !*) include=t ;;
>> +       esac
>> +
>> +       for selector
>> +       do
>> +               orig_selector=$selector
>> +
>> +
> Unnecessary extra blank line.

Thanks.

> [...]
>> +                               ;;
>> +                       *)
>> +                               if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
>> +                               then
>> +                                       echo "error: $title: test selector should contain" \
>> +                                               "only digits: '$orig_selector'" >&2
>> +                                       exit 1
>> +                               fi
>> +               esac
>> +
>> +               # Short cut for "obvious" cases
>> +               test -z "$include" && test -z "$positive" && continue
>> +               test -n "$include" && test -n "$positive" && continue
>> +
>> +               case "$selector" in
>> +                       -*)
>> +                               if test $arg -le ${selector#-}
>> +                               then
>> +                                       include=$positive
>> +                               fi
>> +                               ;;
>> +                       *-)
>> +                               if test $arg -ge ${selector%-}
>> +                               then
>> +                                       include=$positive
>> +                               fi
>> +                               ;;
>> +                       *-*)
>> +                               if test ${selector%%-*} -le $arg \
>> +                                       -a $arg -le ${selector#*-}
> The -a option to 'test' is not portable [1] and is considered obsolete
> by POSIX [2]. Use "test foo && test bar" instead.
>
> [1]: http://www.gnu.org/software/autoconf/manual/autoconf.html#index-g_t_0040command_007btest_007d-1793
> [2]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html

Did not know that.  Thanks.  Changed it.

It is used a number of times thought:

$ git grep '\<test\>.*-a\>' | wc -l
72

About 10 matches are accidental, but the rest are '-a' uses in 'test'.

>> [...]

Thanks a lot for looking into it :)