From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: David Edmondson <dme@dme.org>, notmuch@notmuchmail.org
Subject: Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.
In-Reply-To: <1326977643-19792-2-git-send-email-dme@dme.org>
References: <yf639bcarcs.fsf@taco2.nixu.fi>
	<1326977643-19792-1-git-send-email-dme@dme.org>
	<1326977643-19792-2-git-send-email-dme@dme.org>
User-Agent: Notmuch/0.11+100~gd650abf (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Mon, 23 Jan 2012 20:45:43 +0400
Message-ID: <87lioy2wqw.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Thu, 19 Jan 2012 12:54:02 +0000, David Edmondson <dme@dme.org> wrote:
> Add a new test function to allow simpler testing of emacs
> functionality.
> 
> `test_emacs_expect_t' takes one argument - a list expression to
> evaluate. The test passes if the expression returns `t', otherwise it
> fails and the output is reported to the tester.
> ---
> 
> In the interest of moving forward, don't use eval.
> 
>  test/README                  |    8 ++++++++
>  test/emacs-test-functions.sh |    9 +++++++++
>  test/notmuch-test            |    1 +
>  test/test-lib.el             |    9 +++++++++
>  test/test-lib.sh             |   26 ++++++++++++++++++++++++++
>  5 files changed, 53 insertions(+), 0 deletions(-)
>  create mode 100755 test/emacs-test-functions.sh
> 
> diff --git a/test/README b/test/README
> index bde6db0..9dbe2ee 100644
> --- a/test/README
> +++ b/test/README
> @@ -189,6 +189,14 @@ library for your script to use.
>     tests that may run in the same Emacs instance.  Use `let' instead
>     so the scope of the changed variables is limited to a single test.
>  
> + test_emacs_expect_t <emacs-lisp-expressions>
> +
> +  This function executes the provided emacs lisp script within
> +  emacs in a manner similar to 'test_emacs'. The expressions should
> +  return the value `t' to indicate that the test has passed. If the
> +  test does not return `t' then it is considered failed and all data
> +  returned by the test is reported to the tester.
> +
>   test_done
>  
>     Your test script must have test_done at the end.  Its purpose
> diff --git a/test/emacs-test-functions.sh b/test/emacs-test-functions.sh
> new file mode 100755
> index 0000000..0e1f9fc
> --- /dev/null
> +++ b/test/emacs-test-functions.sh
> @@ -0,0 +1,9 @@
> +#!/usr/bin/env bash
> +
> +test_description="emacs test function sanity"
> +. test-lib.sh
> +
> +test_begin_subtest "emacs test function sanity"
> +test_emacs_expect_t '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.el b/test/test-lib.el
> index 3b817c3..1d51b8d 100644
> --- a/test/test-lib.el
> +++ b/test/test-lib.el
> @@ -76,3 +76,12 @@ nothing."
>  
>  (add-hook-counter 'notmuch-hello-mode-hook)
>  (add-hook-counter 'notmuch-hello-refresh-hook)
> +
> +(defmacro notmuch-test-run-test (&rest body)

Please consider renaming to `notmuch-hello-run'.

> +  "Evaluate a BODY of test expressions and output the result."
> +  `(with-temp-buffer
> +     (let ((result (progn ,@body)))
> +       (insert (if (stringp result)
> +		   result
> +		 (prin1-to-string result)))
> +       (test-output))))
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 7c9ce24..9cf1b92 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -503,6 +503,32 @@ test_expect_equal_file ()
>      fi
>  }
>  
> +test_emacs_expect_t () {
> +	test "$#" = 1 || error "bug in the test script: not 1 parameter to test_emacs_expect_t"
> +
> +	# Run the test.
> +	if ! test_skip "$test_subtest_name"
> +	then
> +		test_emacs "(notmuch-test-run-test $1)" >/dev/null
> +	fi
> +
> +	# Restore state after the test.
> +	exec 1>&6 2>&7		# Restore stdout and stderr
> +	inside_subtest=
> +
> +	# Report success/failure.
> +	if ! test_skip "$test_subtest_name"
> +	then
> +		result=$(cat OUTPUT)
> +		if [ "$result" = t ]
> +		then
> +			test_ok_ "$test_subtest_name"
> +		else
> +			test_failure_ "$test_subtest_name" "${result}"
> +		fi
> +	fi
> +}

Test_skip function must be called once during the test.  Otherwise
$test_count is incremented twice, perhaps there are other side effects
as well.

I think test_emacs_expect_t function must support optional $prereq
argument like other test_equal_* functions (e.g. test_expect_equal).

After fixing the above comments, test_emacs_expect_t function should
look more like test_expect_equal and friends:

  exec 1>&6 2>&7		# Restore stdout and stderr
  [4 lines almost identical to test_expect_equal]

  if ! test_skip "$test_subtest_name"
    test_emacs ...
    ...
  fi

Regards,
  Dmitry

> +
>  NOTMUCH_NEW ()
>  {
>      notmuch new | grep -v -E -e '^Processed [0-9]*( total)? file|Found [0-9]* total file'
> -- 
> 1.7.8.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch