emacs based tests, version 3

emacs based tests, version 3

[David Edmondson]

Taking Dmitry's suggestions on board. The end result does indeed feel
better, thanks!

[PATCH 1/4] test: Add `test_emacs_expect_t'.
[PATCH 2/4] test: Add address cleaning tests.
[PATCH 3/4] emacs: Avoid `mail-header-parse-address' in
[PATCH 4/4] emacs: Another special case for

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

[David Edmondson]

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

[PATCH 2/4] test: Add address cleaning tests.

[David Edmondson]

Including some more test framework in test-lib.el.
---
 test/address-cleaning.el |   29 +++++++++++++++++++++++++++++
 test/address-cleaning.sh |   11 +++++++++++
 test/notmuch-test        |    1 +
 test/test-lib.el         |   29 +++++++++++++++++++++++++++++
 4 files changed, 70 insertions(+), 0 deletions(-)
 create mode 100644 test/address-cleaning.el
 create mode 100755 test/address-cleaning.sh

diff --git a/test/address-cleaning.el b/test/address-cleaning.el
new file mode 100644
index 0000000..59e8d92
--- /dev/null
+++ b/test/address-cleaning.el
@@ -0,0 +1,29 @@
+(defun notmuch-test-address-cleaning-1 ()
+  (notmuch-test-compare (notmuch-show-clean-address "dme@dme.org")
+			"dme@dme.org"))
+
+(defun notmuch-test-address-cleaning-2 ()
+  (let* ((input '("foo@bar.com"
+		  "<foo@bar.com>"
+		  "Foo Bar <foo@bar.com>"
+		  "foo@bar.com <foo@bar.com>"
+		  "\"Foo Bar\" <foo@bar.com>"))
+	 (expected '("foo@bar.com"
+		     "foo@bar.com"
+		     "Foo Bar <foo@bar.com>"
+		     "foo@bar.com"
+		     "Foo Bar <foo@bar.com>"))
+	 (output (mapcar #'notmuch-show-clean-address input)))
+    (notmuch-test-compare output expected)))
+
+(defun notmuch-test-address-cleaning-3 ()
+  (let* ((input '("ДБ <db-uknot@stop.me.uk>"
+		  "foo (at home) <foo@bar.com>"
+		  "foo [at home] <foo@bar.com>"
+		  "Foo Bar"))
+	 (expected '("ДБ <db-uknot@stop.me.uk>"
+		     "foo (at home) <foo@bar.com>"
+		     "foo [at home] <foo@bar.com>"
+		     "Foo Bar"))
+	 (output (mapcar #'notmuch-show-clean-address input)))
+    (notmuch-test-compare output expected)))
diff --git a/test/address-cleaning.sh b/test/address-cleaning.sh
new file mode 100755
index 0000000..7ec011a
--- /dev/null
+++ b/test/address-cleaning.sh
@@ -0,0 +1,11 @@
+#!/usr/bin/env bash
+
+test_description="emacs address cleaning"
+. test-lib.sh
+
+for i in 1 2 3; do
+    test_emacs_expect_t "notmuch-test-address-clean-$i" \
+    '(load "address-cleaning.el") (notmuch-test-address-cleaning-'$i')'
+done
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index d034f99..7768c32 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -53,6 +53,7 @@ TESTS="
   hooks
   argument-parsing
   emacs-test-functions.sh
+  address-cleaning.sh
 "
 TESTS=${NOTMUCH_TESTS:=$TESTS}
 
diff --git a/test/test-lib.el b/test/test-lib.el
index 3b817c3..cf8a46d 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -20,6 +20,8 @@
 ;;
 ;; Authors: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
 
+(require 'cl)	;; This code is generally used uncompiled.
+
 ;; `read-file-name' by default uses `completing-read' function to read
 ;; user input.  It does not respect `standard-input' variable which we
 ;; use in tests to provide user input.  So replace it with a plain
@@ -76,3 +78,30 @@ nothing."
 
 (add-hook-counter 'notmuch-hello-mode-hook)
 (add-hook-counter 'notmuch-hello-refresh-hook)
+
+;; Functions to help when writing tests:
+
+(defun notmuch-test-reporter (output expected)
+  "Report that the `output' does not match the `expected' result."
+  (concat "Expect:\t" (prin1-to-string expected) "\n"
+	  "Output:\t" (prin1-to-string output) "\n"))
+
+(defun notmuch-test-unsimilar (output expected)
+  "`output' and `expected' are dissimilar. Show a summary of
+the differences, ignoring similarities."
+  (cond ((and (listp output)
+	      (listp expected))
+	 (apply #'concat (loop for o in output
+			       for e in expected
+			       if (not (equal o e))
+			       collect (notmuch-test-reporter o e))))
+
+	(t
+	 ;; Catch all case.
+	 (notmuch-test-reporter output expected))))
+
+(defun notmuch-test-compare (output expected)
+  "Compare `output' with `expected'. Report any discrepencies."
+  (if (equal output expected)
+      t
+    (notmuch-test-unsimilar output expected)))
-- 
1.7.7.3

[PATCH 3/4] emacs: Avoid `mail-header-parse-address' in `notmuch-show-clean-address'.

[David Edmondson]

`mail-header-parse-address' expects un-decoded mailbox parts, which is
not what we have at this point. Replace it with simple string
deconstruction.
---
 emacs/notmuch-show.el |   48 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 797f94b..8b2fbb3 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -227,21 +227,43 @@ indentation."
   "Try to clean a single email ADDRESS for display.  Return
 unchanged ADDRESS if parsing fails."
   (condition-case nil
-    (let* ((parsed (mail-header-parse-address address))
-	   (address (car parsed))
-	   (name (cdr parsed)))
-      ;; Remove double quotes. They might be required during transport,
-      ;; but we don't need to see them.
-      (when name
-        (setq name (replace-regexp-in-string "\"" "" name)))
+    (let (p-name p-address)
+      ;; It would be convenient to use `mail-header-parse-address',
+      ;; but that expects un-decoded mailbox parts, whereas our
+      ;; mailbox parts are already decoded (and hence may contain
+      ;; UTF-8). Given that notmuch should handle most of the awkward
+      ;; cases, some simple string deconstruction should be sufficient
+      ;; here.
+      (cond
+       ;; "User <user@dom.ain>" style.
+       ((string-match "\\(.*\\) <\\(.*\\)>" address)
+	(setq p-name (match-string 1 address)
+	      p-address (match-string 2 address)))
+
+       ;; "<user@dom.ain>" style.
+       ((string-match "<\\(.*\\)>" address)
+	(setq p-address (match-string 1 address)))
+
+       ;; Everything else.
+       (t
+	(setq p-address address)))
+      
+      ;; Remove outer double quotes. They might be required during
+      ;; transport, but we don't need to see them.
+      (when (and p-name
+		 (string-match "^\"\\(.*\\)\"$" p-name))
+        (setq p-name (match-string 1 p-name)))
+
       ;; If the address is 'foo@bar.com <foo@bar.com>' then show just
       ;; 'foo@bar.com'.
-      (when (string= name address)
-        (setq name nil))
-
-      (if (not name)
-        address
-        (concat name " <" address ">")))
+      (when (string= p-name p-address)
+        (setq p-name nil))
+
+      ;; If no name results, return just the address.
+      (if (not p-name)
+	  p-address
+	;; Otherwise format the name and address together.
+	(concat p-name " <" p-address ">")))
     (error address)))
 
 (defun notmuch-show-insert-headerline (headers date tags depth)
-- 
1.7.7.3

[PATCH 4/4] emacs: Another special case for `notmuch-show-clean-address'.

[David Edmondson]

Remove backslashes.
---
 emacs/notmuch-show.el    |   14 +++++++++-----
 test/address-cleaning.el |    6 ++++--
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 8b2fbb3..90c9c05 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -248,11 +248,15 @@ unchanged ADDRESS if parsing fails."
        (t
 	(setq p-address address)))
       
-      ;; Remove outer double quotes. They might be required during
-      ;; transport, but we don't need to see them.
-      (when (and p-name
-		 (string-match "^\"\\(.*\\)\"$" p-name))
-        (setq p-name (match-string 1 p-name)))
+      ;; Remove elements of the mailbox part that are not relevant for
+      ;; display, even if they are required during transport.
+      (when p-name
+	;; Outer double quotes.
+	(when (string-match "^\"\\(.*\\)\"$" p-name)
+	  (setq p-name (match-string 1 p-name)))
+
+	;; Backslashes.
+	(setq p-name (replace-regexp-in-string "\\\\" "" p-name)))
 
       ;; If the address is 'foo@bar.com <foo@bar.com>' then show just
       ;; 'foo@bar.com'.
diff --git a/test/address-cleaning.el b/test/address-cleaning.el
index 59e8d92..83d6263 100644
--- a/test/address-cleaning.el
+++ b/test/address-cleaning.el
@@ -20,10 +20,12 @@
   (let* ((input '("ДБ <db-uknot@stop.me.uk>"
 		  "foo (at home) <foo@bar.com>"
 		  "foo [at home] <foo@bar.com>"
-		  "Foo Bar"))
+		  "Foo Bar"
+		  "Fred Dibna \\[extraordinaire\\] <fred@dibna.com>"))
 	 (expected '("ДБ <db-uknot@stop.me.uk>"
 		     "foo (at home) <foo@bar.com>"
 		     "foo [at home] <foo@bar.com>"
-		     "Foo Bar"))
+		     "Foo Bar"
+		     "Fred Dibna [extraordinaire] <fred@dibna.com>"))
 	 (output (mapcar #'notmuch-show-clean-address input)))
     (notmuch-test-compare output expected)))
-- 
1.7.7.3

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

[Dmitry Kurochkin]

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

Re: [PATCH 2/4] test: Add address cleaning tests.

[Dmitry Kurochkin]

On Tue, 17 Jan 2012 12:52:26 +0000, David Edmondson <dme@dme.org> wrote:
> Including some more test framework in test-lib.el.
> ---
>  test/address-cleaning.el |   29 +++++++++++++++++++++++++++++
>  test/address-cleaning.sh |   11 +++++++++++
>  test/notmuch-test        |    1 +
>  test/test-lib.el         |   29 +++++++++++++++++++++++++++++
>  4 files changed, 70 insertions(+), 0 deletions(-)
>  create mode 100644 test/address-cleaning.el
>  create mode 100755 test/address-cleaning.sh
> 

Since test/ directory is used for all kind of tests not just Emacs
UI-specific, so I think address-cleaning.* files should be
emacs-address-cleaning.

Regards,
  Dmitry

> diff --git a/test/address-cleaning.el b/test/address-cleaning.el
> new file mode 100644
> index 0000000..59e8d92
> --- /dev/null
> +++ b/test/address-cleaning.el
> @@ -0,0 +1,29 @@
> +(defun notmuch-test-address-cleaning-1 ()
> +  (notmuch-test-compare (notmuch-show-clean-address "dme@dme.org")
> +			"dme@dme.org"))
> +
> +(defun notmuch-test-address-cleaning-2 ()
> +  (let* ((input '("foo@bar.com"
> +		  "<foo@bar.com>"
> +		  "Foo Bar <foo@bar.com>"
> +		  "foo@bar.com <foo@bar.com>"
> +		  "\"Foo Bar\" <foo@bar.com>"))
> +	 (expected '("foo@bar.com"
> +		     "foo@bar.com"
> +		     "Foo Bar <foo@bar.com>"
> +		     "foo@bar.com"
> +		     "Foo Bar <foo@bar.com>"))
> +	 (output (mapcar #'notmuch-show-clean-address input)))
> +    (notmuch-test-compare output expected)))
> +
> +(defun notmuch-test-address-cleaning-3 ()
> +  (let* ((input '("ДБ <db-uknot@stop.me.uk>"
> +		  "foo (at home) <foo@bar.com>"
> +		  "foo [at home] <foo@bar.com>"
> +		  "Foo Bar"))
> +	 (expected '("ДБ <db-uknot@stop.me.uk>"
> +		     "foo (at home) <foo@bar.com>"
> +		     "foo [at home] <foo@bar.com>"
> +		     "Foo Bar"))
> +	 (output (mapcar #'notmuch-show-clean-address input)))
> +    (notmuch-test-compare output expected)))
> diff --git a/test/address-cleaning.sh b/test/address-cleaning.sh
> new file mode 100755
> index 0000000..7ec011a
> --- /dev/null
> +++ b/test/address-cleaning.sh
> @@ -0,0 +1,11 @@
> +#!/usr/bin/env bash
> +
> +test_description="emacs address cleaning"
> +. test-lib.sh
> +
> +for i in 1 2 3; do
> +    test_emacs_expect_t "notmuch-test-address-clean-$i" \
> +    '(load "address-cleaning.el") (notmuch-test-address-cleaning-'$i')'
> +done
> +
> +test_done
> diff --git a/test/notmuch-test b/test/notmuch-test
> index d034f99..7768c32 100755
> --- a/test/notmuch-test
> +++ b/test/notmuch-test
> @@ -53,6 +53,7 @@ TESTS="
>    hooks
>    argument-parsing
>    emacs-test-functions.sh
> +  address-cleaning.sh
>  "
>  TESTS=${NOTMUCH_TESTS:=$TESTS}
>  
> diff --git a/test/test-lib.el b/test/test-lib.el
> index 3b817c3..cf8a46d 100644
> --- a/test/test-lib.el
> +++ b/test/test-lib.el
> @@ -20,6 +20,8 @@
>  ;;
>  ;; Authors: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
>  
> +(require 'cl)	;; This code is generally used uncompiled.
> +
>  ;; `read-file-name' by default uses `completing-read' function to read
>  ;; user input.  It does not respect `standard-input' variable which we
>  ;; use in tests to provide user input.  So replace it with a plain
> @@ -76,3 +78,30 @@ nothing."
>  
>  (add-hook-counter 'notmuch-hello-mode-hook)
>  (add-hook-counter 'notmuch-hello-refresh-hook)
> +
> +;; Functions to help when writing tests:
> +
> +(defun notmuch-test-reporter (output expected)
> +  "Report that the `output' does not match the `expected' result."
> +  (concat "Expect:\t" (prin1-to-string expected) "\n"
> +	  "Output:\t" (prin1-to-string output) "\n"))
> +
> +(defun notmuch-test-unsimilar (output expected)
> +  "`output' and `expected' are dissimilar. Show a summary of
> +the differences, ignoring similarities."
> +  (cond ((and (listp output)
> +	      (listp expected))
> +	 (apply #'concat (loop for o in output
> +			       for e in expected
> +			       if (not (equal o e))
> +			       collect (notmuch-test-reporter o e))))
> +
> +	(t
> +	 ;; Catch all case.
> +	 (notmuch-test-reporter output expected))))
> +
> +(defun notmuch-test-compare (output expected)
> +  "Compare `output' with `expected'. Report any discrepencies."
> +  (if (equal output expected)
> +      t
> +    (notmuch-test-unsimilar output expected)))
> -- 
> 1.7.7.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/4] test: Add address cleaning tests.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 252 bytes --]

On Tue, 17 Jan 2012 17:11:58 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Since test/ directory is used for all kind of tests not just Emacs
> UI-specific, so I think address-cleaning.* files should be
> emacs-address-cleaning.

Okay.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 801 bytes --]

(And one for the list...)

On Tue, 17 Jan 2012 17:09:35 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> -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.

Sigh. Okay.

> Also, please consider documenting new functions in README.

Missed that, sorry.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH 1/3] test: Don't return the result of checking for running emacs to the tester.

[David Edmondson]

---
 test/test-lib.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index d1fbc05..7c9ce24 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -926,7 +926,7 @@ test_emacs () {
 				--eval '(orphan-watchdog $$)'" || return
 		EMACS_SERVER="$server_name"
 		# wait until the emacs server is up
-		until test_emacs '()' 2>/dev/null; do
+		until test_emacs '()' >/dev/null 2>/dev/null; do
 			sleep 1
 		done
 	fi
-- 
1.7.7.3

[PATCH 2/3] test: Add `test_emacs_expect_t'.

[David Edmondson]

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

Re-worked as Dmitry suggested.

 test/README                  |    8 ++++++++
 test/emacs-test-functions.sh |    9 +++++++++
 test/notmuch-test            |    1 +
 test/test-lib.sh             |   33 +++++++++++++++++++++++++++++++++
 4 files changed, 51 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.sh b/test/test-lib.sh
index 7c9ce24..4b05760 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -503,6 +503,39 @@ 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
+		# 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 "$1" > "${output}"
+		result=$(cat "${output}")
+		rm -f "${output}"
+	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
+		if [ "$result" == t ]
+		then
+			test_ok_ "$test_subtest_name"
+		else
+			test_failure_ "$test_subtest_name" "$(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

[PATCH 3/3] test: Add address cleaning tests.

[David Edmondson]

Including some more test framework in test-lib.el.
---

notmuch-test-address-cleaning-3 currently fails, in order that you can
see the output format in that case.

 test/emacs-address-cleaning.el |   29 +++++++++++++++++++++++++++++
 test/emacs-address-cleaning.sh |   12 ++++++++++++
 test/notmuch-test              |    1 +
 test/test-lib.el               |   29 +++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+), 0 deletions(-)
 create mode 100644 test/emacs-address-cleaning.el
 create mode 100755 test/emacs-address-cleaning.sh

diff --git a/test/emacs-address-cleaning.el b/test/emacs-address-cleaning.el
new file mode 100644
index 0000000..59e8d92
--- /dev/null
+++ b/test/emacs-address-cleaning.el
@@ -0,0 +1,29 @@
+(defun notmuch-test-address-cleaning-1 ()
+  (notmuch-test-compare (notmuch-show-clean-address "dme@dme.org")
+			"dme@dme.org"))
+
+(defun notmuch-test-address-cleaning-2 ()
+  (let* ((input '("foo@bar.com"
+		  "<foo@bar.com>"
+		  "Foo Bar <foo@bar.com>"
+		  "foo@bar.com <foo@bar.com>"
+		  "\"Foo Bar\" <foo@bar.com>"))
+	 (expected '("foo@bar.com"
+		     "foo@bar.com"
+		     "Foo Bar <foo@bar.com>"
+		     "foo@bar.com"
+		     "Foo Bar <foo@bar.com>"))
+	 (output (mapcar #'notmuch-show-clean-address input)))
+    (notmuch-test-compare output expected)))
+
+(defun notmuch-test-address-cleaning-3 ()
+  (let* ((input '("ДБ <db-uknot@stop.me.uk>"
+		  "foo (at home) <foo@bar.com>"
+		  "foo [at home] <foo@bar.com>"
+		  "Foo Bar"))
+	 (expected '("ДБ <db-uknot@stop.me.uk>"
+		     "foo (at home) <foo@bar.com>"
+		     "foo [at home] <foo@bar.com>"
+		     "Foo Bar"))
+	 (output (mapcar #'notmuch-show-clean-address input)))
+    (notmuch-test-compare output expected)))
diff --git a/test/emacs-address-cleaning.sh b/test/emacs-address-cleaning.sh
new file mode 100755
index 0000000..1a6eff5
--- /dev/null
+++ b/test/emacs-address-cleaning.sh
@@ -0,0 +1,12 @@
+#!/usr/bin/env bash
+
+test_description="emacs address cleaning"
+. test-lib.sh
+
+for i in 1 2 3; do
+    test_begin_subtest "notmuch-test-address-clean-$i"
+    test_emacs_expect_t \
+	'(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-'$i')'
+done
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index d034f99..3f1740c 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -53,6 +53,7 @@ TESTS="
   hooks
   argument-parsing
   emacs-test-functions.sh
+  emacs-address-cleaning.sh
 "
 TESTS=${NOTMUCH_TESTS:=$TESTS}
 
diff --git a/test/test-lib.el b/test/test-lib.el
index 3b817c3..cf8a46d 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -20,6 +20,8 @@
 ;;
 ;; Authors: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
 
+(require 'cl)	;; This code is generally used uncompiled.
+
 ;; `read-file-name' by default uses `completing-read' function to read
 ;; user input.  It does not respect `standard-input' variable which we
 ;; use in tests to provide user input.  So replace it with a plain
@@ -76,3 +78,30 @@ nothing."
 
 (add-hook-counter 'notmuch-hello-mode-hook)
 (add-hook-counter 'notmuch-hello-refresh-hook)
+
+;; Functions to help when writing tests:
+
+(defun notmuch-test-reporter (output expected)
+  "Report that the `output' does not match the `expected' result."
+  (concat "Expect:\t" (prin1-to-string expected) "\n"
+	  "Output:\t" (prin1-to-string output) "\n"))
+
+(defun notmuch-test-unsimilar (output expected)
+  "`output' and `expected' are dissimilar. Show a summary of
+the differences, ignoring similarities."
+  (cond ((and (listp output)
+	      (listp expected))
+	 (apply #'concat (loop for o in output
+			       for e in expected
+			       if (not (equal o e))
+			       collect (notmuch-test-reporter o e))))
+
+	(t
+	 ;; Catch all case.
+	 (notmuch-test-reporter output expected))))
+
+(defun notmuch-test-compare (output expected)
+  "Compare `output' with `expected'. Report any discrepencies."
+  (if (equal output expected)
+      t
+    (notmuch-test-unsimilar output expected)))
-- 
1.7.7.3

Re: [PATCH 1/3] test: Don't return the result of checking for running emacs to the tester.

[Dmitry Kurochkin]

Can you please elaborate why this is needed?

Regards,
  Dmitry

Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.

[Dmitry Kurochkin]

On Tue, 17 Jan 2012 14:07:03 +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.
> ---
> 
> Re-worked as Dmitry suggested.
> 
>  test/README                  |    8 ++++++++
>  test/emacs-test-functions.sh |    9 +++++++++
>  test/notmuch-test            |    1 +
>  test/test-lib.sh             |   33 +++++++++++++++++++++++++++++++++
>  4 files changed, 51 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.sh b/test/test-lib.sh
> index 7c9ce24..4b05760 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -503,6 +503,39 @@ 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
> +		# 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 "$1" > "${output}"
> +		result=$(cat "${output}")
> +		rm -f "${output}"
> +	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
> +		if [ "$result" == t ]
> +		then
> +			test_ok_ "$test_subtest_name"
> +		else
> +			test_failure_ "$test_subtest_name" "$(eval printf ${result})"
> +		fi
> +	fi
> +}
> +

Sorry, I still do not understand why we can not implement
test_emacs_expect_t() like:

  result=${test_emacs $@}
  test_expect_equal $result t

Can you please explain?

Regards,
  Dmitry

>  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

Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 752 bytes --]

On Tue, 17 Jan 2012 18:26:41 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Sorry, I still do not understand why we can not implement
> test_emacs_expect_t() like:
> 
>   result=${test_emacs $@}
>   test_expect_equal $result t
> 
> Can you please explain?

In the failure case test_expect_equal does:

  test_failure_ "$test_subtest_name" "$(diff -u $testname.expected $testname.output)"

that diff output is not useful here, because the test harness doesn't
have any expected output other than `t' with which to diff the actual
output.

The emacs-address-cleaning test shows how we will provide expected
vs. actual output directly from within emacs, making it easier for the
developer to figure out what went wrong.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/3] test: Don't return the result of checking for running emacs to the tester.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 382 bytes --]

(And for the list...)

On Tue, 17 Jan 2012 18:20:04 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Can you please elaborate why this is needed?

This code:

		    # wait until the emacs server is up
		    until test_emacs '()' 2>/dev/null; do
			    sleep 1
		    done

outputs 'nil', so the first caller to test_emacs has 'nil\n' prepended
to their expected output.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.

[Dmitry Kurochkin]

On Tue, 17 Jan 2012 14:35:07 +0000, David Edmondson <dme@dme.org> wrote:
> On Tue, 17 Jan 2012 18:26:41 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Sorry, I still do not understand why we can not implement
> > test_emacs_expect_t() like:
> > 
> >   result=${test_emacs $@}
> >   test_expect_equal $result t
> > 
> > Can you please explain?
> 
> In the failure case test_expect_equal does:
> 
>   test_failure_ "$test_subtest_name" "$(diff -u $testname.expected $testname.output)"
> 
> that diff output is not useful here, because the test harness doesn't
> have any expected output other than `t' with which to diff the actual
> output.
> 
> The emacs-address-cleaning test shows how we will provide expected
> vs. actual output directly from within emacs, making it easier for the
> developer to figure out what went wrong.

Thanks for the explanation.

Regards,
  Dmitry

Re: [PATCH 1/3] test: Don't return the result of checking for running emacs to the tester.

[Dmitry Kurochkin]

On Tue, 17 Jan 2012 14:37:52 +0000, David Edmondson <dme@dme.org> wrote:
> (And for the list...)
> 
> On Tue, 17 Jan 2012 18:20:04 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Can you please elaborate why this is needed?
> 
> This code:
> 
> 		    # wait until the emacs server is up
> 		    until test_emacs '()' 2>/dev/null; do
> 			    sleep 1
> 		    done
> 
> outputs 'nil', so the first caller to test_emacs has 'nil\n' prepended
> to their expected output.

Thanks.  Would be nice to have this explained in the commit message.  No
need to resend just because of this though.

Regards,
  Dmitry

Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 941 bytes --]

On Tue, 17 Jan 2012 18:49:36 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > +		# 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 "$1" > "${output}"
> > +		result=$(cat "${output}")
> > +		rm -f "${output}"
> 
> I wonder if there is any bash trick which can help here?

I'm not aware of one, but I'm not a bash expert.

> Another option is to start emacs server before using test_emacs in
> subshell.  See emacs-subject-to-filename for an example.  I think this
> is a better option than using a temporary file.

I think that's a very poor option. Forcing knowledge the breakage into
all of the users may make applying any future fix more difficult.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.

[Tomi Ollila]

On Tue, 17 Jan 2012 15:09:22 +0000, David Edmondson <dme@dme.org> wrote:
> On Tue, 17 Jan 2012 18:49:36 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > > +		# 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 "$1" > "${output}"
> > > +		result=$(cat "${output}")
> > > +		rm -f "${output}"
> > 
> > I wonder if there is any bash trick which can help here?
> 
> I'm not aware of one, but I'm not a bash expert.

No shell trick possible there.

> 
> > Another option is to start emacs server before using test_emacs in
> > subshell.  See emacs-subject-to-filename for an example.  I think this
> > is a better option than using a temporary file.
> 
> I think that's a very poor option. Forcing knowledge the breakage into
> all of the users may make applying any future fix more difficult.

The alternative would be to replace
emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"

with something like:

   case $1 in --stdout-to-output)
	shift
	output=$(emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)")
        return
   esac
   emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"

Tomi

Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.

[Tomi Ollila]

On Tue, 17 Jan 2012 14:07:03 +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.
> ---
> 
> Re-worked as Dmitry suggested.
> 
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 7c9ce24..4b05760 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -503,6 +503,39 @@ 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
> +		# 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 "$1" > "${output}"
> +		result=$(cat "${output}")
> +		rm -f "${output}"
> +	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
> +		if [ "$result" == t ]

		if [ "$result" = t ]

to be compatible with POSIX and consistent with rest code.

> +		then
> +			test_ok_ "$test_subtest_name"
> +		else
> +			test_failure_ "$test_subtest_name" "$(eval printf ${result})"

This added 'eval' made me investigate further... running 

emacsclient --eval '(print (concat "a" "b" "\t" "c" "\n" "z"))'

outputs "ab	c\nz" (tab between 'ab' and 'c', quotes (") around
the whole output and newlines as "\n" (even '\r' is converted)).

If emacs tests run via test_emacs_expect_t wrote their output 
to ${output} directly above code could be much cleaner in many
places. Environment variable could be used for the file name.

Tomi

Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 928 bytes --]

On Wed, 18 Jan 2012 16:55:59 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> > +	# Report success/failure.
> > +	if ! test_skip "$test_subtest_name"
> > +	then
> > +		if [ "$result" == t ]
> 
> 		if [ "$result" = t ]
> 
> to be compatible with POSIX and consistent with rest code.

I'm happy to change this.

> > +			test_failure_ "$test_subtest_name" "$(eval printf ${result})"
> 
> This added 'eval' made me investigate further... running 
> 
> emacsclient --eval '(print (concat "a" "b" "\t" "c" "\n" "z"))'
> 
> outputs "ab	c\nz" (tab between 'ab' and 'c', quotes (") around
> the whole output and newlines as "\n" (even '\r' is converted)).
> 
> If emacs tests run via test_emacs_expect_t wrote their output 
> to ${output} directly above code could be much cleaner in many
> places. Environment variable could be used for the file name.

I'm reluctant to change this just to avoid calling eval.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.

[Tomi Ollila]

On Thu, 19 Jan 2012 09:59:16 +0000, David Edmondson <dme@dme.org> wrote:
> On Wed, 18 Jan 2012 16:55:59 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> > > +	# Report success/failure.
> > > +	if ! test_skip "$test_subtest_name"
> > > +	then
> > > +		if [ "$result" == t ]
> > 
> > 		if [ "$result" = t ]
> > 
> > to be compatible with POSIX and consistent with rest code.
> 
> I'm happy to change this.

As your older patch has been marked obsolete and this really is the
only place where there is == in comparison code please do that change.

> > > +			test_failure_ "$test_subtest_name" "$(eval printf ${result})"
> > 
> > This added 'eval' made me investigate further... running 
> > 
> > emacsclient --eval '(print (concat "a" "b" "\t" "c" "\n" "z"))'
> > 
> > outputs "ab	c\nz" (tab between 'ab' and 'c', quotes (") around
> > the whole output and newlines as "\n" (even '\r' is converted)).
> > 
> > If emacs tests run via test_emacs_expect_t wrote their output 
> > to ${output} directly above code could be much cleaner in many
> > places. Environment variable could be used for the file name.
> 
> I'm reluctant to change this just to avoid calling eval.

Consider the following:

$ emacsclient --eval '(print "$(echo rm -rf /); echo `date +%Y`")'
"$(echo rm -rf /); echo `date +%Y`"

x='"$(echo rm -rf /); echo `date +%Y`"'

$echo $x
"$(echo rm -rf /); echo `date +%Y`"

$echo $(eval printf $x)
rm -rf /; echo 2012

x='"$(echo rm /); echo `date +%Y`"'

$(eval printf $x)
rm: cannot remove `/;': No such file or directory
rm: cannot remove `2012': No such file or directory

one just needs to make sure there is no $:s and `: in 
the output... hmm, nor ';' ...NOR '&':s ...

I am not absolutely sure this might actually happen
but surely I'm not sure it would not...

... The suggestion having environment variable would
now work, but what about

test_emacs "(setq test-output-file \"${output}\") $1"


Tomi

PS:

this needs to be changed:
	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
to
	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $*)"
then
	test_emacs   "(setq test-output-file \"${output}\")"   "$1"

would be safe

Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 299 bytes --]

On Thu, 19 Jan 2012 12:32:21 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> Consider the following:
> 
> $ emacsclient --eval '(print "$(echo rm -rf /); echo `date +%Y`")'
> "$(echo rm -rf /); echo `date +%Y`"

Or:

$ emacsclient --eval '(shell-command "rm -rf /")'

What's your point?

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.

[Tomi Ollila]

On Thu, 19 Jan 2012 10:42:31 +0000, David Edmondson <dme@dme.org> wrote:
> On Thu, 19 Jan 2012 12:32:21 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> > Consider the following:
> > 
> > $ emacsclient --eval '(print "$(echo rm -rf /); echo `date +%Y`")'
> > "$(echo rm -rf /); echo `date +%Y`"
> 
> Or:
> 
> $ emacsclient --eval '(shell-command "rm -rf /")'
> 
> What's your point?

The point it that you need to quote all shell metacharacters 
(at least '$', '`', ';' and '&') always in your test output for eval:ing...
Writing to file from elisp test function for comparison is simpler.

Tomi

[PATCH 1/3] test: Don't return the result of checking for running emacs to the tester.

[David Edmondson]

---
 test/test-lib.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index d1fbc05..7c9ce24 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -926,7 +926,7 @@ test_emacs () {
 				--eval '(orphan-watchdog $$)'" || return
 		EMACS_SERVER="$server_name"
 		# wait until the emacs server is up
-		until test_emacs '()' 2>/dev/null; do
+		until test_emacs '()' >/dev/null 2>/dev/null; do
 			sleep 1
 		done
 	fi
-- 
1.7.8.3

[PATCH 2/3] test: Add `test_emacs_expect_t'.

[David Edmondson]

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)
+  "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
+}
+
 NOTMUCH_NEW ()
 {
     notmuch new | grep -v -E -e '^Processed [0-9]*( total)? file|Found [0-9]* total file'
-- 
1.7.8.3

[PATCH 3/3] test: Add address cleaning tests.

[David Edmondson]

Including some more test framework in test-lib.el.
---
 test/emacs-address-cleaning.el |   29 +++++++++++++++++++++++++++++
 test/emacs-address-cleaning.sh |   12 ++++++++++++
 test/notmuch-test              |    1 +
 test/test-lib.el               |   30 ++++++++++++++++++++++++++++++
 4 files changed, 72 insertions(+), 0 deletions(-)
 create mode 100644 test/emacs-address-cleaning.el
 create mode 100755 test/emacs-address-cleaning.sh

diff --git a/test/emacs-address-cleaning.el b/test/emacs-address-cleaning.el
new file mode 100644
index 0000000..59e8d92
--- /dev/null
+++ b/test/emacs-address-cleaning.el
@@ -0,0 +1,29 @@
+(defun notmuch-test-address-cleaning-1 ()
+  (notmuch-test-compare (notmuch-show-clean-address "dme@dme.org")
+			"dme@dme.org"))
+
+(defun notmuch-test-address-cleaning-2 ()
+  (let* ((input '("foo@bar.com"
+		  "<foo@bar.com>"
+		  "Foo Bar <foo@bar.com>"
+		  "foo@bar.com <foo@bar.com>"
+		  "\"Foo Bar\" <foo@bar.com>"))
+	 (expected '("foo@bar.com"
+		     "foo@bar.com"
+		     "Foo Bar <foo@bar.com>"
+		     "foo@bar.com"
+		     "Foo Bar <foo@bar.com>"))
+	 (output (mapcar #'notmuch-show-clean-address input)))
+    (notmuch-test-compare output expected)))
+
+(defun notmuch-test-address-cleaning-3 ()
+  (let* ((input '("ДБ <db-uknot@stop.me.uk>"
+		  "foo (at home) <foo@bar.com>"
+		  "foo [at home] <foo@bar.com>"
+		  "Foo Bar"))
+	 (expected '("ДБ <db-uknot@stop.me.uk>"
+		     "foo (at home) <foo@bar.com>"
+		     "foo [at home] <foo@bar.com>"
+		     "Foo Bar"))
+	 (output (mapcar #'notmuch-show-clean-address input)))
+    (notmuch-test-compare output expected)))
diff --git a/test/emacs-address-cleaning.sh b/test/emacs-address-cleaning.sh
new file mode 100755
index 0000000..1a6eff5
--- /dev/null
+++ b/test/emacs-address-cleaning.sh
@@ -0,0 +1,12 @@
+#!/usr/bin/env bash
+
+test_description="emacs address cleaning"
+. test-lib.sh
+
+for i in 1 2 3; do
+    test_begin_subtest "notmuch-test-address-clean-$i"
+    test_emacs_expect_t \
+	'(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-'$i')'
+done
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index d034f99..3f1740c 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -53,6 +53,7 @@ TESTS="
   hooks
   argument-parsing
   emacs-test-functions.sh
+  emacs-address-cleaning.sh
 "
 TESTS=${NOTMUCH_TESTS:=$TESTS}
 
diff --git a/test/test-lib.el b/test/test-lib.el
index 1d51b8d..033270d 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -20,6 +20,8 @@
 ;;
 ;; Authors: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
 
+(require 'cl)	;; This code is generally used uncompiled.
+
 ;; `read-file-name' by default uses `completing-read' function to read
 ;; user input.  It does not respect `standard-input' variable which we
 ;; use in tests to provide user input.  So replace it with a plain
@@ -77,6 +79,7 @@ nothing."
 (add-hook-counter 'notmuch-hello-mode-hook)
 (add-hook-counter 'notmuch-hello-refresh-hook)
 
+
 (defmacro notmuch-test-run-test (&rest body)
   "Evaluate a BODY of test expressions and output the result."
   `(with-temp-buffer
@@ -85,3 +88,30 @@ nothing."
 		   result
 		 (prin1-to-string result)))
        (test-output))))
+
+;; Functions to help when writing tests:
+
+(defun notmuch-test-reporter (output expected)
+  "Report that the `output' does not match the `expected' result."
+  (concat "Expect:\t" (prin1-to-string expected) "\n"
+	  "Output:\t" (prin1-to-string output) "\n"))
+
+(defun notmuch-test-unsimilar (output expected)
+  "`output' and `expected' are dissimilar. Show a summary of
+the differences, ignoring similarities."
+  (cond ((and (listp output)
+	      (listp expected))
+	 (apply #'concat (loop for o in output
+			       for e in expected
+			       if (not (equal o e))
+			       collect (notmuch-test-reporter o e))))
+
+	(t
+	 ;; Catch all case.
+	 (notmuch-test-reporter output expected))))
+
+(defun notmuch-test-compare (output expected)
+  "Compare `output' with `expected'. Report any discrepencies."
+  (if (equal output expected)
+      t
+    (notmuch-test-unsimilar output expected)))
-- 
1.7.8.3

Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

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.

I've dealt with all of the comments for this patchset that I received
with one exception (declaring test 3 as failing in patch 3).

Any other comments before I submit another version?

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/3] test: Don't return the result of checking for running emacs to the tester.

[Dmitry Kurochkin]

LGTM.

Regards,
  Dmitry

Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.

[Dmitry Kurochkin]

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

Re: [PATCH 3/3] test: Add address cleaning tests.

[Dmitry Kurochkin]

On Thu, 19 Jan 2012 12:54:03 +0000, David Edmondson <dme@dme.org> wrote:
> Including some more test framework in test-lib.el.

IMO test-lib.el changes should be in a separate patch.

> ---
>  test/emacs-address-cleaning.el |   29 +++++++++++++++++++++++++++++
>  test/emacs-address-cleaning.sh |   12 ++++++++++++
>  test/notmuch-test              |    1 +
>  test/test-lib.el               |   30 ++++++++++++++++++++++++++++++
>  4 files changed, 72 insertions(+), 0 deletions(-)
>  create mode 100644 test/emacs-address-cleaning.el
>  create mode 100755 test/emacs-address-cleaning.sh
> 
> diff --git a/test/emacs-address-cleaning.el b/test/emacs-address-cleaning.el
> new file mode 100644
> index 0000000..59e8d92
> --- /dev/null
> +++ b/test/emacs-address-cleaning.el
> @@ -0,0 +1,29 @@
> +(defun notmuch-test-address-cleaning-1 ()
> +  (notmuch-test-compare (notmuch-show-clean-address "dme@dme.org")
> +			"dme@dme.org"))
> +
> +(defun notmuch-test-address-cleaning-2 ()
> +  (let* ((input '("foo@bar.com"
> +		  "<foo@bar.com>"
> +		  "Foo Bar <foo@bar.com>"
> +		  "foo@bar.com <foo@bar.com>"
> +		  "\"Foo Bar\" <foo@bar.com>"))
> +	 (expected '("foo@bar.com"
> +		     "foo@bar.com"
> +		     "Foo Bar <foo@bar.com>"
> +		     "foo@bar.com"
> +		     "Foo Bar <foo@bar.com>"))
> +	 (output (mapcar #'notmuch-show-clean-address input)))
> +    (notmuch-test-compare output expected)))
> +
> +(defun notmuch-test-address-cleaning-3 ()
> +  (let* ((input '("ДБ <db-uknot@stop.me.uk>"
> +		  "foo (at home) <foo@bar.com>"
> +		  "foo [at home] <foo@bar.com>"
> +		  "Foo Bar"))
> +	 (expected '("ДБ <db-uknot@stop.me.uk>"
> +		     "foo (at home) <foo@bar.com>"
> +		     "foo [at home] <foo@bar.com>"
> +		     "Foo Bar"))
> +	 (output (mapcar #'notmuch-show-clean-address input)))
> +    (notmuch-test-compare output expected)))

At a glance, the tests look good.  Though more review of the tests
itself would be nice.

> diff --git a/test/emacs-address-cleaning.sh b/test/emacs-address-cleaning.sh
> new file mode 100755
> index 0000000..1a6eff5
> --- /dev/null
> +++ b/test/emacs-address-cleaning.sh
> @@ -0,0 +1,12 @@
> +#!/usr/bin/env bash
> +
> +test_description="emacs address cleaning"
> +. test-lib.sh
> +
> +for i in 1 2 3; do
> +    test_begin_subtest "notmuch-test-address-clean-$i"
> +    test_emacs_expect_t \
> +	'(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-'$i')'
> +done
> +
> +test_done
> diff --git a/test/notmuch-test b/test/notmuch-test
> index d034f99..3f1740c 100755
> --- a/test/notmuch-test
> +++ b/test/notmuch-test
> @@ -53,6 +53,7 @@ TESTS="
>    hooks
>    argument-parsing
>    emacs-test-functions.sh
> +  emacs-address-cleaning.sh
>  "
>  TESTS=${NOTMUCH_TESTS:=$TESTS}
>  
> diff --git a/test/test-lib.el b/test/test-lib.el
> index 1d51b8d..033270d 100644
> --- a/test/test-lib.el
> +++ b/test/test-lib.el
> @@ -20,6 +20,8 @@
>  ;;
>  ;; Authors: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
>  
> +(require 'cl)	;; This code is generally used uncompiled.
> +
>  ;; `read-file-name' by default uses `completing-read' function to read
>  ;; user input.  It does not respect `standard-input' variable which we
>  ;; use in tests to provide user input.  So replace it with a plain
> @@ -77,6 +79,7 @@ nothing."
>  (add-hook-counter 'notmuch-hello-mode-hook)
>  (add-hook-counter 'notmuch-hello-refresh-hook)
>  
> +

Please revert.

>  (defmacro notmuch-test-run-test (&rest body)
>    "Evaluate a BODY of test expressions and output the result."
>    `(with-temp-buffer
> @@ -85,3 +88,30 @@ nothing."
>  		   result
>  		 (prin1-to-string result)))
>         (test-output))))
> +
> +;; Functions to help when writing tests:
> +

IMO this comment is misplaced.  There are other functions above which
also help to write tests e.g. `test-output.  I think we should either
remove the comment or move all test helpers in one place below the
comment (in a separate patch).

> +(defun notmuch-test-reporter (output expected)

Please consider renaming to `notmuch-test-report-unexpected'.

> +  "Report that the `output' does not match the `expected' result."
> +  (concat "Expect:\t" (prin1-to-string expected) "\n"
> +	  "Output:\t" (prin1-to-string output) "\n"))
> +
> +(defun notmuch-test-unsimilar (output expected)

Please consider renaming to notmuch-test-unexpected.

> +  "`output' and `expected' are dissimilar. Show a summary of

Previous line says "unsimilar", now it is "dissimilar".  I am not sure
which one is correct.  Also, it is not clear what "similar" means for
test results.  IMO "not equal" would be more clear.

> +the differences, ignoring similarities."
> +  (cond ((and (listp output)
> +	      (listp expected))
> +	 (apply #'concat (loop for o in output
> +			       for e in expected
> +			       if (not (equal o e))
> +			       collect (notmuch-test-reporter o e))))
> +
> +	(t
> +	 ;; Catch all case.
> +	 (notmuch-test-reporter output expected))))
> +
> +(defun notmuch-test-compare (output expected)

Please consider renaming to notmuch-test-expect-equal for consistency
with shell tests.

> +  "Compare `output' with `expected'. Report any discrepencies."
> +  (if (equal output expected)
> +      t
> +    (notmuch-test-unsimilar output expected)))

I do not like that equality check is split between notmuch-test-compare
and notmuch-test-unsimilar.  It makes the code harder to read (took me a
while to understand why "catch all case" above does not need the
equality check).  Also, notmuch-test-unsimilar would be hard to reuse
because the comparison check is done in one case and not done in
another.  How about merging them?  The code would look something like:

  (cond
    (both are lists)
      ; code from notmuch-test-unsimilar
    (catch all case)
      (if not equal)
        (notmuch-test-report-unexpected output expected))

Regards,
  Dmitry

> -- 
> 1.7.8.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 1/4 v42] test: Don't return the result of checking for running emacs to the tester.

[David Edmondson]

When checking for a running emacs, test_emacs evaluates the empty list
'()'. This returns 'nil' when emacs is running, which is then
prepended to the actual test result. Given that it is not part of the
actual test output the test harness can incorrectly report test
failure (or success).
---

Commentary updated.

 test/test-lib.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 0da60fb..82c686c 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -926,7 +926,7 @@ test_emacs () {
 				--eval '(orphan-watchdog $$)'" || return
 		EMACS_SERVER="$server_name"
 		# wait until the emacs server is up
-		until test_emacs '()' 2>/dev/null; do
+		until test_emacs '()' >/dev/null 2>/dev/null; do
 			sleep 1
 		done
 	fi
-- 
1.7.8.3

[PATCH 2/4 v42] test: Add `test_emacs_expect_t'.

[David Edmondson]

Add a new test function to allow simpler testing of emacs
functionality.

`test_emacs_expect_t' takes one argument - a lisp expression to
evaluate. The test passes if the expression returns `t', otherwise it
fails and the output is reported to the tester.
---

As per Dmitry:
 - don't call `test_skip' twice,
 - allow for a prereq.

 test/README                  |    8 ++++++++
 test/emacs-test-functions.sh |    9 +++++++++
 test/notmuch-test            |    1 +
 test/test-lib.el             |    9 +++++++++
 test/test-lib.sh             |   29 +++++++++++++++++++++++++++++
 5 files changed, 56 insertions(+), 0 deletions(-)
 create mode 100755 test/emacs-test-functions.sh

diff --git a/test/README b/test/README
index 44ff653..43656a3 100644
--- a/test/README
+++ b/test/README
@@ -202,6 +202,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 59c5868..96752f0 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -83,3 +83,12 @@ nothing."
 
 (add-hook-counter 'notmuch-hello-mode-hook)
 (add-hook-counter 'notmuch-hello-refresh-hook)
+
+(defmacro notmuch-test-run (&rest body)
+  "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 82c686c..8158328 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -503,6 +503,35 @@ test_expect_equal_file ()
     fi
 }
 
+test_emacs_expect_t () {
+	test "$#" = 2 && { prereq=$1; shift; } || prereq=
+	test "$#" = 1 ||
+	error "bug in the test script: not 1 or 2 parameters to test_emacs_expect_t"
+
+	# Run the test.
+	if ! test_skip "$test_subtest_name"
+	then
+		test_emacs "(notmuch-test-run $1)" >/dev/null
+
+		# Restore state after the test.
+		exec 1>&6 2>&7		# Restore stdout and stderr
+		inside_subtest=
+
+		# Report success/failure.
+		result=$(cat OUTPUT)
+		if [ "$result" = t ]
+		then
+			test_ok_ "$test_subtest_name"
+		else
+			test_failure_ "$test_subtest_name" "${result}"
+		fi
+	else
+		# Restore state after the (non) test.
+		exec 1>&6 2>&7		# Restore stdout and stderr
+		inside_subtest=
+	fi
+}
+
 NOTMUCH_NEW ()
 {
     notmuch new | grep -v -E -e '^Processed [0-9]*( total)? file|Found [0-9]* total file'
-- 
1.7.8.3

[PATCH 3/4 v42] test: Add more helpers for emacs tests.

[David Edmondson]

---

Split out from the tests and re-factored.

 test/test-lib.el |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index 96752f0..c4a5db4 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -20,6 +20,8 @@
 ;;
 ;; Authors: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
 
+(require 'cl)	;; This code is generally used uncompiled.
+
 ;; `read-file-name' by default uses `completing-read' function to read
 ;; user input.  It does not respect `standard-input' variable which we
 ;; use in tests to provide user input.  So replace it with a plain
@@ -92,3 +94,23 @@ nothing."
 		   result
 		 (prin1-to-string result)))
        (test-output))))
+
+(defun notmuch-test-report-unexpected (output expected)
+  "Report that the OUTPUT does not match the EXPECTED result."
+  (concat "Expect:\t" (prin1-to-string expected) "\n"
+	  "Output:\t" (prin1-to-string output) "\n"))
+
+(defun notmuch-test-compare (output expected)
+  "Compare OUTPUT with EXPECTED. Report any discrepencies."
+  (if (equal output expected)
+      t
+    (cond
+     ((and (listp output)
+	   (listp expected))
+      (apply #'concat (loop for o in output
+			    for e in expected
+			    if (not (equal o e))
+			    collect (notmuch-test-report-unexpected o e))))
+   
+     (t
+      (notmuch-test-report-unexpected output expected)))))
-- 
1.7.8.3

[PATCH 4/4 v42] test: Add address cleaning tests.

[David Edmondson]

---

Anticipate the failure of test 3.

 test/emacs-address-cleaning.el |   29 +++++++++++++++++++++++++++++
 test/emacs-address-cleaning.sh |   19 +++++++++++++++++++
 test/notmuch-test              |    1 +
 3 files changed, 49 insertions(+), 0 deletions(-)
 create mode 100644 test/emacs-address-cleaning.el
 create mode 100755 test/emacs-address-cleaning.sh

diff --git a/test/emacs-address-cleaning.el b/test/emacs-address-cleaning.el
new file mode 100644
index 0000000..59e8d92
--- /dev/null
+++ b/test/emacs-address-cleaning.el
@@ -0,0 +1,29 @@
+(defun notmuch-test-address-cleaning-1 ()
+  (notmuch-test-compare (notmuch-show-clean-address "dme@dme.org")
+			"dme@dme.org"))
+
+(defun notmuch-test-address-cleaning-2 ()
+  (let* ((input '("foo@bar.com"
+		  "<foo@bar.com>"
+		  "Foo Bar <foo@bar.com>"
+		  "foo@bar.com <foo@bar.com>"
+		  "\"Foo Bar\" <foo@bar.com>"))
+	 (expected '("foo@bar.com"
+		     "foo@bar.com"
+		     "Foo Bar <foo@bar.com>"
+		     "foo@bar.com"
+		     "Foo Bar <foo@bar.com>"))
+	 (output (mapcar #'notmuch-show-clean-address input)))
+    (notmuch-test-compare output expected)))
+
+(defun notmuch-test-address-cleaning-3 ()
+  (let* ((input '("ДБ <db-uknot@stop.me.uk>"
+		  "foo (at home) <foo@bar.com>"
+		  "foo [at home] <foo@bar.com>"
+		  "Foo Bar"))
+	 (expected '("ДБ <db-uknot@stop.me.uk>"
+		     "foo (at home) <foo@bar.com>"
+		     "foo [at home] <foo@bar.com>"
+		     "Foo Bar"))
+	 (output (mapcar #'notmuch-show-clean-address input)))
+    (notmuch-test-compare output expected)))
diff --git a/test/emacs-address-cleaning.sh b/test/emacs-address-cleaning.sh
new file mode 100755
index 0000000..0d85bdc
--- /dev/null
+++ b/test/emacs-address-cleaning.sh
@@ -0,0 +1,19 @@
+#!/usr/bin/env bash
+
+test_description="emacs address cleaning"
+. test-lib.sh
+
+test_begin_subtest "notmuch-test-address-clean part 1"
+test_emacs_expect_t \
+    '(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-1)'
+
+test_begin_subtest "notmuch-test-address-clean part 2"
+test_emacs_expect_t \
+    '(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-2)'
+
+test_begin_subtest "notmuch-test-address-clean part 3"
+test_subtest_known_broken
+test_emacs_expect_t \
+    '(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-3)'
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index d034f99..3f1740c 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -53,6 +53,7 @@ TESTS="
   hooks
   argument-parsing
   emacs-test-functions.sh
+  emacs-address-cleaning.sh
 "
 TESTS=${NOTMUCH_TESTS:=$TESTS}
 
-- 
1.7.8.3

Re: [PATCH 1/4 v42] test: Don't return the result of checking for running emacs to the tester.

[Dmitry Kurochkin]

On Mon, 23 Jan 2012 18:05:44 +0000, David Edmondson <dme@dme.org> wrote:
> When checking for a running emacs, test_emacs evaluates the empty list
> '()'. This returns 'nil' when emacs is running, which is then
> prepended to the actual test result. Given that it is not part of the
> actual test output the test harness can incorrectly report test
> failure (or success).
> ---
> 
> Commentary updated.
> 

LGTM

Regards,
  Dmitry

>  test/test-lib.sh |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 0da60fb..82c686c 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -926,7 +926,7 @@ test_emacs () {
>  				--eval '(orphan-watchdog $$)'" || return
>  		EMACS_SERVER="$server_name"
>  		# wait until the emacs server is up
> -		until test_emacs '()' 2>/dev/null; do
> +		until test_emacs '()' >/dev/null 2>/dev/null; do
>  			sleep 1
>  		done
>  	fi
> -- 
> 1.7.8.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

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

[Dmitry Kurochkin]

On Mon, 23 Jan 2012 18:05:45 +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 lisp expression to
> evaluate. The test passes if the expression returns `t', otherwise it
> fails and the output is reported to the tester.
> ---
> 
> As per Dmitry:
>  - don't call `test_skip' twice,
>  - allow for a prereq.
> 

LGTM

Regards,
  Dmitry

>  test/README                  |    8 ++++++++
>  test/emacs-test-functions.sh |    9 +++++++++
>  test/notmuch-test            |    1 +
>  test/test-lib.el             |    9 +++++++++
>  test/test-lib.sh             |   29 +++++++++++++++++++++++++++++
>  5 files changed, 56 insertions(+), 0 deletions(-)
>  create mode 100755 test/emacs-test-functions.sh
> 
> diff --git a/test/README b/test/README
> index 44ff653..43656a3 100644
> --- a/test/README
> +++ b/test/README
> @@ -202,6 +202,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 59c5868..96752f0 100644
> --- a/test/test-lib.el
> +++ b/test/test-lib.el
> @@ -83,3 +83,12 @@ nothing."
>  
>  (add-hook-counter 'notmuch-hello-mode-hook)
>  (add-hook-counter 'notmuch-hello-refresh-hook)
> +
> +(defmacro notmuch-test-run (&rest body)
> +  "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 82c686c..8158328 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -503,6 +503,35 @@ test_expect_equal_file ()
>      fi
>  }
>  
> +test_emacs_expect_t () {
> +	test "$#" = 2 && { prereq=$1; shift; } || prereq=
> +	test "$#" = 1 ||
> +	error "bug in the test script: not 1 or 2 parameters to test_emacs_expect_t"
> +
> +	# Run the test.
> +	if ! test_skip "$test_subtest_name"
> +	then
> +		test_emacs "(notmuch-test-run $1)" >/dev/null
> +
> +		# Restore state after the test.
> +		exec 1>&6 2>&7		# Restore stdout and stderr
> +		inside_subtest=
> +
> +		# Report success/failure.
> +		result=$(cat OUTPUT)
> +		if [ "$result" = t ]
> +		then
> +			test_ok_ "$test_subtest_name"
> +		else
> +			test_failure_ "$test_subtest_name" "${result}"
> +		fi
> +	else
> +		# Restore state after the (non) test.
> +		exec 1>&6 2>&7		# Restore stdout and stderr
> +		inside_subtest=
> +	fi
> +}
> +
>  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

Re: [PATCH 4/4 v42] test: Add address cleaning tests.

[Dmitry Kurochkin]

On Mon, 23 Jan 2012 18:05:47 +0000, David Edmondson <dme@dme.org> wrote:
> ---
> 
> Anticipate the failure of test 3.
> 

LGTM

Regards,
  Dmitry

>  test/emacs-address-cleaning.el |   29 +++++++++++++++++++++++++++++
>  test/emacs-address-cleaning.sh |   19 +++++++++++++++++++
>  test/notmuch-test              |    1 +
>  3 files changed, 49 insertions(+), 0 deletions(-)
>  create mode 100644 test/emacs-address-cleaning.el
>  create mode 100755 test/emacs-address-cleaning.sh
> 
> diff --git a/test/emacs-address-cleaning.el b/test/emacs-address-cleaning.el
> new file mode 100644
> index 0000000..59e8d92
> --- /dev/null
> +++ b/test/emacs-address-cleaning.el
> @@ -0,0 +1,29 @@
> +(defun notmuch-test-address-cleaning-1 ()
> +  (notmuch-test-compare (notmuch-show-clean-address "dme@dme.org")
> +			"dme@dme.org"))
> +
> +(defun notmuch-test-address-cleaning-2 ()
> +  (let* ((input '("foo@bar.com"
> +		  "<foo@bar.com>"
> +		  "Foo Bar <foo@bar.com>"
> +		  "foo@bar.com <foo@bar.com>"
> +		  "\"Foo Bar\" <foo@bar.com>"))
> +	 (expected '("foo@bar.com"
> +		     "foo@bar.com"
> +		     "Foo Bar <foo@bar.com>"
> +		     "foo@bar.com"
> +		     "Foo Bar <foo@bar.com>"))
> +	 (output (mapcar #'notmuch-show-clean-address input)))
> +    (notmuch-test-compare output expected)))
> +
> +(defun notmuch-test-address-cleaning-3 ()
> +  (let* ((input '("ДБ <db-uknot@stop.me.uk>"
> +		  "foo (at home) <foo@bar.com>"
> +		  "foo [at home] <foo@bar.com>"
> +		  "Foo Bar"))
> +	 (expected '("ДБ <db-uknot@stop.me.uk>"
> +		     "foo (at home) <foo@bar.com>"
> +		     "foo [at home] <foo@bar.com>"
> +		     "Foo Bar"))
> +	 (output (mapcar #'notmuch-show-clean-address input)))
> +    (notmuch-test-compare output expected)))
> diff --git a/test/emacs-address-cleaning.sh b/test/emacs-address-cleaning.sh
> new file mode 100755
> index 0000000..0d85bdc
> --- /dev/null
> +++ b/test/emacs-address-cleaning.sh
> @@ -0,0 +1,19 @@
> +#!/usr/bin/env bash
> +
> +test_description="emacs address cleaning"
> +. test-lib.sh
> +
> +test_begin_subtest "notmuch-test-address-clean part 1"
> +test_emacs_expect_t \
> +    '(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-1)'
> +
> +test_begin_subtest "notmuch-test-address-clean part 2"
> +test_emacs_expect_t \
> +    '(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-2)'
> +
> +test_begin_subtest "notmuch-test-address-clean part 3"
> +test_subtest_known_broken
> +test_emacs_expect_t \
> +    '(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-3)'
> +
> +test_done
> diff --git a/test/notmuch-test b/test/notmuch-test
> index d034f99..3f1740c 100755
> --- a/test/notmuch-test
> +++ b/test/notmuch-test
> @@ -53,6 +53,7 @@ TESTS="
>    hooks
>    argument-parsing
>    emacs-test-functions.sh
> +  emacs-address-cleaning.sh
>  "
>  TESTS=${NOTMUCH_TESTS:=$TESTS}
>  
> -- 
> 1.7.8.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 3/4 v42] test: Add more helpers for emacs tests.

[Dmitry Kurochkin]

On Mon, 23 Jan 2012 18:05:46 +0000, David Edmondson <dme@dme.org> wrote:
> ---
> 
> Split out from the tests and re-factored.
> 
>  test/test-lib.el |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/test/test-lib.el b/test/test-lib.el
> index 96752f0..c4a5db4 100644
> --- a/test/test-lib.el
> +++ b/test/test-lib.el
> @@ -20,6 +20,8 @@
>  ;;
>  ;; Authors: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
>  
> +(require 'cl)	;; This code is generally used uncompiled.
> +
>  ;; `read-file-name' by default uses `completing-read' function to read
>  ;; user input.  It does not respect `standard-input' variable which we
>  ;; use in tests to provide user input.  So replace it with a plain
> @@ -92,3 +94,23 @@ nothing."
>  		   result
>  		 (prin1-to-string result)))
>         (test-output))))
> +
> +(defun notmuch-test-report-unexpected (output expected)
> +  "Report that the OUTPUT does not match the EXPECTED result."
> +  (concat "Expect:\t" (prin1-to-string expected) "\n"
> +	  "Output:\t" (prin1-to-string output) "\n"))
> +
> +(defun notmuch-test-compare (output expected)
> +  "Compare OUTPUT with EXPECTED. Report any discrepencies."
> +  (if (equal output expected)
> +      t
> +    (cond
> +     ((and (listp output)
> +	   (listp expected))
> +      (apply #'concat (loop for o in output
> +			    for e in expected
> +			    if (not (equal o e))
> +			    collect (notmuch-test-report-unexpected o e))))
> +   
> +     (t
> +      (notmuch-test-report-unexpected output expected)))))

As we discussed it on IRC, I have two comments on the above code:

1. rename notmuch-test-compare to notmuch-test-expect-equal

2. move the top level equal check to the non-list branch

But both of these are subjective and minor, so I leave it to David to
decide whether to change or ignore them.  Otherwise, the patch looks
good to me.


While replying to this email, I noticed a trailing whitespace.  I may
have missed them in other patches.  David, can you please check for
trailing white spaces and clean them?

Regards,
  Dmitry

> -- 
> 1.7.8.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 3/4 v42] test: Add more helpers for emacs tests.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 688 bytes --]

On Tue, 24 Jan 2012 19:45:18 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> 1. rename notmuch-test-compare to notmuch-test-expect-equal

I'll change this.

> 2. move the top level equal check to the non-list branch

I'd rather not change this, though I'll write some commentary to explain
how things are.

> But both of these are subjective and minor, so I leave it to David to
> decide whether to change or ignore them.  Otherwise, the patch looks
> good to me.
> 
> While replying to this email, I noticed a trailing whitespace.  I may
> have missed them in other patches.  David, can you please check for
> trailing white spaces and clean them?

Yes.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH 0/4 v43] emacs test helpers

[David Edmondson]

Small changes as per Dmitry's last comments.

David Edmondson (4):
  test: Don't return the result of checking for running emacs to the
    tester.
  test: Add `test_emacs_expect_t'.
  test: Add more helpers for emacs tests.
  test: Add address cleaning tests.

 test/README                    |    8 ++++++++
 test/emacs-address-cleaning.el |   29 +++++++++++++++++++++++++++++
 test/emacs-address-cleaning.sh |   19 +++++++++++++++++++
 test/emacs-test-functions.sh   |    9 +++++++++
 test/notmuch-test              |    2 ++
 test/test-lib.el               |   35 +++++++++++++++++++++++++++++++++++
 test/test-lib.sh               |   31 ++++++++++++++++++++++++++++++-
 7 files changed, 132 insertions(+), 1 deletions(-)
 create mode 100644 test/emacs-address-cleaning.el
 create mode 100755 test/emacs-address-cleaning.sh
 create mode 100755 test/emacs-test-functions.sh

-- 
1.7.8.3

[PATCH 1/4 v43] test: Don't return the result of checking for running emacs to the tester.

[David Edmondson]

When checking for a running emacs, test_emacs evaluates the empty list
'()'. This returns 'nil' when emacs is running, which is then
prepended to the actual test result. Given that it is not part of the
actual test output the test harness can incorrectly report test
failure (or success).
---
 test/test-lib.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 0da60fb..82c686c 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -926,7 +926,7 @@ test_emacs () {
 				--eval '(orphan-watchdog $$)'" || return
 		EMACS_SERVER="$server_name"
 		# wait until the emacs server is up
-		until test_emacs '()' 2>/dev/null; do
+		until test_emacs '()' >/dev/null 2>/dev/null; do
 			sleep 1
 		done
 	fi
-- 
1.7.8.3

[PATCH 2/4 v43] test: Add `test_emacs_expect_t'.

[David Edmondson]

Add a new test function to allow simpler testing of emacs
functionality.

`test_emacs_expect_t' takes one argument - a lisp expression to
evaluate. The test passes if the expression returns `t', otherwise it
fails and the output is reported to the tester.
---
 test/README                  |    8 ++++++++
 test/emacs-test-functions.sh |    9 +++++++++
 test/notmuch-test            |    1 +
 test/test-lib.el             |    9 +++++++++
 test/test-lib.sh             |   29 +++++++++++++++++++++++++++++
 5 files changed, 56 insertions(+), 0 deletions(-)
 create mode 100755 test/emacs-test-functions.sh

diff --git a/test/README b/test/README
index 44ff653..43656a3 100644
--- a/test/README
+++ b/test/README
@@ -202,6 +202,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 59c5868..96752f0 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -83,3 +83,12 @@ nothing."
 
 (add-hook-counter 'notmuch-hello-mode-hook)
 (add-hook-counter 'notmuch-hello-refresh-hook)
+
+(defmacro notmuch-test-run (&rest body)
+  "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 82c686c..8158328 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -503,6 +503,35 @@ test_expect_equal_file ()
     fi
 }
 
+test_emacs_expect_t () {
+	test "$#" = 2 && { prereq=$1; shift; } || prereq=
+	test "$#" = 1 ||
+	error "bug in the test script: not 1 or 2 parameters to test_emacs_expect_t"
+
+	# Run the test.
+	if ! test_skip "$test_subtest_name"
+	then
+		test_emacs "(notmuch-test-run $1)" >/dev/null
+
+		# Restore state after the test.
+		exec 1>&6 2>&7		# Restore stdout and stderr
+		inside_subtest=
+
+		# Report success/failure.
+		result=$(cat OUTPUT)
+		if [ "$result" = t ]
+		then
+			test_ok_ "$test_subtest_name"
+		else
+			test_failure_ "$test_subtest_name" "${result}"
+		fi
+	else
+		# Restore state after the (non) test.
+		exec 1>&6 2>&7		# Restore stdout and stderr
+		inside_subtest=
+	fi
+}
+
 NOTMUCH_NEW ()
 {
     notmuch new | grep -v -E -e '^Processed [0-9]*( total)? file|Found [0-9]* total file'
-- 
1.7.8.3

[PATCH 3/4 v43] test: Add more helpers for emacs tests.

[David Edmondson]

---
 test/test-lib.el |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index 96752f0..bc75f06 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -20,6 +20,8 @@
 ;;
 ;; Authors: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
 
+(require 'cl)	;; This code is generally used uncompiled.
+
 ;; `read-file-name' by default uses `completing-read' function to read
 ;; user input.  It does not respect `standard-input' variable which we
 ;; use in tests to provide user input.  So replace it with a plain
@@ -92,3 +94,27 @@ nothing."
 		   result
 		 (prin1-to-string result)))
        (test-output))))
+
+(defun notmuch-test-report-unexpected (output expected)
+  "Report that the OUTPUT does not match the EXPECTED result."
+  (concat "Expect:\t" (prin1-to-string expected) "\n"
+	  "Output:\t" (prin1-to-string output) "\n"))
+
+(defun notmuch-test-expect-equal (output expected)
+  "Compare OUTPUT with EXPECTED. Report any discrepencies."
+  (if (equal output expected)
+      t
+    (cond
+     ((and (listp output)
+	   (listp expected))
+      ;; Reporting the difference between two lists is done by
+      ;; reporting differing elements of OUTPUT and EXPECTED
+      ;; pairwise. This is expected to make analysis of failures
+      ;; simpler.
+      (apply #'concat (loop for o in output
+			    for e in expected
+			    if (not (equal o e))
+			    collect (notmuch-test-report-unexpected o e))))
+
+     (t
+      (notmuch-test-report-unexpected output expected)))))
-- 
1.7.8.3

[PATCH 4/4 v43] test: Add address cleaning tests.

[David Edmondson]

---
 test/emacs-address-cleaning.el |   29 +++++++++++++++++++++++++++++
 test/emacs-address-cleaning.sh |   19 +++++++++++++++++++
 test/notmuch-test              |    1 +
 3 files changed, 49 insertions(+), 0 deletions(-)
 create mode 100644 test/emacs-address-cleaning.el
 create mode 100755 test/emacs-address-cleaning.sh

diff --git a/test/emacs-address-cleaning.el b/test/emacs-address-cleaning.el
new file mode 100644
index 0000000..19e9e05
--- /dev/null
+++ b/test/emacs-address-cleaning.el
@@ -0,0 +1,29 @@
+(defun notmuch-test-address-cleaning-1 ()
+  (notmuch-test-expect-equal (notmuch-show-clean-address "dme@dme.org")
+			"dme@dme.org"))
+
+(defun notmuch-test-address-cleaning-2 ()
+  (let* ((input '("foo@bar.com"
+		  "<foo@bar.com>"
+		  "Foo Bar <foo@bar.com>"
+		  "foo@bar.com <foo@bar.com>"
+		  "\"Foo Bar\" <foo@bar.com>"))
+	 (expected '("foo@bar.com"
+		     "foo@bar.com"
+		     "Foo Bar <foo@bar.com>"
+		     "foo@bar.com"
+		     "Foo Bar <foo@bar.com>"))
+	 (output (mapcar #'notmuch-show-clean-address input)))
+    (notmuch-test-expect-equal output expected)))
+
+(defun notmuch-test-address-cleaning-3 ()
+  (let* ((input '("ДБ <db-uknot@stop.me.uk>"
+		  "foo (at home) <foo@bar.com>"
+		  "foo [at home] <foo@bar.com>"
+		  "Foo Bar"))
+	 (expected '("ДБ <db-uknot@stop.me.uk>"
+		     "foo (at home) <foo@bar.com>"
+		     "foo [at home] <foo@bar.com>"
+		     "Foo Bar"))
+	 (output (mapcar #'notmuch-show-clean-address input)))
+    (notmuch-test-expect-equal output expected)))
diff --git a/test/emacs-address-cleaning.sh b/test/emacs-address-cleaning.sh
new file mode 100755
index 0000000..0d85bdc
--- /dev/null
+++ b/test/emacs-address-cleaning.sh
@@ -0,0 +1,19 @@
+#!/usr/bin/env bash
+
+test_description="emacs address cleaning"
+. test-lib.sh
+
+test_begin_subtest "notmuch-test-address-clean part 1"
+test_emacs_expect_t \
+    '(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-1)'
+
+test_begin_subtest "notmuch-test-address-clean part 2"
+test_emacs_expect_t \
+    '(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-2)'
+
+test_begin_subtest "notmuch-test-address-clean part 3"
+test_subtest_known_broken
+test_emacs_expect_t \
+    '(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-3)'
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index d034f99..3f1740c 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -53,6 +53,7 @@ TESTS="
   hooks
   argument-parsing
   emacs-test-functions.sh
+  emacs-address-cleaning.sh
 "
 TESTS=${NOTMUCH_TESTS:=$TESTS}
 
-- 
1.7.8.3

Re: [PATCH 0/4 v43] emacs test helpers

[Dmitry Kurochkin]

On Tue, 24 Jan 2012 16:14:03 +0000, David Edmondson <dme@dme.org> wrote:
> Small changes as per Dmitry's last comments.
> 

LGTM

Regards,
  Dmitry

> David Edmondson (4):
>   test: Don't return the result of checking for running emacs to the
>     tester.
>   test: Add `test_emacs_expect_t'.
>   test: Add more helpers for emacs tests.
>   test: Add address cleaning tests.
> 
>  test/README                    |    8 ++++++++
>  test/emacs-address-cleaning.el |   29 +++++++++++++++++++++++++++++
>  test/emacs-address-cleaning.sh |   19 +++++++++++++++++++
>  test/emacs-test-functions.sh   |    9 +++++++++
>  test/notmuch-test              |    2 ++
>  test/test-lib.el               |   35 +++++++++++++++++++++++++++++++++++
>  test/test-lib.sh               |   31 ++++++++++++++++++++++++++++++-
>  7 files changed, 132 insertions(+), 1 deletions(-)
>  create mode 100644 test/emacs-address-cleaning.el
>  create mode 100755 test/emacs-address-cleaning.sh
>  create mode 100755 test/emacs-test-functions.sh
> 
> -- 
> 1.7.8.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 0/4 v43] emacs test helpers

[Tomi Ollila]

On Tue, 24 Jan 2012 16:14:03 +0000, David Edmondson <dme@dme.org> wrote:
> Small changes as per Dmitry's last comments.
> 
> David Edmondson (4):
>   test: Don't return the result of checking for running emacs to the
>     tester.
>   test: Add `test_emacs_expect_t'.
>   test: Add more helpers for emacs tests.
>   test: Add address cleaning tests.

LGTM -- all 4.


Tomi

Re: [PATCH 0/4 v43] emacs test helpers

[David Bremner]

On Tue, 24 Jan 2012 16:14:03 +0000, David Edmondson <dme@dme.org> wrote:
> Small changes as per Dmitry's last comments.

Pushed.

d