Re: [PATCH 1/4] test: Add `test_emacs_expect_t'.

[Dmitry Kurochkin <dmitry.kurochkin@gmail.com>]

On Tue, 17 Jan 2012 12:52:25 +0000, David Edmondson <dme@dme.org> wrote:
> Add a new test function to allow simpler testing of emacs
> functionality.
> 
> `test_emacs_expect_t' takes two arguments:
>   - the name of the test,
>   - some lisp to evaluate.
> 
> The test passes if the lisp returns `t', otherwise it fails and the
> output is reported to the tester.

-1

This is not what I suggested.  I do not like the approach when a single
function is used to both declare a subtest and test for result (as
opposed to test_begin_subtest).  The fact that it is possible to write
tests in two different ways makes it hard to maintain and improve the
test framework (one example would be known broken test support).  I
consider the proper way to write tests to be using the
test_begin_subtest function.  Other functions are not currently
deprecated, but I am against adding new code that make the situation
worse.

Also, please consider documenting new functions in README.

Regards,
  Dmitry

> ---
>  test/emacs-test-functions.sh |    8 ++++++++
>  test/notmuch-test            |    1 +
>  test/test-lib.sh             |   24 ++++++++++++++++++++++++
>  3 files changed, 33 insertions(+), 0 deletions(-)
>  create mode 100755 test/emacs-test-functions.sh
> 
> diff --git a/test/emacs-test-functions.sh b/test/emacs-test-functions.sh
> new file mode 100755
> index 0000000..969cc78
> --- /dev/null
> +++ b/test/emacs-test-functions.sh
> @@ -0,0 +1,8 @@
> +#!/usr/bin/env bash
> +
> +test_description="emacs test function sanity"
> +. test-lib.sh
> +
> +test_emacs_expect_t "emacs test function sanity" 't'
> +
> +test_done
> diff --git a/test/notmuch-test b/test/notmuch-test
> index 6a99ae3..d034f99 100755
> --- a/test/notmuch-test
> +++ b/test/notmuch-test
> @@ -52,6 +52,7 @@ TESTS="
>    python
>    hooks
>    argument-parsing
> +  emacs-test-functions.sh
>  "
>  TESTS=${NOTMUCH_TESTS:=$TESTS}
>  
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 7c9ce24..15da973 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -503,6 +503,30 @@ test_expect_equal_file ()
>      fi
>  }
>  
> +test_emacs_expect_t () {
> +	test "$#" = 2 || error "bug in the test script: not 2 parameters to test_emacs_expect_t"
> +	test_reset_state_
> +	if ! test_skip "$1"
> +	then
> +		# We cannot call 'test_emacs' in a subshell, because
> +		# the setting of EMACS_SERVER would not persist
> +		# throughout a sequence of tests, so we use a
> +		# temporary file.
> +		tmp="$TMPDIR"; if [ -z "$tmp" ]; then tmp=/tmp; fi
> +		output="$tmp/test_emacs_output.$$"
> +		test_emacs "$2" >"${output}"
> +		result=$(cat "${output}")
> +		rm -f "${output}"
> +
> +		if [ "$result" == t ]
> +		then
> +			test_ok_ "$1"
> +		else
> +			test_failure_ "$1" "$(eval printf ${result})"
> +		fi
> +	fi
> +}
> +
>  NOTMUCH_NEW ()
>  {
>      notmuch new | grep -v -E -e '^Processed [0-9]*( total)? file|Found [0-9]* total file'
> -- 
> 1.7.7.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch