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

[Eric Sunshine <sunshine@sunshineco.com>]

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/

> +    $ 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/

> +       (
> +               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'

> +       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.

> +       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.

> +       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".

> +       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.

> +               positive=t
> +               case "$selector" in
> +                       !*)
> +                               positive=
> +                               selector=${selector##?}
> +                               ;;
> +               esac
> +
> +               test -z "$selector" && continue
> +
> +               case "$selector" in
> +                       *-*)
> +                               if expr "z${selector%%-*}" : "z[0-9]*[^0-9]" >/dev/null
> +                               then
> +                                       echo "error: $title: range start should contain only" \
> +                                               "digits: '$orig_selector'" >&2
> +                                       exit 1
> +                               fi
> +                               if expr "z${selector#*-}" : "z[0-9]*[^0-9]" >/dev/null
> +                               then
> +                                       echo "error: $title: range end should contain only" \
> +                                               "digits: '$orig_selector'" >&2
> +                                       exit 1
> +                               fi

Weird ranges like "1-4-6" and "1-!5" will be caught by the "error:
range end" clause. Okay.

> +                               ;;
> +                       *)
> +                               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

> +                               then
> +                                       include=$positive
> +                               fi
> +                               ;;
> +                       *)
> +                               if test $arg -eq $selector
> +                               then
> +                                       include=$positive
> +                               fi
> +                               ;;
> +               esac
> +       done
> +
> +       test -n "$include"
> +}
> +
>  maybe_teardown_verbose () {
>         test -z "$verbose_only" && return
>         exec 4>/dev/null 3>/dev/null
> @@ -470,6 +572,13 @@ test_skip () {
>                 fi
>                 skipped_reason="missing $missing_prereq${of_prereq}"
>         fi
> +       if test -z "$to_skip" && test -n "$run_list" &&
> +               ! match_test_selector_list '--run' $test_count "$run_list"
> +       then
> +               to_skip=t
> +               skipped_reason="--run"
> +       fi
> +
>         case "$to_skip" in
>         t)
>                 say_color skip >&3 "skipping test: $@"
> --
> 1.7.9
>