[PATCH 2/3] test: improve known broken tests support

[Austin Clements]

Great idea!

Just a heads-up, this will conflict with the first two patches in the
atomicity series (though git may not detect the second conflict).
Both are trivial patches, though, so the merge should be easy.

Three minor comments below.

On Sun, Jul 3, 2011 at 9:59 PM, Dmitry Kurochkin
<dmitry.kurochkin at gmail.com> wrote:
> There is existing support for broken tests.  But it is not convenient
> to use.  The primary issue is that we have to maintain a set of
> test_expect_*_failure functions which are equivalent to the normal
> test_expect_* counterparts except for what functions are called for
> result reporting.  The patch adds test_subtest_known_broken function
> which marks a subset as broken, making the normal test_expect_*
> functions behave as test_expect_*_failure.  All test_expect_*_failure
> functions are removed.  Test_known_broken_failure_ is changed to
> format details the same way as test_failure_ does.
>
> Another benefit of this change is that the diff when a broken test is
> fixed would be small and nice.
>
> Documentation is updated accordingly.
> ---
>  test/README      |   17 ++++++++---------
>  test/test-lib.sh |   53 +++++++++++++++--------------------------------------
>  2 files changed, 23 insertions(+), 47 deletions(-)
>
> diff --git a/test/README b/test/README
> index a245bf1..f926b9f 100644
> --- a/test/README
> +++ b/test/README
> @@ -132,20 +132,19 @@ library for your script to use.
>    <script>.  If it yields success, test is considered
>    successful.  <message> should state what it is testing.
>
> - test_expect_failure <message> <script>
> -
> -   This is NOT the opposite of test_expect_success, but is used
> -   to mark a test that demonstrates a known breakage.  Unlike
> -   the usual test_expect_success tests, which say "ok" on
> -   success and "FAIL" on failure, this will say "FIXED" on
> -   success and "still broken" on failure.  Failures from these
> -   tests won't cause -i (immediate) to stop.
> -
>  test_begin_subtest <message>
>
>    Set the test description message for a subsequent test_expect_equal
>    invocation (see below).
>
> + test_subtest_known_broken
> +
> +   Mark the current test as broken.  Such tests are expected to fail.
> +   Unlike the normal tests, which say "PASS" on success and "FAIL" on
> +   failure, these will say "FIXED" on success and "BROKEN" on failure.
> +   Failures from these tests won't cause -i (immediate) to stop.  This
> +   must be called before any test_expect_* function.

Perhaps "A test must call this before any test_expect_* function"?
There can be earlier test_expect_* calls from other tests.

> +
>  test_expect_equal <output> <expected>
>
>    This is an often-used convenience function built on top of
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 22e387e..0cd4170 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -424,6 +424,7 @@ test_begin_subtest ()
>        error "bug in test script: Missing test_expect_equal in ${BASH_SOURCE[1]}:${BASH_LINENO[0]}"
>     fi
>     test_subtest_name="$1"
> +    test_subtest_known_broken_=
>     # Remember stdout and stderr file descriptors and redirect test
>     # output to the previously prepared file descriptors 3 and 4 (see
>     # below)
> @@ -484,29 +485,6 @@ test_expect_equal_file ()
>     fi
>  }
>
> -test_expect_equal_failure ()
> -{
> -       exec 1>&6 2>&7          # Restore stdout and stderr
> -       inside_subtest=
> -       test "$#" = 3 && { prereq=$1; shift; } || prereq=
> -       test "$#" = 2 ||
> -       error "bug in the test script: not 2 or 3 parameters to test_expect_equal"
> -
> -       output="$1"
> -       expected="$2"
> -       if ! test_skip "$@"
> -       then
> -               if [ "$output" = "$expected" ]; then
> -                       test_known_broken_ok_ "$test_subtest_name"
> -               else
> -                       test_known_broken_failure_ "$test_subtest_name"
> -                       testname=$this_test.$test_count
> -                       echo "$expected" > $testname.expected
> -                       echo "$output" > $testname.output
> -               fi
> -    fi
> -}
> -
>  NOTMUCH_NEW ()
>  {
>     notmuch new | grep -v -E -e '^Processed [0-9]*( total)? file|Found [0-9]* total file'
> @@ -568,12 +546,20 @@ test_have_prereq () {
>  # the text_expect_* functions instead.
>
>  test_ok_ () {
> +       if [ "$test_subtest_known_broken_" = 1 ]; then
> +               test_known_broken_ok_ "$@"
> +               return
> +       fi
>        test_success=$(($test_success + 1))
>        say_color pass "%-6s" "PASS"
>        echo " $@"
>  }
>
>  test_failure_ () {
> +       if [ "$test_subtest_known_broken_" = 1 ]; then
> +               test_known_broken_failure_ "$@"
> +               return
> +       fi
>        test_failure=$(($test_failure + 1))
>        say_color error "%-6s" "FAIL"
>        echo " $1"
> @@ -592,7 +578,10 @@ test_known_broken_ok_ () {
>  test_known_broken_failure_ () {
>        test_broken=$(($test_broken+1))
>        say_color pass "%-6s" "BROKEN"
> -       echo " $@"
> +       echo " $1"
> +       shift
> +       echo "$@" | sed -e 's/^/        /'
> +       if test "$verbose" != "t"; then cat test.output; fi

Would it be possible to keep just the one copy of the failure printing
code in test_failure_?

>  }
>
>  test_debug () {
> @@ -636,20 +625,8 @@ test_skip () {
>        esac
>  }
>
> -test_expect_failure () {
> -       test "$#" = 3 && { prereq=$1; shift; } || prereq=
> -       test "$#" = 2 ||
> -       error "bug in the test script: not 2 or 3 parameters to test-expect-failure"
> -       if ! test_skip "$@"
> -       then
> -               test_run_ "$2"
> -               if [ "$?" = 0 -a "$eval_ret" = 0 ]
> -               then
> -                       test_known_broken_ok_ "$1"
> -               else
> -                       test_known_broken_failure_ "$1"
> -               fi
> -       fi
> +test_subtest_known_broken () {
> +       test_subtest_known_broken_=1

The convention in test-lib appears to be "t" for true variables.

>  }
>
>  test_expect_success () {
> --
> 1.7.5.4