unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 1/5] test: do not set `message-signature' in test_emacs
@ 2011-06-26 23:52 Dmitry Kurochkin
  2011-06-26 23:52 ` [PATCH 2/5] test: cleanup test_emacs Dmitry Kurochkin
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Dmitry Kurochkin @ 2011-06-26 23:52 UTC (permalink / raw)
  To: notmuch

It is no longer needed since tests are run in a temporary home
directory instead of the user's one.
---
 test/test-lib.sh |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index cc20f41..3ec388c 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -855,8 +855,6 @@ fi
 #
 # notmuch-test-wait	Function for tests to use to wait for process completion
 #
-# message-signature	Avoiding appending user's signature on messages
-#
 # set-frame-width	80 columns (avoids crazy 10-column default of --batch)
 
 emacs \$BATCH --no-init-file --no-site-file \
@@ -865,7 +863,6 @@ emacs \$BATCH --no-init-file --no-site-file \
 	--eval "(defun notmuch-test-wait ()
 			(while (get-buffer-process (current-buffer))
 				(sleep-for 0.1)))" \
-	--eval "(setq message-signature nil)" \
 	--eval "(progn (set-frame-width (window-frame (get-buffer-window)) 80) \$@)"
 EOF
 	chmod a+x ./run_emacs
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 2/5] test: cleanup test_emacs
  2011-06-26 23:52 [PATCH 1/5] test: do not set `message-signature' in test_emacs Dmitry Kurochkin
@ 2011-06-26 23:52 ` Dmitry Kurochkin
  2011-06-26 23:52 ` [PATCH 3/5] test: wrap and indent test_emacs calls Dmitry Kurochkin
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Dmitry Kurochkin @ 2011-06-26 23:52 UTC (permalink / raw)
  To: notmuch

Move auxiliary function definition and configuration from command
line to test-lib.el.
---
 test/test-lib.el |    8 ++++++++
 test/test-lib.sh |    9 +--------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index 9439996..344a02e 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -20,6 +20,14 @@
 ;;
 ;; Authors: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
 
+;; avoid crazy 10-column default of --batch
+(set-frame-width (window-frame (get-buffer-window)) 80)
+
+(defun notmuch-test-wait ()
+  "Wait for process completion."
+  (while (get-buffer-process (current-buffer))
+    (sleep-for 0.1)))
+
 (defun visible-buffer-string ()
   "Same as `buffer-string', but excludes invisible text."
   (visible-buffer-substring (point-min) (point-max)))
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 3ec388c..5f61960 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -852,18 +852,11 @@ fi
 # --directory		Ensure that the local elisp sources are found
 #
 # --load		Force loading of notmuch.el and test-lib.el
-#
-# notmuch-test-wait	Function for tests to use to wait for process completion
-#
-# set-frame-width	80 columns (avoids crazy 10-column default of --batch)
 
 emacs \$BATCH --no-init-file --no-site-file \
 	--directory ../../emacs --load notmuch.el \
 	--directory .. --load test-lib.el \
-	--eval "(defun notmuch-test-wait ()
-			(while (get-buffer-process (current-buffer))
-				(sleep-for 0.1)))" \
-	--eval "(progn (set-frame-width (window-frame (get-buffer-window)) 80) \$@)"
+	--eval "(progn \$@)"
 EOF
 	chmod a+x ./run_emacs
 	./run_emacs "$@"
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 3/5] test: wrap and indent test_emacs calls
  2011-06-26 23:52 [PATCH 1/5] test: do not set `message-signature' in test_emacs Dmitry Kurochkin
  2011-06-26 23:52 ` [PATCH 2/5] test: cleanup test_emacs Dmitry Kurochkin
@ 2011-06-26 23:52 ` Dmitry Kurochkin
  2011-06-26 23:52 ` [PATCH 4/5] test: save buffer content to file instead of printing it in Emacs tests Dmitry Kurochkin
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Dmitry Kurochkin @ 2011-06-26 23:52 UTC (permalink / raw)
  To: notmuch

Most test_emacs calls have long arguments that consist of many
expressions.  Putting them on a single line makes it hard to read
and produces poor diff when they are changed.  The patch puts
every expression in test_emacs calls on a separate line.
---
 test/emacs       |  124 ++++++++++++++++++++++++++++++++++++++++++++----------
 test/test-lib.sh |   14 ++++++-
 2 files changed, 114 insertions(+), 24 deletions(-)

diff --git a/test/emacs b/test/emacs
index 6f82b08..1e46c0d 100755
--- a/test/emacs
+++ b/test/emacs
@@ -7,32 +7,51 @@ EXPECTED=../emacs.expected-output
 add_email_corpus
 
 test_begin_subtest "Basic notmuch-hello view in emacs"
-test_emacs '(notmuch-hello) (princ (buffer-string))' >OUTPUT
+test_emacs '(notmuch-hello)
+	    (princ (buffer-string))' >OUTPUT
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello
 
 test_begin_subtest "Saved search with 0 results"
-test_emacs '(setq notmuch-show-empty-saved-searches t) (setq notmuch-saved-searches '\''(("inbox" . "tag:inbox") ("unread" . "tag:unread") ("empty" . "tag:doesnotexist"))) (notmuch-hello) (princ (buffer-string))' >OUTPUT
+test_emacs '(setq notmuch-show-empty-saved-searches t)
+	    (setq notmuch-saved-searches
+		  '\''(("inbox" . "tag:inbox")
+		       ("unread" . "tag:unread")
+		       ("empty" . "tag:doesnotexist")))
+	    (notmuch-hello)
+	    (princ (buffer-string))' >OUTPUT
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-with-empty
 
 test_begin_subtest "No saved searches displayed (all with 0 results)"
-test_emacs '(setq notmuch-saved-searches '\''(("empty" . "tag:doesnotexist"))) (notmuch-hello) (princ (buffer-string))' >OUTPUT
+test_emacs '(setq notmuch-saved-searches
+		  '\''(("empty" . "tag:doesnotexist")))
+	    (notmuch-hello)
+	    (princ (buffer-string))' >OUTPUT
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-no-saved-searches
 
 test_begin_subtest "Basic notmuch-search view in emacs"
-test_emacs '(notmuch-search "tag:inbox") (notmuch-test-wait) (princ (buffer-string))' >OUTPUT
+test_emacs '(notmuch-search "tag:inbox")
+	    (notmuch-test-wait)
+	    (princ (buffer-string))' >OUTPUT
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-search-tag-inbox
 
 test_begin_subtest "Navigation of notmuch-hello to search results"
-test_emacs '(notmuch-hello) (goto-char (point-min)) (re-search-forward "inbox") (widget-button-press (point)) (notmuch-test-wait) (princ (buffer-string))' >OUTPUT
+test_emacs '(notmuch-hello)
+	    (goto-char (point-min))
+	    (re-search-forward "inbox")
+	    (widget-button-press (point))
+	    (notmuch-test-wait)
+	    (princ (buffer-string))' >OUTPUT
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-view-inbox
 
 test_begin_subtest "Basic notmuch-show view in emacs"
 maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
-test_emacs "(notmuch-show \"$maildir_storage_thread\") (princ (buffer-string))" >OUTPUT
+test_emacs "(notmuch-show \"$maildir_storage_thread\")
+	    (princ (buffer-string))" >OUTPUT
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
 
 test_begin_subtest "notmuch-show for message with invalid From"
-add_message "[subject]=\"message-with-invalid-from\"" "[from]=\"\\\"Invalid \\\" From\\\" <test_suite@notmuchmail.org>\""
+add_message "[subject]=\"message-with-invalid-from\"" \
+	    "[from]=\"\\\"Invalid \\\" From\\\" <test_suite@notmuchmail.org>\""
 thread=$(notmuch search --output=threads subject:message-with-invalid-from)
 output=$(test_emacs "(notmuch-show \"$thread\") (princ (buffer-string))")
 test_expect_equal "$output" \
@@ -44,33 +63,54 @@ Date: Tue, 05 Jan 2001 15:43:57 -0000
 This is just a test message (#1)'
 
 test_begin_subtest "Navigation of notmuch-search to thread view"
-test_emacs '(notmuch-search "tag:inbox") (notmuch-test-wait) (goto-char (point-min)) (re-search-forward "Working with Maildir") (notmuch-search-show-thread) (notmuch-test-wait) (princ (buffer-string))' >OUTPUT
+test_emacs '(notmuch-search "tag:inbox")
+	    (notmuch-test-wait)
+	    (goto-char (point-min))
+	    (re-search-forward "Working with Maildir")
+	    (notmuch-search-show-thread)
+	    (notmuch-test-wait)
+	    (princ (buffer-string))' >OUTPUT
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
 
 test_begin_subtest "Add tag from search view"
 os_x_darwin_thread=$(notmuch search --output=threads id:ddd65cda0911171950o4eea4389v86de9525e46052d3@mail.gmail.com)
-test_emacs "(notmuch-search \"$os_x_darwin_thread\") (notmuch-test-wait) (notmuch-search-add-tag \"tag-from-search-view\")"
+test_emacs "(notmuch-search \"$os_x_darwin_thread\")
+	    (notmuch-test-wait)
+	    (notmuch-search-add-tag \"tag-from-search-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-search-view unread)"
 
 test_begin_subtest "Remove tag from search view"
-test_emacs "(notmuch-search \"$os_x_darwin_thread\") (notmuch-test-wait) (notmuch-search-remove-tag \"tag-from-search-view\")"
+test_emacs "(notmuch-search \"$os_x_darwin_thread\")
+	    (notmuch-test-wait)
+	    (notmuch-search-remove-tag \"tag-from-search-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
 
 test_begin_subtest "Add tag from notmuch-show view"
-test_emacs "(notmuch-show \"$os_x_darwin_thread\") (notmuch-show-add-tag \"tag-from-show-view\")"
+test_emacs "(notmuch-show \"$os_x_darwin_thread\")
+	    (notmuch-show-add-tag \"tag-from-show-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-show-view unread)"
 
 test_begin_subtest "Remove tag from notmuch-show view"
-test_emacs "(notmuch-show \"$os_x_darwin_thread\") (notmuch-show-remove-tag \"tag-from-show-view\")"
+test_emacs "(notmuch-show \"$os_x_darwin_thread\")
+	    (notmuch-show-remove-tag \"tag-from-show-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
 
 test_begin_subtest "Message with .. in Message-Id:"
 add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
-test_emacs '(notmuch-search "id:\"123..456@example\"") (notmuch-test-wait) (notmuch-search-add-tag "search-add") (notmuch-search-add-tag "search-remove") (notmuch-search-remove-tag "search-remove") (notmuch-show "id:\"123..456@example\"") (notmuch-test-wait) (notmuch-show-add-tag "show-add") (notmuch-show-add-tag "show-remove") (notmuch-show-remove-tag "show-remove")'
+test_emacs '(notmuch-search "id:\"123..456@example\"")
+	    (notmuch-test-wait)
+	    (notmuch-search-add-tag "search-add")
+	    (notmuch-search-add-tag "search-remove")
+	    (notmuch-search-remove-tag "search-remove")
+	    (notmuch-show "id:\"123..456@example\"")
+	    (notmuch-test-wait)
+	    (notmuch-show-add-tag "show-add")
+	    (notmuch-show-add-tag "show-remove")
+	    (notmuch-show-remove-tag "show-remove")'
 output=$(notmuch search 'id:"123..456@example"' | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Message with .. in Message-Id (inbox search-add show-add)"
 
@@ -83,7 +123,18 @@ mkdir -p mail/sent/tmp
 
 ../smtp-dummy sent_message &
 smtp_dummy_pid=$!
-test_emacs "(setq message-send-mail-function 'message-smtpmail-send-it) (setq smtpmail-smtp-server \"localhost\") (setq smtpmail-smtp-service \"25025\") (notmuch-hello) (notmuch-mua-mail) (message-goto-to) (insert \"user@example.com\nDate: Fri, 29 Mar 1974 10:00:00 -0000\") (message-goto-subject) (insert \"Testing message sent via SMTP\") (message-goto-body) (insert \"This is a test that messages are sent via SMTP\") (message-send-and-exit)" >/dev/null 2>&1
+test_emacs "(setq message-send-mail-function 'message-smtpmail-send-it)
+	    (setq smtpmail-smtp-server \"localhost\")
+	    (setq smtpmail-smtp-service \"25025\")
+	    (notmuch-hello)
+	    (notmuch-mua-mail)
+	    (message-goto-to)
+	    (insert \"user@example.com\nDate: Fri, 29 Mar 1974 10:00:00 -0000\")
+	    (message-goto-subject)
+	    (insert \"Testing message sent via SMTP\")
+	    (message-goto-body)
+	    (insert \"This is a test that messages are sent via SMTP\")
+	    (message-send-and-exit)" >/dev/null 2>&1
 wait ${smtp_dummy_pid}
 
 sed \
@@ -109,7 +160,9 @@ output=$(notmuch search 'subject:"testing message sent via SMTP"' | notmuch_sear
 test_expect_equal "$output" "thread:XXX   1974-03-29 [1/1] Notmuch Test Suite; Testing message sent via SMTP (inbox)"
 
 test_begin_subtest "notmuch-fcc-dirs set to nil"
-test_emacs "(setq notmuch-fcc-dirs nil) (notmuch-mua-mail) (princ (buffer-string))" > OUTPUT
+test_emacs "(setq notmuch-fcc-dirs nil)
+	    (notmuch-mua-mail)
+	    (princ (buffer-string))" > OUTPUT
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -124,7 +177,9 @@ mkdir -p mail/sent-string/new
 mkdir -p mail/sent-string/tmp
 
 test_begin_subtest "notmuch-fcc-dirs set to a string"
-test_emacs "(setq notmuch-fcc-dirs \"sent-string\") (notmuch-mua-mail) (princ (buffer-string))" > OUTPUT
+test_emacs "(setq notmuch-fcc-dirs \"sent-string\")
+	    (notmuch-mua-mail)
+	    (princ (buffer-string))" > OUTPUT
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -143,7 +198,11 @@ mkdir -p mail/failure/new
 mkdir -p mail/failure/tmp
 
 test_begin_subtest "notmuch-fcc-dirs set to a list (with match)"
-test_emacs "(setq notmuch-fcc-dirs '((\"notmuchmail.org\" . \"sent-list-match\") (\".*\" . \"failure\"))) (notmuch-mua-mail) (princ (buffer-string))" > OUTPUT
+test_emacs "(setq notmuch-fcc-dirs
+		  '((\"notmuchmail.org\" . \"sent-list-match\")
+		    (\".*\" . \"failure\")))
+	    (notmuch-mua-mail)
+	    (princ (buffer-string))" > OUTPUT
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -159,7 +218,11 @@ mkdir -p mail/sent-list-catch-all/new
 mkdir -p mail/sent-list-catch-all/tmp
  
 test_begin_subtest "notmuch-fcc-dirs set to a list (catch-all)"
-test_emacs "(setq notmuch-fcc-dirs '((\"example.com\" . \"failure\") (\".*\" . \"sent-list-catch-all\"))) (notmuch-mua-mail) (princ (buffer-string))" > OUTPUT
+test_emacs "(setq notmuch-fcc-dirs
+		  '((\"example.com\" . \"failure\")
+		    (\".*\" . \"sent-list-catch-all\")))
+	    (notmuch-mua-mail)
+	    (princ (buffer-string))" > OUTPUT
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -170,7 +233,11 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "notmuch-fcc-dirs set to a list (no match)"
-test_emacs "(setq notmuch-fcc-dirs '((\"example.com\" . \"failure\") (\"nomatchhere.net\" . \"failure\"))) (notmuch-mua-mail) (princ (buffer-string))" > OUTPUT
+test_emacs "(setq notmuch-fcc-dirs
+		  '((\"example.com\" . \"failure\")
+		    (\"nomatchhere.net\" . \"failure\")))
+	    (notmuch-mua-mail)
+	    (princ (buffer-string))" > OUTPUT
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -182,7 +249,12 @@ test_expect_equal_file OUTPUT EXPECTED
 test_begin_subtest "Reply within emacs"
 # We sed away everything before the ^From in the output to avoid getting
 # confused by messages such as "Parsing /home/cworth/.mailrc... done"
-test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"") (notmuch-test-wait) (notmuch-search-reply-to-thread) (princ (buffer-string))' | sed -ne '/^From/,$ p' | sed -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/' >OUTPUT
+test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"")
+	    (notmuch-test-wait)
+	    (notmuch-search-reply-to-thread)
+	    (princ (buffer-string))' |
+sed -ne '/^From/,$ p' |
+sed -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/' >OUTPUT
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: user@example.com
@@ -197,17 +269,23 @@ test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Save attachment from within emacs using notmuch-show-save-attachments"
 # save as archive to test that Emacs does not re-compress .gz
-echo ./attachment1.gz | test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com") (notmuch-show-save-attachments)' > /dev/null 2>&1
+echo ./attachment1.gz |
+test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
+	    (notmuch-show-save-attachments)' > /dev/null 2>&1
 test_expect_equal_file "$EXPECTED/attachment" attachment1.gz
 
 test_begin_subtest "Save attachment from within emacs using notmuch-show-save-part"
 # save as archive to test that Emacs does not re-compress .gz
-echo ./attachment2.gz | test_emacs '(notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com" 5)' > /dev/null 2>&1
+echo ./attachment2.gz |
+test_emacs '(notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com" 5)' > /dev/null 2>&1
 test_expect_equal_file "$EXPECTED/attachment" attachment2.gz
 
 test_begin_subtest "View raw message within emacs"
 first_line=$(head -n1 $EXPECTED/raw-message-cf0c4d-52ad0a)
-test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com") (notmuch-show-view-raw-message) (princ (buffer-string))' | sed -ne "/$first_line/,\$ p" >OUTPUT
+test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
+	    (notmuch-show-view-raw-message)
+	    (princ (buffer-string))' |
+sed -ne "/$first_line/,\$ p" >OUTPUT
 test_expect_equal_file OUTPUT $EXPECTED/raw-message-cf0c4d-52ad0a
 
 test_begin_subtest "Hiding/showing signature in notmuch-show view"
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 5f61960..ad1506c 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -394,7 +394,19 @@ emacs_deliver_message ()
     mkdir -p "$MAIL_DIR"/sent/{cur,new,tmp}
     ../smtp-dummy sent_message &
     smtp_dummy_pid=$!
-    test_emacs "(setq message-send-mail-function 'message-smtpmail-send-it) (setq smtpmail-smtp-server \"localhost\") (setq smtpmail-smtp-service \"25025\") (notmuch-hello) (notmuch-mua-mail) (message-goto-to) (insert \"test_suite@notmuchmail.org\nDate: 01 Jan 2000 12:00:00 -0000\") (message-goto-subject) (insert \"${subject}\") (message-goto-body) (insert \"${body}\") $@ (message-send-and-exit)" >/dev/null 2>&1
+    test_emacs "(setq message-send-mail-function 'message-smtpmail-send-it)
+		(setq smtpmail-smtp-server \"localhost\")
+		(setq smtpmail-smtp-service \"25025\")
+		(notmuch-hello)
+		(notmuch-mua-mail)
+		(message-goto-to)
+		(insert \"test_suite@notmuchmail.org\nDate: 01 Jan 2000 12:00:00 -0000\")
+		(message-goto-subject)
+		(insert \"${subject}\")
+		(message-goto-body)
+		(insert \"${body}\")
+		$@
+		(message-send-and-exit)" >/dev/null 2>&1
     wait ${smtp_dummy_pid}
     notmuch new >/dev/null
 }
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 4/5] test: save buffer content to file instead of printing it in Emacs tests
  2011-06-26 23:52 [PATCH 1/5] test: do not set `message-signature' in test_emacs Dmitry Kurochkin
  2011-06-26 23:52 ` [PATCH 2/5] test: cleanup test_emacs Dmitry Kurochkin
  2011-06-26 23:52 ` [PATCH 3/5] test: wrap and indent test_emacs calls Dmitry Kurochkin
@ 2011-06-26 23:52 ` Dmitry Kurochkin
  2011-06-26 23:52 ` [PATCH 5/5] test: remove some sed(1) calls " Dmitry Kurochkin
  2011-06-27  3:54 ` [PATCH 1/2] test: use emacs_deliver_message in Emacs SMTP send test Dmitry Kurochkin
  4 siblings, 0 replies; 22+ messages in thread
From: Dmitry Kurochkin @ 2011-06-26 23:52 UTC (permalink / raw)
  To: notmuch

Before the change, the common Emacs test scheme was to print
buffer content to stdout and redirect it to a file or capture it
in a shell variable.  This does not work if we switch to using
emacsclient(1) for running the tests, because you can not print
to the stdout in this case. (Actually, you can print to stdout
from Emacs server, but you can not capture the output on
emacsclient(1)).

The patch introduces new Emacs test auxiliary functions:
`test-output' and `test-visible-output'.  These functions are
used to save buffer content to a file directly from Emacs.  For
most tests the changes are trivial, because Emacs stdout output
was redirected to a file anyway.  But some tests captured the
output in a shell variable and compare it with the expected
output using test_expect_equal.  These tests are changed to use
files and test_expect_equal_file instead.

Note: even if we do not switch Emacs tests to emacsclient(1), the
patch makes tests cleaner and is an improvement.
---
 test/emacs                     |   71 ++++++++++++++++++++-------------------
 test/emacs-large-search-buffer |   23 +++++++++----
 test/test-lib.el               |   10 ++++++
 3 files changed, 62 insertions(+), 42 deletions(-)

diff --git a/test/emacs b/test/emacs
index 1e46c0d..fc4c01f 100755
--- a/test/emacs
+++ b/test/emacs
@@ -8,7 +8,7 @@ add_email_corpus
 
 test_begin_subtest "Basic notmuch-hello view in emacs"
 test_emacs '(notmuch-hello)
-	    (princ (buffer-string))' >OUTPUT
+	    (test-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello
 
 test_begin_subtest "Saved search with 0 results"
@@ -18,20 +18,20 @@ test_emacs '(setq notmuch-show-empty-saved-searches t)
 		       ("unread" . "tag:unread")
 		       ("empty" . "tag:doesnotexist")))
 	    (notmuch-hello)
-	    (princ (buffer-string))' >OUTPUT
+	    (test-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-with-empty
 
 test_begin_subtest "No saved searches displayed (all with 0 results)"
 test_emacs '(setq notmuch-saved-searches
 		  '\''(("empty" . "tag:doesnotexist")))
 	    (notmuch-hello)
-	    (princ (buffer-string))' >OUTPUT
+	    (test-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-no-saved-searches
 
 test_begin_subtest "Basic notmuch-search view in emacs"
 test_emacs '(notmuch-search "tag:inbox")
 	    (notmuch-test-wait)
-	    (princ (buffer-string))' >OUTPUT
+	    (test-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-search-tag-inbox
 
 test_begin_subtest "Navigation of notmuch-hello to search results"
@@ -40,27 +40,30 @@ test_emacs '(notmuch-hello)
 	    (re-search-forward "inbox")
 	    (widget-button-press (point))
 	    (notmuch-test-wait)
-	    (princ (buffer-string))' >OUTPUT
+	    (test-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-view-inbox
 
 test_begin_subtest "Basic notmuch-show view in emacs"
 maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
 test_emacs "(notmuch-show \"$maildir_storage_thread\")
-	    (princ (buffer-string))" >OUTPUT
+	    (test-output)"
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
 
 test_begin_subtest "notmuch-show for message with invalid From"
 add_message "[subject]=\"message-with-invalid-from\"" \
 	    "[from]=\"\\\"Invalid \\\" From\\\" <test_suite@notmuchmail.org>\""
 thread=$(notmuch search --output=threads subject:message-with-invalid-from)
-output=$(test_emacs "(notmuch-show \"$thread\") (princ (buffer-string))")
-test_expect_equal "$output" \
-'"Invalid " From" <test_suite@notmuchmail.org> (2001-01-05) (inbox)
+test_emacs "(notmuch-show \"$thread\")
+	    (test-output)"
+cat <<EOF >EXPECTED
+"Invalid " From" <test_suite@notmuchmail.org> (2001-01-05) (inbox)
 Subject: message-with-invalid-from
 To: Notmuch Test Suite <test_suite@notmuchmail.org>
 Date: Tue, 05 Jan 2001 15:43:57 -0000
 
-This is just a test message (#1)'
+This is just a test message (#1)
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Navigation of notmuch-search to thread view"
 test_emacs '(notmuch-search "tag:inbox")
@@ -69,7 +72,7 @@ test_emacs '(notmuch-search "tag:inbox")
 	    (re-search-forward "Working with Maildir")
 	    (notmuch-search-show-thread)
 	    (notmuch-test-wait)
-	    (princ (buffer-string))' >OUTPUT
+	    (test-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
 
 test_begin_subtest "Add tag from search view"
@@ -162,7 +165,7 @@ test_expect_equal "$output" "thread:XXX   1974-03-29 [1/1] Notmuch Test Suite; T
 test_begin_subtest "notmuch-fcc-dirs set to nil"
 test_emacs "(setq notmuch-fcc-dirs nil)
 	    (notmuch-mua-mail)
-	    (princ (buffer-string))" > OUTPUT
+	    (test-output)"
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -179,7 +182,7 @@ mkdir -p mail/sent-string/tmp
 test_begin_subtest "notmuch-fcc-dirs set to a string"
 test_emacs "(setq notmuch-fcc-dirs \"sent-string\")
 	    (notmuch-mua-mail)
-	    (princ (buffer-string))" > OUTPUT
+	    (test-output)"
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -202,7 +205,7 @@ test_emacs "(setq notmuch-fcc-dirs
 		  '((\"notmuchmail.org\" . \"sent-list-match\")
 		    (\".*\" . \"failure\")))
 	    (notmuch-mua-mail)
-	    (princ (buffer-string))" > OUTPUT
+	    (test-output)"
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -222,7 +225,7 @@ test_emacs "(setq notmuch-fcc-dirs
 		  '((\"example.com\" . \"failure\")
 		    (\".*\" . \"sent-list-catch-all\")))
 	    (notmuch-mua-mail)
-	    (princ (buffer-string))" > OUTPUT
+	    (test-output)"
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -237,7 +240,7 @@ test_emacs "(setq notmuch-fcc-dirs
 		  '((\"example.com\" . \"failure\")
 		    (\"nomatchhere.net\" . \"failure\")))
 	    (notmuch-mua-mail)
-	    (princ (buffer-string))" > OUTPUT
+	    (test-output)"
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -252,9 +255,9 @@ test_begin_subtest "Reply within emacs"
 test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"")
 	    (notmuch-test-wait)
 	    (notmuch-search-reply-to-thread)
-	    (princ (buffer-string))' |
-sed -ne '/^From/,$ p' |
-sed -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/' >OUTPUT
+	    (test-output)'
+sed -i -ne '/^From/,$ p' OUTPUT
+sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/' OUTPUT
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: user@example.com
@@ -284,8 +287,8 @@ test_begin_subtest "View raw message within emacs"
 first_line=$(head -n1 $EXPECTED/raw-message-cf0c4d-52ad0a)
 test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
 	    (notmuch-show-view-raw-message)
-	    (princ (buffer-string))' |
-sed -ne "/$first_line/,\$ p" >OUTPUT
+	    (test-output)'
+sed -i -ne "/$first_line/,\$ p" OUTPUT
 test_expect_equal_file OUTPUT $EXPECTED/raw-message-cf0c4d-52ad0a
 
 test_begin_subtest "Hiding/showing signature in notmuch-show view"
@@ -295,7 +298,7 @@ test_emacs "(notmuch-show \"$maildir_storage_thread\")
 	    (button-activate (button-at (point)))
 	    (search-backward \"Click/Enter to hide.\")
 	    (button-activate (button-at (point)))
-	    (princ (buffer-string))" >OUTPUT
+	    (test-output)"
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
 
 test_begin_subtest "Detection and hiding of top-post quoting of message"
@@ -323,7 +326,7 @@ Q: Why is top-posting such a bad thing?
 A: Top-posting.
 Q: What is the most annoying thing in e-mail?"'
 test_emacs "(notmuch-show \"top-posting\")
-	    (princ (visible-buffer-string))" >OUTPUT
+	    (test-visible-output)"
 echo "Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (inbox)
 Subject: The problem with top-posting
 To: Notmuch Test Suite <test_suite@notmuchmail.org>
@@ -346,19 +349,17 @@ Thanks for the advice! I will be sure to put it to good use.
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Hiding message in notmuch-show view"
-output=$(test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
-		     (notmuch-show-toggle-message)
-		     (princ (visible-buffer-string))')
-expected=$(cat $EXPECTED/notmuch-show-thread-with-hidden-messages)
-test_expect_equal "$output" "$expected"
+test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
+	    (notmuch-show-toggle-message)
+	    (test-visible-output)'
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
 
 test_begin_subtest "Hiding message with visible citation in notmuch-show view"
-output=$(test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
-		     (search-forward "Click/Enter to show.")
-		     (button-activate (button-at (point)))
-		     (notmuch-show-toggle-message)
-		     (princ (visible-buffer-string))')
-expected=$(cat $EXPECTED/notmuch-show-thread-with-hidden-messages)
-test_expect_equal "$output" "$expected"
+test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
+	    (search-forward "Click/Enter to show.")
+	    (button-activate (button-at (point)))
+	    (notmuch-show-toggle-message)
+	    (test-visible-output)'
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
 
 test_done
diff --git a/test/emacs-large-search-buffer b/test/emacs-large-search-buffer
index c78ce33..6095e9d 100755
--- a/test/emacs-large-search-buffer
+++ b/test/emacs-large-search-buffer
@@ -19,16 +19,25 @@ done
 notmuch new > /dev/null
 
 test_begin_subtest "Ensure that emacs doesn't drop results"
-expected="$(notmuch search '*' | sed -e 's/^thread:[0-9a-f]*  //' -e 's/;//' -e  's/xx*/[BLOB]/')
-End of search results."
+notmuch search '*' > EXPEXTED
+sed -i -e 's/^thread:[0-9a-f]*  //' -e 's/;//' -e 's/xx*/[BLOB]/' EXPEXTED
+echo 'End of search results.' >> EXPEXTED
 
-output=$(test_emacs '(notmuch-search "*") (notmuch-test-wait) (princ (buffer-string))' | sed -e s',  *, ,g' -e 's/xxx*/[BLOB]/g')
-test_expect_equal "$output" "$expected"
+test_emacs '(notmuch-search "*")
+	    (notmuch-test-wait)
+	    (test-output)'
+sed -i -e s',  *, ,g' -e 's/xxx*/[BLOB]/g' OUTPUT
+test_expect_equal_file OUTPUT EXPEXTED
 
 test_begin_subtest "Ensure that emacs doesn't drop error messages"
-output=$(test_emacs '(notmuch-search "--this-option-does-not-exist") (notmuch-test-wait) (princ (buffer-string))')
-test_expect_equal "$output" "Error: Unexpected output from notmuch search:
+test_emacs '(notmuch-search "--this-option-does-not-exist")
+	    (notmuch-test-wait)
+	    (test-output)'
+cat <<EOF >EXPEXTED
+Error: Unexpected output from notmuch search:
 Unrecognized option: --this-option-does-not-exist
-End of search results. (process returned 1)"
+End of search results. (process returned 1)
+EOF
+test_expect_equal_file OUTPUT EXPEXTED
 
 test_done
diff --git a/test/test-lib.el b/test/test-lib.el
index 344a02e..4e7f5cf 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -28,6 +28,16 @@
   (while (get-buffer-process (current-buffer))
     (sleep-for 0.1)))
 
+(defun test-output (&optional filename)
+  "Save current buffer to file FILENAME.  Default FILENAME is OUTPUT."
+  (write-region (point-min) (point-max) (or filename "OUTPUT")))
+
+(defun test-visible-output (&optional filename)
+  "Save visible text in current buffer to file FILENAME.  Default
+FILENAME is OUTPUT."
+  (let ((text (visible-buffer-string)))
+    (with-temp-file (or filename "OUTPUT") (insert text))))
+
 (defun visible-buffer-string ()
   "Same as `buffer-string', but excludes invisible text."
   (visible-buffer-substring (point-min) (point-max)))
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 5/5] test: remove some sed(1) calls in Emacs tests
  2011-06-26 23:52 [PATCH 1/5] test: do not set `message-signature' in test_emacs Dmitry Kurochkin
                   ` (2 preceding siblings ...)
  2011-06-26 23:52 ` [PATCH 4/5] test: save buffer content to file instead of printing it in Emacs tests Dmitry Kurochkin
@ 2011-06-26 23:52 ` Dmitry Kurochkin
  2011-06-27  3:54 ` [PATCH 1/2] test: use emacs_deliver_message in Emacs SMTP send test Dmitry Kurochkin
  4 siblings, 0 replies; 22+ messages in thread
From: Dmitry Kurochkin @ 2011-06-26 23:52 UTC (permalink / raw)
  To: notmuch

Few Emacs tests used sed(1) to remove unexpected output in the
beginning to avoid getting confused by messages such as "Parsing
/home/cworth/.mailrc... done".  This is no longer needed since
tests are run in a temporary home directory instead of the user's
one.  So remove these sed(1) calls.
---
 test/emacs |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/test/emacs b/test/emacs
index fc4c01f..f3239ea 100755
--- a/test/emacs
+++ b/test/emacs
@@ -250,13 +250,10 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Reply within emacs"
-# We sed away everything before the ^From in the output to avoid getting
-# confused by messages such as "Parsing /home/cworth/.mailrc... done"
 test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"")
 	    (notmuch-test-wait)
 	    (notmuch-search-reply-to-thread)
 	    (test-output)'
-sed -i -ne '/^From/,$ p' OUTPUT
 sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/' OUTPUT
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
@@ -284,11 +281,9 @@ test_emacs '(notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052
 test_expect_equal_file "$EXPECTED/attachment" attachment2.gz
 
 test_begin_subtest "View raw message within emacs"
-first_line=$(head -n1 $EXPECTED/raw-message-cf0c4d-52ad0a)
 test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
 	    (notmuch-show-view-raw-message)
 	    (test-output)'
-sed -i -ne "/$first_line/,\$ p" OUTPUT
 test_expect_equal_file OUTPUT $EXPECTED/raw-message-cf0c4d-52ad0a
 
 test_begin_subtest "Hiding/showing signature in notmuch-show view"
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 1/2] test: use emacs_deliver_message in Emacs SMTP send test
  2011-06-26 23:52 [PATCH 1/5] test: do not set `message-signature' in test_emacs Dmitry Kurochkin
                   ` (3 preceding siblings ...)
  2011-06-26 23:52 ` [PATCH 5/5] test: remove some sed(1) calls " Dmitry Kurochkin
@ 2011-06-27  3:54 ` Dmitry Kurochkin
  2011-06-27  3:54   ` [PATCH 2/2] test: use emacsclient(1) for Emacs tests Dmitry Kurochkin
  4 siblings, 1 reply; 22+ messages in thread
From: Dmitry Kurochkin @ 2011-06-27  3:54 UTC (permalink / raw)
  To: notmuch

Minor changes to expected results of other Emacs tests were
needed because the message Date header changed.
---
 test/emacs |   34 +++++++++-------------------------
 1 files changed, 9 insertions(+), 25 deletions(-)

diff --git a/test/emacs b/test/emacs
index f3239ea..4f16b41 100755
--- a/test/emacs
+++ b/test/emacs
@@ -118,28 +118,12 @@ output=$(notmuch search 'id:"123..456@example"' | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Message with .. in Message-Id (inbox search-add show-add)"
 
 test_begin_subtest "Sending a message via (fake) SMTP"
-
-# Before we can send a message, we have to prepare the FCC maildir
-mkdir -p mail/sent/cur
-mkdir -p mail/sent/new
-mkdir -p mail/sent/tmp
-
-../smtp-dummy sent_message &
-smtp_dummy_pid=$!
-test_emacs "(setq message-send-mail-function 'message-smtpmail-send-it)
-	    (setq smtpmail-smtp-server \"localhost\")
-	    (setq smtpmail-smtp-service \"25025\")
-	    (notmuch-hello)
-	    (notmuch-mua-mail)
-	    (message-goto-to)
-	    (insert \"user@example.com\nDate: Fri, 29 Mar 1974 10:00:00 -0000\")
-	    (message-goto-subject)
-	    (insert \"Testing message sent via SMTP\")
-	    (message-goto-body)
-	    (insert \"This is a test that messages are sent via SMTP\")
-	    (message-send-and-exit)" >/dev/null 2>&1
-wait ${smtp_dummy_pid}
-
+emacs_deliver_message \
+    'Testing message sent via SMTP' \
+    'This is a test that messages are sent via SMTP' \
+    '(message-goto-to)
+     (kill-whole-line)
+     (insert "To: user@example.com\n")'
 sed \
     -e s',^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' \
     -e s',^Message-ID: <.*>$,Message-ID: <XXX>,' < sent_message >OUTPUT
@@ -147,7 +131,7 @@ cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: user@example.com
 Subject: Testing message sent via SMTP
-Date: Fri, 29 Mar 1974 10:00:00 -0000
+Date: 01 Jan 2000 12:00:00 -0000
 User-Agent: Notmuch/XXX Emacs/XXX
 Message-ID: <XXX>
 MIME-Version: 1.0
@@ -160,7 +144,7 @@ test_expect_equal_file OUTPUT EXPECTED
 test_begin_subtest "Verify that sent messages are saved/searchable (via FCC)"
 notmuch new > /dev/null
 output=$(notmuch search 'subject:"testing message sent via SMTP"' | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   1974-03-29 [1/1] Notmuch Test Suite; Testing message sent via SMTP (inbox)"
+test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; Testing message sent via SMTP (inbox)"
 
 test_begin_subtest "notmuch-fcc-dirs set to nil"
 test_emacs "(setq notmuch-fcc-dirs nil)
@@ -262,7 +246,7 @@ Subject: Re: Testing message sent via SMTP
 In-Reply-To: <XXX>
 Fcc: $(pwd)/mail/sent
 --text follows this line--
-On Fri, 29 Mar 1974 10:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
+On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
 > This is a test that messages are sent via SMTP
 EOF
 test_expect_equal_file OUTPUT EXPECTED
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-27  3:54 ` [PATCH 1/2] test: use emacs_deliver_message in Emacs SMTP send test Dmitry Kurochkin
@ 2011-06-27  3:54   ` Dmitry Kurochkin
  2011-06-27 20:02     ` Austin Clements
  0 siblings, 1 reply; 22+ messages in thread
From: Dmitry Kurochkin @ 2011-06-27  3:54 UTC (permalink / raw)
  To: notmuch

Before the change, every Emacs tests ran in a separate Emacs
instance.  Starting Emacs many times wastes considerable time and
it gets worse as the test suite grows.  The patch solves this by
using a single Emacs server and emacsclient(1) to run multiple
tests.  Emacs server is started on the first test_emacs call and
stopped when test_done is called or the test is killed by a
signal.  Several auxiliary scripts useful for debugging and test
development are generated instead of the run_emacs script:

  * emacs_server_start - start Emacs server
  * emacs_server_stop  - stop Emacs server
  * emacs_start        - start Emacs
  * emacs_run          - execute ELisp expressions in running Emacs server

Since multiple tests are run in a single Emacs instance, they
must not change Emacs environment because it may affect other
tests.  For now, the only Emacs environment modifications done by
the tests are variable settings.  Before the change, variables
were set with `setq' which affected other tests.  The patch
changes all variables to use `let', so the scope of the change is
limited to a single test.
---
 test/emacs       |   74 +++++++++++++-------------
 test/test-lib.el |    6 ++
 test/test-lib.sh |  149 ++++++++++++++++++++++++++++++++++++++++++-----------
 3 files changed, 161 insertions(+), 68 deletions(-)

diff --git a/test/emacs b/test/emacs
index 4f16b41..f1939dc 100755
--- a/test/emacs
+++ b/test/emacs
@@ -12,20 +12,20 @@ test_emacs '(notmuch-hello)
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello
 
 test_begin_subtest "Saved search with 0 results"
-test_emacs '(setq notmuch-show-empty-saved-searches t)
-	    (setq notmuch-saved-searches
-		  '\''(("inbox" . "tag:inbox")
-		       ("unread" . "tag:unread")
-		       ("empty" . "tag:doesnotexist")))
-	    (notmuch-hello)
-	    (test-output)'
+test_emacs '(let ((notmuch-show-empty-saved-searches t)
+		  (notmuch-saved-searches
+		   '\''(("inbox" . "tag:inbox")
+			("unread" . "tag:unread")
+			("empty" . "tag:doesnotexist"))))
+	      (notmuch-hello)
+	      (test-output))'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-with-empty
 
 test_begin_subtest "No saved searches displayed (all with 0 results)"
-test_emacs '(setq notmuch-saved-searches
-		  '\''(("empty" . "tag:doesnotexist")))
-	    (notmuch-hello)
-	    (test-output)'
+test_emacs '(let ((notmuch-saved-searches
+		   '\''(("empty" . "tag:doesnotexist"))))
+	      (notmuch-hello)
+	      (test-output))'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-no-saved-searches
 
 test_begin_subtest "Basic notmuch-search view in emacs"
@@ -147,9 +147,9 @@ output=$(notmuch search 'subject:"testing message sent via SMTP"' | notmuch_sear
 test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; Testing message sent via SMTP (inbox)"
 
 test_begin_subtest "notmuch-fcc-dirs set to nil"
-test_emacs "(setq notmuch-fcc-dirs nil)
-	    (notmuch-mua-mail)
-	    (test-output)"
+test_emacs "(let ((notmuch-fcc-dirs nil))
+	      (notmuch-mua-mail)
+	      (test-output))"
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -164,9 +164,9 @@ mkdir -p mail/sent-string/new
 mkdir -p mail/sent-string/tmp
 
 test_begin_subtest "notmuch-fcc-dirs set to a string"
-test_emacs "(setq notmuch-fcc-dirs \"sent-string\")
-	    (notmuch-mua-mail)
-	    (test-output)"
+test_emacs "(let ((notmuch-fcc-dirs \"sent-string\"))
+	      (notmuch-mua-mail)
+	      (test-output))"
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -185,11 +185,11 @@ mkdir -p mail/failure/new
 mkdir -p mail/failure/tmp
 
 test_begin_subtest "notmuch-fcc-dirs set to a list (with match)"
-test_emacs "(setq notmuch-fcc-dirs
-		  '((\"notmuchmail.org\" . \"sent-list-match\")
-		    (\".*\" . \"failure\")))
-	    (notmuch-mua-mail)
-	    (test-output)"
+test_emacs "(let ((notmuch-fcc-dirs
+		   '((\"notmuchmail.org\" . \"sent-list-match\")
+		     (\".*\" . \"failure\"))))
+	      (notmuch-mua-mail)
+	      (test-output))"
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -205,11 +205,11 @@ mkdir -p mail/sent-list-catch-all/new
 mkdir -p mail/sent-list-catch-all/tmp
  
 test_begin_subtest "notmuch-fcc-dirs set to a list (catch-all)"
-test_emacs "(setq notmuch-fcc-dirs
-		  '((\"example.com\" . \"failure\")
-		    (\".*\" . \"sent-list-catch-all\")))
-	    (notmuch-mua-mail)
-	    (test-output)"
+test_emacs "(let ((notmuch-fcc-dirs
+		   '((\"example.com\" . \"failure\")
+		     (\".*\" . \"sent-list-catch-all\"))))
+	      (notmuch-mua-mail)
+	      (test-output))"
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -220,11 +220,11 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "notmuch-fcc-dirs set to a list (no match)"
-test_emacs "(setq notmuch-fcc-dirs
-		  '((\"example.com\" . \"failure\")
-		    (\"nomatchhere.net\" . \"failure\")))
-	    (notmuch-mua-mail)
-	    (test-output)"
+test_emacs "(let ((notmuch-fcc-dirs
+		   '((\"example.com\" . \"failure\")
+		     (\"nomatchhere.net\" . \"failure\"))))
+	      (notmuch-mua-mail)
+	      (test-output))"
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: 
@@ -253,15 +253,15 @@ test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Save attachment from within emacs using notmuch-show-save-attachments"
 # save as archive to test that Emacs does not re-compress .gz
-echo ./attachment1.gz |
-test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
-	    (notmuch-show-save-attachments)' > /dev/null 2>&1
+test_emacs '(let ((standard-input "\"attachment1.gz\""))
+	      (notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
+	      (notmuch-show-save-attachments))'
 test_expect_equal_file "$EXPECTED/attachment" attachment1.gz
 
 test_begin_subtest "Save attachment from within emacs using notmuch-show-save-part"
 # save as archive to test that Emacs does not re-compress .gz
-echo ./attachment2.gz |
-test_emacs '(notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com" 5)' > /dev/null 2>&1
+test_emacs '(let ((standard-input "\"attachment2.gz\""))
+	      (notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com" 5))' > /dev/null 2>&1
 test_expect_equal_file "$EXPECTED/attachment" attachment2.gz
 
 test_begin_subtest "View raw message within emacs"
diff --git a/test/test-lib.el b/test/test-lib.el
index 4e7f5cf..a5a3125 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -23,6 +23,12 @@
 ;; avoid crazy 10-column default of --batch
 (set-frame-width (window-frame (get-buffer-window)) 80)
 
+;; `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
+;; `read' call.
+(setq read-file-name-function (lambda (&rest _) (read)))
+
 (defun notmuch-test-wait ()
   "Wait for process completion."
   (while (get-buffer-process (current-buffer))
diff --git a/test/test-lib.sh b/test/test-lib.sh
index ad1506c..1c1581b 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -57,6 +57,9 @@ unset CDPATH
 
 unset GREP_OPTIONS
 
+# PID of running Emacs server
+emacs_server_pid=
+
 # Convenience
 #
 # A regexp to match 5 and 40 hexdigits
@@ -174,6 +177,7 @@ test_success=0
 
 die () {
 	code=$?
+	emacs_server_stop
 	if test -n "$GIT_EXIT_OK"
 	then
 		exit $code
@@ -394,19 +398,20 @@ emacs_deliver_message ()
     mkdir -p "$MAIL_DIR"/sent/{cur,new,tmp}
     ../smtp-dummy sent_message &
     smtp_dummy_pid=$!
-    test_emacs "(setq message-send-mail-function 'message-smtpmail-send-it)
-		(setq smtpmail-smtp-server \"localhost\")
-		(setq smtpmail-smtp-service \"25025\")
-		(notmuch-hello)
-		(notmuch-mua-mail)
-		(message-goto-to)
-		(insert \"test_suite@notmuchmail.org\nDate: 01 Jan 2000 12:00:00 -0000\")
-		(message-goto-subject)
-		(insert \"${subject}\")
-		(message-goto-body)
-		(insert \"${body}\")
-		$@
-		(message-send-and-exit)" >/dev/null 2>&1
+    test_emacs \
+	"(let ((message-send-mail-function 'message-smtpmail-send-it)
+	       (smtpmail-smtp-server \"localhost\")
+	       (smtpmail-smtp-service \"25025\"))
+	   (notmuch-hello)
+	   (notmuch-mua-mail)
+	   (message-goto-to)
+	   (insert \"test_suite@notmuchmail.org\nDate: 01 Jan 2000 12:00:00 -0000\")
+	   (message-goto-subject)
+	   (insert \"${subject}\")
+	   (message-goto-body)
+	   (insert \"${body}\")
+	   $@
+	   (message-send-and-exit))" >/dev/null 2>&1
     wait ${smtp_dummy_pid}
     notmuch new >/dev/null
 }
@@ -828,6 +833,8 @@ test_done () {
 
 	echo
 
+	emacs_server_stop
+
 	if [ "$test_failure" = "0" ]; then
 	    if [ "$test_broken" = "0" ]; then	    
 		rm -rf "$remove_tmp"
@@ -838,24 +845,26 @@ test_done () {
 	fi
 }
 
-test_emacs () {
-	# Construct a little test script here for the benefit of the user,
-	# (who can easily run "run_emacs" to get the same emacs environment
-	# for investigating any failures).    
-	cat <<EOF > run_emacs
+# Generate some scripts for running Emacs tests.  These scripts are
+# used by Emacs tests and help investigating failures.  The following
+# scripts are generated:
+#
+# * emacs_server_start	- start Emacs server
+# * emacs_server_stop	- stop Emacs server
+# * emacs_start		- start Emacs
+# * emacs_run		- execute ELisp expressions in running Emacs server
+emacs_generate_scripts ()
+{
+	server_name="notmuch-test-suite-$$"
+
+	cat <<EOF > "$TMP_DIRECTORY/emacs_server_start"
 #!/bin/sh
 export PATH=$PATH
 export NOTMUCH_CONFIG=$NOTMUCH_CONFIG
 
-# We assume that the user will give a command-line argument only if
-# wanting to run in batch mode.
-if [ \$# -gt 0 ]; then
-	BATCH=--batch
-fi
-
 # Here's what we are using here:
 #
-# --batch:		Quit after given commands and print all (messages)
+# --daemon		Start Emacs as a daemon
 #
 # --no-init-file	Don't load users ~/.emacs
 #
@@ -865,13 +874,90 @@ fi
 #
 # --load		Force loading of notmuch.el and test-lib.el
 
-emacs \$BATCH --no-init-file --no-site-file \
-	--directory ../../emacs --load notmuch.el \
-	--directory .. --load test-lib.el \
-	--eval "(progn \$@)"
+emacs --daemon --no-init-file --no-site-file \
+	--directory "$TEST_DIRECTORY/../emacs" --load notmuch.el \
+	--directory "$TEST_DIRECTORY" --load test-lib.el \
+	--eval '(setq server-name "$server_name")'
+EOF
+	chmod a+x "$TMP_DIRECTORY/emacs_server_start"
+
+	cat <<EOF > "$TMP_DIRECTORY/emacs_server_stop"
+#!/bin/sh
+
+dir=\$(dirname "\$0")
+"\$dir"/emacs_run '(kill-emacs)'
 EOF
-	chmod a+x ./run_emacs
-	./run_emacs "$@"
+	chmod a+x "$TMP_DIRECTORY/emacs_server_stop"
+
+	cat <<EOF > "$TMP_DIRECTORY/emacs_start"
+#!/bin/sh
+export PATH=$PATH
+export NOTMUCH_CONFIG=$NOTMUCH_CONFIG
+
+# Here's what we are using here:
+#
+# --no-init-file	Don't load users ~/.emacs
+#
+# --no-site-file	Don't load the site-wide startup stuff
+#
+# --directory		Ensure that the local elisp sources are found
+#
+# --load		Force loading of notmuch.el and test-lib.el
+
+emacs --no-init-file --no-site-file \
+	--directory "$TEST_DIRECTORY/../emacs" --load notmuch.el \
+	--directory "$TEST_DIRECTORY" --load test-lib.el
+EOF
+	chmod a+x "$TMP_DIRECTORY/emacs_start"
+
+	cat <<EOF > "$TMP_DIRECTORY/emacs_run"
+#!/bin/sh
+
+# Here's what we are using here:
+#
+# --socket-name		Emacs server name
+#
+# --eval		Evaluate ELisp expressions
+
+emacsclient --socket-name "$server_name" --eval "(progn \$@)"
+EOF
+	chmod a+x "$TMP_DIRECTORY/emacs_run"
+}
+
+# Start Emacs server if it is not running.
+emacs_server_start ()
+{
+	[ -n "$emacs_server_pid" ] && return
+
+	output=$("$TMP_DIRECTORY/emacs_server_start" 2>&1)
+	if [ "$?" -ne 0 ]; then
+		echo "$output"
+		return 1
+	fi
+
+	emacs_server_pid=$("$TMP_DIRECTORY/emacs_run" '(emacs-pid)')
+	[ "$?" -eq 0 -a -n "$emacs_server_pid" ]
+}
+
+# Stop Emacs server if it is running.
+emacs_server_stop ()
+{
+	[ -z "$emacs_server_pid" ] && return
+
+	emacs_server_pid=
+	output=$("$TMP_DIRECTORY/emacs_server_stop" 2>&1)
+	if [ "$?" -ne 0 ]; then
+		echo "$output"
+		return 1
+	fi
+}
+
+# Evaluate ELisp expressions in Emacs server.  Server is started if it
+# is not running.
+test_emacs () {
+	emacs_server_start || return
+
+	"$TMP_DIRECTORY/emacs_run" "$@"
 }
 
 
@@ -999,6 +1085,7 @@ primary_email=test_suite@notmuchmail.org
 other_email=test_suite_other@notmuchmail.org;test_suite@otherdomain.org
 EOF
 
+emacs_generate_scripts
 
 # Use -P to resolve symlinks in our working directory so that the cwd
 # in subprocesses like git equals our $PWD (for pathname comparisons).
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-27  3:54   ` [PATCH 2/2] test: use emacsclient(1) for Emacs tests Dmitry Kurochkin
@ 2011-06-27 20:02     ` Austin Clements
  2011-06-27 20:22       ` Dmitry Kurochkin
  0 siblings, 1 reply; 22+ messages in thread
From: Austin Clements @ 2011-06-27 20:02 UTC (permalink / raw)
  To: Dmitry Kurochkin; +Cc: notmuch

This looks like a great idea!  The test suite has been getting irritating slow.

A few minor comments: This patch would be clearer if it the
setq-to-let translation were a separate patch.  It would also be worth
adding a big comment at the top of the test explaining why all of the
tests let-bind everything instead of setq'ing, primarily for the
benefit of people writing new tests.

I might just be having trouble reading the patch, but the difference
between emacs_start and emacs_server_start seems unclear.  Perhaps the
comments should explain how somebody would use these scripts?


My bigger concern with this change is that it may leave behind stale
emacs daemons if the script gets interrupted.  The only way I know to
reliably kill a child process is to open a pipe to it and have it exit
on its own when it reads EOF.  Unfortunately, I couldn't find a way to
do this with an emacs daemon (it appears daemon mode aggressively
cleans up things like pipes), but here's a different approach:

    coproc emacs --batch --eval "(while t (eval (read)))"
    EMACSFD=${COPROC[1]}
    trap "echo '(kill-emacs)' >&$EMACSFD" EXIT

    echo '(message "Hi")' >&$EMACSFD
    # ...

This is, basically, a poor man's emacs server, but the coprocess pipe
binds it tightly to the shell.  If the shell exits for *any* reason,
the pipe will be closed by the kernel, emacs will read an EOF, and
exit.  The trap is there just to cleanly shut down in case of a normal
exit [1].  This also has the advantage that read-from-minibuffer still
works:

    echo '(message (read-from-minibuffer ""))' >&$EMACSFD
    echo 'Test' >&$EMACSFD

Thoughts?

[1] If you don't do this, emacs complains that it can't read from
stdin before it exits.  It would be nice to catch this condition in
the elisp code and not bother with the trap, but the error thrown is
just an 'error, so I don't think we can catch and ignore it without
catching and ignoring *all* errors.

On Sun, Jun 26, 2011 at 11:54 PM, Dmitry Kurochkin
<dmitry.kurochkin@gmail.com> wrote:
> Before the change, every Emacs tests ran in a separate Emacs
> instance.  Starting Emacs many times wastes considerable time and
> it gets worse as the test suite grows.  The patch solves this by
> using a single Emacs server and emacsclient(1) to run multiple
> tests.  Emacs server is started on the first test_emacs call and
> stopped when test_done is called or the test is killed by a
> signal.  Several auxiliary scripts useful for debugging and test
> development are generated instead of the run_emacs script:
>
>  * emacs_server_start - start Emacs server
>  * emacs_server_stop  - stop Emacs server
>  * emacs_start        - start Emacs
>  * emacs_run          - execute ELisp expressions in running Emacs server
>
> Since multiple tests are run in a single Emacs instance, they
> must not change Emacs environment because it may affect other
> tests.  For now, the only Emacs environment modifications done by
> the tests are variable settings.  Before the change, variables
> were set with `setq' which affected other tests.  The patch
> changes all variables to use `let', so the scope of the change is
> limited to a single test.
> ---
>  test/emacs       |   74 +++++++++++++-------------
>  test/test-lib.el |    6 ++
>  test/test-lib.sh |  149 ++++++++++++++++++++++++++++++++++++++++++-----------
>  3 files changed, 161 insertions(+), 68 deletions(-)
>
> diff --git a/test/emacs b/test/emacs
> index 4f16b41..f1939dc 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -12,20 +12,20 @@ test_emacs '(notmuch-hello)
>  test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello
>
>  test_begin_subtest "Saved search with 0 results"
> -test_emacs '(setq notmuch-show-empty-saved-searches t)
> -           (setq notmuch-saved-searches
> -                 '\''(("inbox" . "tag:inbox")
> -                      ("unread" . "tag:unread")
> -                      ("empty" . "tag:doesnotexist")))
> -           (notmuch-hello)
> -           (test-output)'
> +test_emacs '(let ((notmuch-show-empty-saved-searches t)
> +                 (notmuch-saved-searches
> +                  '\''(("inbox" . "tag:inbox")
> +                       ("unread" . "tag:unread")
> +                       ("empty" . "tag:doesnotexist"))))
> +             (notmuch-hello)
> +             (test-output))'
>  test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-with-empty
>
>  test_begin_subtest "No saved searches displayed (all with 0 results)"
> -test_emacs '(setq notmuch-saved-searches
> -                 '\''(("empty" . "tag:doesnotexist")))
> -           (notmuch-hello)
> -           (test-output)'
> +test_emacs '(let ((notmuch-saved-searches
> +                  '\''(("empty" . "tag:doesnotexist"))))
> +             (notmuch-hello)
> +             (test-output))'
>  test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-no-saved-searches
>
>  test_begin_subtest "Basic notmuch-search view in emacs"
> @@ -147,9 +147,9 @@ output=$(notmuch search 'subject:"testing message sent via SMTP"' | notmuch_sear
>  test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; Testing message sent via SMTP (inbox)"
>
>  test_begin_subtest "notmuch-fcc-dirs set to nil"
> -test_emacs "(setq notmuch-fcc-dirs nil)
> -           (notmuch-mua-mail)
> -           (test-output)"
> +test_emacs "(let ((notmuch-fcc-dirs nil))
> +             (notmuch-mua-mail)
> +             (test-output))"
>  cat <<EOF >EXPECTED
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  To:
> @@ -164,9 +164,9 @@ mkdir -p mail/sent-string/new
>  mkdir -p mail/sent-string/tmp
>
>  test_begin_subtest "notmuch-fcc-dirs set to a string"
> -test_emacs "(setq notmuch-fcc-dirs \"sent-string\")
> -           (notmuch-mua-mail)
> -           (test-output)"
> +test_emacs "(let ((notmuch-fcc-dirs \"sent-string\"))
> +             (notmuch-mua-mail)
> +             (test-output))"
>  cat <<EOF >EXPECTED
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  To:
> @@ -185,11 +185,11 @@ mkdir -p mail/failure/new
>  mkdir -p mail/failure/tmp
>
>  test_begin_subtest "notmuch-fcc-dirs set to a list (with match)"
> -test_emacs "(setq notmuch-fcc-dirs
> -                 '((\"notmuchmail.org\" . \"sent-list-match\")
> -                   (\".*\" . \"failure\")))
> -           (notmuch-mua-mail)
> -           (test-output)"
> +test_emacs "(let ((notmuch-fcc-dirs
> +                  '((\"notmuchmail.org\" . \"sent-list-match\")
> +                    (\".*\" . \"failure\"))))
> +             (notmuch-mua-mail)
> +             (test-output))"
>  cat <<EOF >EXPECTED
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  To:
> @@ -205,11 +205,11 @@ mkdir -p mail/sent-list-catch-all/new
>  mkdir -p mail/sent-list-catch-all/tmp
>
>  test_begin_subtest "notmuch-fcc-dirs set to a list (catch-all)"
> -test_emacs "(setq notmuch-fcc-dirs
> -                 '((\"example.com\" . \"failure\")
> -                   (\".*\" . \"sent-list-catch-all\")))
> -           (notmuch-mua-mail)
> -           (test-output)"
> +test_emacs "(let ((notmuch-fcc-dirs
> +                  '((\"example.com\" . \"failure\")
> +                    (\".*\" . \"sent-list-catch-all\"))))
> +             (notmuch-mua-mail)
> +             (test-output))"
>  cat <<EOF >EXPECTED
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  To:
> @@ -220,11 +220,11 @@ EOF
>  test_expect_equal_file OUTPUT EXPECTED
>
>  test_begin_subtest "notmuch-fcc-dirs set to a list (no match)"
> -test_emacs "(setq notmuch-fcc-dirs
> -                 '((\"example.com\" . \"failure\")
> -                   (\"nomatchhere.net\" . \"failure\")))
> -           (notmuch-mua-mail)
> -           (test-output)"
> +test_emacs "(let ((notmuch-fcc-dirs
> +                  '((\"example.com\" . \"failure\")
> +                    (\"nomatchhere.net\" . \"failure\"))))
> +             (notmuch-mua-mail)
> +             (test-output))"
>  cat <<EOF >EXPECTED
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  To:
> @@ -253,15 +253,15 @@ test_expect_equal_file OUTPUT EXPECTED
>
>  test_begin_subtest "Save attachment from within emacs using notmuch-show-save-attachments"
>  # save as archive to test that Emacs does not re-compress .gz
> -echo ./attachment1.gz |
> -test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
> -           (notmuch-show-save-attachments)' > /dev/null 2>&1
> +test_emacs '(let ((standard-input "\"attachment1.gz\""))
> +             (notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
> +             (notmuch-show-save-attachments))'
>  test_expect_equal_file "$EXPECTED/attachment" attachment1.gz
>
>  test_begin_subtest "Save attachment from within emacs using notmuch-show-save-part"
>  # save as archive to test that Emacs does not re-compress .gz
> -echo ./attachment2.gz |
> -test_emacs '(notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com" 5)' > /dev/null 2>&1
> +test_emacs '(let ((standard-input "\"attachment2.gz\""))
> +             (notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com" 5))' > /dev/null 2>&1
>  test_expect_equal_file "$EXPECTED/attachment" attachment2.gz
>
>  test_begin_subtest "View raw message within emacs"
> diff --git a/test/test-lib.el b/test/test-lib.el
> index 4e7f5cf..a5a3125 100644
> --- a/test/test-lib.el
> +++ b/test/test-lib.el
> @@ -23,6 +23,12 @@
>  ;; avoid crazy 10-column default of --batch
>  (set-frame-width (window-frame (get-buffer-window)) 80)
>
> +;; `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
> +;; `read' call.
> +(setq read-file-name-function (lambda (&rest _) (read)))
> +
>  (defun notmuch-test-wait ()
>   "Wait for process completion."
>   (while (get-buffer-process (current-buffer))
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index ad1506c..1c1581b 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -57,6 +57,9 @@ unset CDPATH
>
>  unset GREP_OPTIONS
>
> +# PID of running Emacs server
> +emacs_server_pid=
> +
>  # Convenience
>  #
>  # A regexp to match 5 and 40 hexdigits
> @@ -174,6 +177,7 @@ test_success=0
>
>  die () {
>        code=$?
> +       emacs_server_stop
>        if test -n "$GIT_EXIT_OK"
>        then
>                exit $code
> @@ -394,19 +398,20 @@ emacs_deliver_message ()
>     mkdir -p "$MAIL_DIR"/sent/{cur,new,tmp}
>     ../smtp-dummy sent_message &
>     smtp_dummy_pid=$!
> -    test_emacs "(setq message-send-mail-function 'message-smtpmail-send-it)
> -               (setq smtpmail-smtp-server \"localhost\")
> -               (setq smtpmail-smtp-service \"25025\")
> -               (notmuch-hello)
> -               (notmuch-mua-mail)
> -               (message-goto-to)
> -               (insert \"test_suite@notmuchmail.org\nDate: 01 Jan 2000 12:00:00 -0000\")
> -               (message-goto-subject)
> -               (insert \"${subject}\")
> -               (message-goto-body)
> -               (insert \"${body}\")
> -               $@
> -               (message-send-and-exit)" >/dev/null 2>&1
> +    test_emacs \
> +       "(let ((message-send-mail-function 'message-smtpmail-send-it)
> +              (smtpmail-smtp-server \"localhost\")
> +              (smtpmail-smtp-service \"25025\"))
> +          (notmuch-hello)
> +          (notmuch-mua-mail)
> +          (message-goto-to)
> +          (insert \"test_suite@notmuchmail.org\nDate: 01 Jan 2000 12:00:00 -0000\")
> +          (message-goto-subject)
> +          (insert \"${subject}\")
> +          (message-goto-body)
> +          (insert \"${body}\")
> +          $@
> +          (message-send-and-exit))" >/dev/null 2>&1
>     wait ${smtp_dummy_pid}
>     notmuch new >/dev/null
>  }
> @@ -828,6 +833,8 @@ test_done () {
>
>        echo
>
> +       emacs_server_stop
> +
>        if [ "$test_failure" = "0" ]; then
>            if [ "$test_broken" = "0" ]; then
>                rm -rf "$remove_tmp"
> @@ -838,24 +845,26 @@ test_done () {
>        fi
>  }
>
> -test_emacs () {
> -       # Construct a little test script here for the benefit of the user,
> -       # (who can easily run "run_emacs" to get the same emacs environment
> -       # for investigating any failures).
> -       cat <<EOF > run_emacs
> +# Generate some scripts for running Emacs tests.  These scripts are
> +# used by Emacs tests and help investigating failures.  The following
> +# scripts are generated:
> +#
> +# * emacs_server_start - start Emacs server
> +# * emacs_server_stop  - stop Emacs server
> +# * emacs_start                - start Emacs
> +# * emacs_run          - execute ELisp expressions in running Emacs server
> +emacs_generate_scripts ()
> +{
> +       server_name="notmuch-test-suite-$$"
> +
> +       cat <<EOF > "$TMP_DIRECTORY/emacs_server_start"
>  #!/bin/sh
>  export PATH=$PATH
>  export NOTMUCH_CONFIG=$NOTMUCH_CONFIG
>
> -# We assume that the user will give a command-line argument only if
> -# wanting to run in batch mode.
> -if [ \$# -gt 0 ]; then
> -       BATCH=--batch
> -fi
> -
>  # Here's what we are using here:
>  #
> -# --batch:             Quit after given commands and print all (messages)
> +# --daemon             Start Emacs as a daemon
>  #
>  # --no-init-file       Don't load users ~/.emacs
>  #
> @@ -865,13 +874,90 @@ fi
>  #
>  # --load               Force loading of notmuch.el and test-lib.el
>
> -emacs \$BATCH --no-init-file --no-site-file \
> -       --directory ../../emacs --load notmuch.el \
> -       --directory .. --load test-lib.el \
> -       --eval "(progn \$@)"
> +emacs --daemon --no-init-file --no-site-file \
> +       --directory "$TEST_DIRECTORY/../emacs" --load notmuch.el \
> +       --directory "$TEST_DIRECTORY" --load test-lib.el \
> +       --eval '(setq server-name "$server_name")'
> +EOF
> +       chmod a+x "$TMP_DIRECTORY/emacs_server_start"
> +
> +       cat <<EOF > "$TMP_DIRECTORY/emacs_server_stop"
> +#!/bin/sh
> +
> +dir=\$(dirname "\$0")
> +"\$dir"/emacs_run '(kill-emacs)'
>  EOF
> -       chmod a+x ./run_emacs
> -       ./run_emacs "$@"
> +       chmod a+x "$TMP_DIRECTORY/emacs_server_stop"
> +
> +       cat <<EOF > "$TMP_DIRECTORY/emacs_start"
> +#!/bin/sh
> +export PATH=$PATH
> +export NOTMUCH_CONFIG=$NOTMUCH_CONFIG
> +
> +# Here's what we are using here:
> +#
> +# --no-init-file       Don't load users ~/.emacs
> +#
> +# --no-site-file       Don't load the site-wide startup stuff
> +#
> +# --directory          Ensure that the local elisp sources are found
> +#
> +# --load               Force loading of notmuch.el and test-lib.el
> +
> +emacs --no-init-file --no-site-file \
> +       --directory "$TEST_DIRECTORY/../emacs" --load notmuch.el \
> +       --directory "$TEST_DIRECTORY" --load test-lib.el
> +EOF
> +       chmod a+x "$TMP_DIRECTORY/emacs_start"
> +
> +       cat <<EOF > "$TMP_DIRECTORY/emacs_run"
> +#!/bin/sh
> +
> +# Here's what we are using here:
> +#
> +# --socket-name                Emacs server name
> +#
> +# --eval               Evaluate ELisp expressions
> +
> +emacsclient --socket-name "$server_name" --eval "(progn \$@)"
> +EOF
> +       chmod a+x "$TMP_DIRECTORY/emacs_run"
> +}
> +
> +# Start Emacs server if it is not running.
> +emacs_server_start ()
> +{
> +       [ -n "$emacs_server_pid" ] && return
> +
> +       output=$("$TMP_DIRECTORY/emacs_server_start" 2>&1)
> +       if [ "$?" -ne 0 ]; then
> +               echo "$output"
> +               return 1
> +       fi
> +
> +       emacs_server_pid=$("$TMP_DIRECTORY/emacs_run" '(emacs-pid)')
> +       [ "$?" -eq 0 -a -n "$emacs_server_pid" ]
> +}
> +
> +# Stop Emacs server if it is running.
> +emacs_server_stop ()
> +{
> +       [ -z "$emacs_server_pid" ] && return
> +
> +       emacs_server_pid=
> +       output=$("$TMP_DIRECTORY/emacs_server_stop" 2>&1)
> +       if [ "$?" -ne 0 ]; then
> +               echo "$output"
> +               return 1
> +       fi
> +}
> +
> +# Evaluate ELisp expressions in Emacs server.  Server is started if it
> +# is not running.
> +test_emacs () {
> +       emacs_server_start || return
> +
> +       "$TMP_DIRECTORY/emacs_run" "$@"
>  }
>
>
> @@ -999,6 +1085,7 @@ primary_email=test_suite@notmuchmail.org
>  other_email=test_suite_other@notmuchmail.org;test_suite@otherdomain.org
>  EOF
>
> +emacs_generate_scripts
>
>  # Use -P to resolve symlinks in our working directory so that the cwd
>  # in subprocesses like git equals our $PWD (for pathname comparisons).
> --
> 1.7.5.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
>

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-27 20:02     ` Austin Clements
@ 2011-06-27 20:22       ` Dmitry Kurochkin
  2011-06-27 20:32         ` Austin Clements
  2011-06-28  1:03         ` Dmitry Kurochkin
  0 siblings, 2 replies; 22+ messages in thread
From: Dmitry Kurochkin @ 2011-06-27 20:22 UTC (permalink / raw)
  To: Austin Clements; +Cc: notmuch

On Mon, 27 Jun 2011 16:02:12 -0400, Austin Clements <amdragon@mit.edu> wrote:
> This looks like a great idea!  The test suite has been getting irritating slow.
> 
> A few minor comments: This patch would be clearer if it the
> setq-to-let translation were a separate patch.  It would also be worth
> adding a big comment at the top of the test explaining why all of the
> tests let-bind everything instead of setq'ing, primarily for the
> benefit of people writing new tests.
> 

Agreed, will separate and add the warning.

> I might just be having trouble reading the patch, but the difference
> between emacs_start and emacs_server_start seems unclear.  Perhaps the
> comments should explain how somebody would use these scripts?
> 

emacs_start start a normal Emacs in non-daemon mode.  Something you
might prefer when debugging.

> 
> My bigger concern with this change is that it may leave behind stale
> emacs daemons if the script gets interrupted.

That is an issue indeed.  I would probably do smth like a periodic check
inside Emacs that the shell is alive, but your approach below looks more
reliable.

>  The only way I know to
> reliably kill a child process is to open a pipe to it and have it exit
> on its own when it reads EOF.  Unfortunately, I couldn't find a way to
> do this with an emacs daemon (it appears daemon mode aggressively
> cleans up things like pipes), but here's a different approach:
> 
>     coproc emacs --batch --eval "(while t (eval (read)))"
>     EMACSFD=${COPROC[1]}
>     trap "echo '(kill-emacs)' >&$EMACSFD" EXIT
> 
>     echo '(message "Hi")' >&$EMACSFD
>     # ...
> 
> This is, basically, a poor man's emacs server, but the coprocess pipe
> binds it tightly to the shell.  If the shell exits for *any* reason,
> the pipe will be closed by the kernel, emacs will read an EOF, and
> exit.

I like this idea.

>  The trap is there just to cleanly shut down in case of a normal
> exit [1].

For normal exit we should just put this into test_done.  Otherwise it is
not a normal exit and we do not care about Emacs error message.  No?

>  This also has the advantage that read-from-minibuffer still
> works:
> 
>     echo '(message (read-from-minibuffer ""))' >&$EMACSFD
>     echo 'Test' >&$EMACSFD
> 
> Thoughts?
> 

I like it and I will implement it.  Thanks for the idea.

Regards,
  Dmitry

> [1] If you don't do this, emacs complains that it can't read from
> stdin before it exits.  It would be nice to catch this condition in
> the elisp code and not bother with the trap, but the error thrown is
> just an 'error, so I don't think we can catch and ignore it without
> catching and ignoring *all* errors.
> 
> On Sun, Jun 26, 2011 at 11:54 PM, Dmitry Kurochkin
> <dmitry.kurochkin@gmail.com> wrote:
> > Before the change, every Emacs tests ran in a separate Emacs
> > instance.  Starting Emacs many times wastes considerable time and
> > it gets worse as the test suite grows.  The patch solves this by
> > using a single Emacs server and emacsclient(1) to run multiple
> > tests.  Emacs server is started on the first test_emacs call and
> > stopped when test_done is called or the test is killed by a
> > signal.  Several auxiliary scripts useful for debugging and test
> > development are generated instead of the run_emacs script:
> >
> >  * emacs_server_start - start Emacs server
> >  * emacs_server_stop  - stop Emacs server
> >  * emacs_start        - start Emacs
> >  * emacs_run          - execute ELisp expressions in running Emacs server
> >
> > Since multiple tests are run in a single Emacs instance, they
> > must not change Emacs environment because it may affect other
> > tests.  For now, the only Emacs environment modifications done by
> > the tests are variable settings.  Before the change, variables
> > were set with `setq' which affected other tests.  The patch
> > changes all variables to use `let', so the scope of the change is
> > limited to a single test.
> > ---
> >  test/emacs       |   74 +++++++++++++-------------
> >  test/test-lib.el |    6 ++
> >  test/test-lib.sh |  149 ++++++++++++++++++++++++++++++++++++++++++-----------
> >  3 files changed, 161 insertions(+), 68 deletions(-)
> >
> > diff --git a/test/emacs b/test/emacs
> > index 4f16b41..f1939dc 100755
> > --- a/test/emacs
> > +++ b/test/emacs
> > @@ -12,20 +12,20 @@ test_emacs '(notmuch-hello)
> >  test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello
> >
> >  test_begin_subtest "Saved search with 0 results"
> > -test_emacs '(setq notmuch-show-empty-saved-searches t)
> > -           (setq notmuch-saved-searches
> > -                 '\''(("inbox" . "tag:inbox")
> > -                      ("unread" . "tag:unread")
> > -                      ("empty" . "tag:doesnotexist")))
> > -           (notmuch-hello)
> > -           (test-output)'
> > +test_emacs '(let ((notmuch-show-empty-saved-searches t)
> > +                 (notmuch-saved-searches
> > +                  '\''(("inbox" . "tag:inbox")
> > +                       ("unread" . "tag:unread")
> > +                       ("empty" . "tag:doesnotexist"))))
> > +             (notmuch-hello)
> > +             (test-output))'
> >  test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-with-empty
> >
> >  test_begin_subtest "No saved searches displayed (all with 0 results)"
> > -test_emacs '(setq notmuch-saved-searches
> > -                 '\''(("empty" . "tag:doesnotexist")))
> > -           (notmuch-hello)
> > -           (test-output)'
> > +test_emacs '(let ((notmuch-saved-searches
> > +                  '\''(("empty" . "tag:doesnotexist"))))
> > +             (notmuch-hello)
> > +             (test-output))'
> >  test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-no-saved-searches
> >
> >  test_begin_subtest "Basic notmuch-search view in emacs"
> > @@ -147,9 +147,9 @@ output=$(notmuch search 'subject:"testing message sent via SMTP"' | notmuch_sear
> >  test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; Testing message sent via SMTP (inbox)"
> >
> >  test_begin_subtest "notmuch-fcc-dirs set to nil"
> > -test_emacs "(setq notmuch-fcc-dirs nil)
> > -           (notmuch-mua-mail)
> > -           (test-output)"
> > +test_emacs "(let ((notmuch-fcc-dirs nil))
> > +             (notmuch-mua-mail)
> > +             (test-output))"
> >  cat <<EOF >EXPECTED
> >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> >  To:
> > @@ -164,9 +164,9 @@ mkdir -p mail/sent-string/new
> >  mkdir -p mail/sent-string/tmp
> >
> >  test_begin_subtest "notmuch-fcc-dirs set to a string"
> > -test_emacs "(setq notmuch-fcc-dirs \"sent-string\")
> > -           (notmuch-mua-mail)
> > -           (test-output)"
> > +test_emacs "(let ((notmuch-fcc-dirs \"sent-string\"))
> > +             (notmuch-mua-mail)
> > +             (test-output))"
> >  cat <<EOF >EXPECTED
> >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> >  To:
> > @@ -185,11 +185,11 @@ mkdir -p mail/failure/new
> >  mkdir -p mail/failure/tmp
> >
> >  test_begin_subtest "notmuch-fcc-dirs set to a list (with match)"
> > -test_emacs "(setq notmuch-fcc-dirs
> > -                 '((\"notmuchmail.org\" . \"sent-list-match\")
> > -                   (\".*\" . \"failure\")))
> > -           (notmuch-mua-mail)
> > -           (test-output)"
> > +test_emacs "(let ((notmuch-fcc-dirs
> > +                  '((\"notmuchmail.org\" . \"sent-list-match\")
> > +                    (\".*\" . \"failure\"))))
> > +             (notmuch-mua-mail)
> > +             (test-output))"
> >  cat <<EOF >EXPECTED
> >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> >  To:
> > @@ -205,11 +205,11 @@ mkdir -p mail/sent-list-catch-all/new
> >  mkdir -p mail/sent-list-catch-all/tmp
> >
> >  test_begin_subtest "notmuch-fcc-dirs set to a list (catch-all)"
> > -test_emacs "(setq notmuch-fcc-dirs
> > -                 '((\"example.com\" . \"failure\")
> > -                   (\".*\" . \"sent-list-catch-all\")))
> > -           (notmuch-mua-mail)
> > -           (test-output)"
> > +test_emacs "(let ((notmuch-fcc-dirs
> > +                  '((\"example.com\" . \"failure\")
> > +                    (\".*\" . \"sent-list-catch-all\"))))
> > +             (notmuch-mua-mail)
> > +             (test-output))"
> >  cat <<EOF >EXPECTED
> >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> >  To:
> > @@ -220,11 +220,11 @@ EOF
> >  test_expect_equal_file OUTPUT EXPECTED
> >
> >  test_begin_subtest "notmuch-fcc-dirs set to a list (no match)"
> > -test_emacs "(setq notmuch-fcc-dirs
> > -                 '((\"example.com\" . \"failure\")
> > -                   (\"nomatchhere.net\" . \"failure\")))
> > -           (notmuch-mua-mail)
> > -           (test-output)"
> > +test_emacs "(let ((notmuch-fcc-dirs
> > +                  '((\"example.com\" . \"failure\")
> > +                    (\"nomatchhere.net\" . \"failure\"))))
> > +             (notmuch-mua-mail)
> > +             (test-output))"
> >  cat <<EOF >EXPECTED
> >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> >  To:
> > @@ -253,15 +253,15 @@ test_expect_equal_file OUTPUT EXPECTED
> >
> >  test_begin_subtest "Save attachment from within emacs using notmuch-show-save-attachments"
> >  # save as archive to test that Emacs does not re-compress .gz
> > -echo ./attachment1.gz |
> > -test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
> > -           (notmuch-show-save-attachments)' > /dev/null 2>&1
> > +test_emacs '(let ((standard-input "\"attachment1.gz\""))
> > +             (notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
> > +             (notmuch-show-save-attachments))'
> >  test_expect_equal_file "$EXPECTED/attachment" attachment1.gz
> >
> >  test_begin_subtest "Save attachment from within emacs using notmuch-show-save-part"
> >  # save as archive to test that Emacs does not re-compress .gz
> > -echo ./attachment2.gz |
> > -test_emacs '(notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com" 5)' > /dev/null 2>&1
> > +test_emacs '(let ((standard-input "\"attachment2.gz\""))
> > +             (notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com" 5))' > /dev/null 2>&1
> >  test_expect_equal_file "$EXPECTED/attachment" attachment2.gz
> >
> >  test_begin_subtest "View raw message within emacs"
> > diff --git a/test/test-lib.el b/test/test-lib.el
> > index 4e7f5cf..a5a3125 100644
> > --- a/test/test-lib.el
> > +++ b/test/test-lib.el
> > @@ -23,6 +23,12 @@
> >  ;; avoid crazy 10-column default of --batch
> >  (set-frame-width (window-frame (get-buffer-window)) 80)
> >
> > +;; `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
> > +;; `read' call.
> > +(setq read-file-name-function (lambda (&rest _) (read)))
> > +
> >  (defun notmuch-test-wait ()
> >   "Wait for process completion."
> >   (while (get-buffer-process (current-buffer))
> > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > index ad1506c..1c1581b 100755
> > --- a/test/test-lib.sh
> > +++ b/test/test-lib.sh
> > @@ -57,6 +57,9 @@ unset CDPATH
> >
> >  unset GREP_OPTIONS
> >
> > +# PID of running Emacs server
> > +emacs_server_pid=
> > +
> >  # Convenience
> >  #
> >  # A regexp to match 5 and 40 hexdigits
> > @@ -174,6 +177,7 @@ test_success=0
> >
> >  die () {
> >        code=$?
> > +       emacs_server_stop
> >        if test -n "$GIT_EXIT_OK"
> >        then
> >                exit $code
> > @@ -394,19 +398,20 @@ emacs_deliver_message ()
> >     mkdir -p "$MAIL_DIR"/sent/{cur,new,tmp}
> >     ../smtp-dummy sent_message &
> >     smtp_dummy_pid=$!
> > -    test_emacs "(setq message-send-mail-function 'message-smtpmail-send-it)
> > -               (setq smtpmail-smtp-server \"localhost\")
> > -               (setq smtpmail-smtp-service \"25025\")
> > -               (notmuch-hello)
> > -               (notmuch-mua-mail)
> > -               (message-goto-to)
> > -               (insert \"test_suite@notmuchmail.org\nDate: 01 Jan 2000 12:00:00 -0000\")
> > -               (message-goto-subject)
> > -               (insert \"${subject}\")
> > -               (message-goto-body)
> > -               (insert \"${body}\")
> > -               $@
> > -               (message-send-and-exit)" >/dev/null 2>&1
> > +    test_emacs \
> > +       "(let ((message-send-mail-function 'message-smtpmail-send-it)
> > +              (smtpmail-smtp-server \"localhost\")
> > +              (smtpmail-smtp-service \"25025\"))
> > +          (notmuch-hello)
> > +          (notmuch-mua-mail)
> > +          (message-goto-to)
> > +          (insert \"test_suite@notmuchmail.org\nDate: 01 Jan 2000 12:00:00 -0000\")
> > +          (message-goto-subject)
> > +          (insert \"${subject}\")
> > +          (message-goto-body)
> > +          (insert \"${body}\")
> > +          $@
> > +          (message-send-and-exit))" >/dev/null 2>&1
> >     wait ${smtp_dummy_pid}
> >     notmuch new >/dev/null
> >  }
> > @@ -828,6 +833,8 @@ test_done () {
> >
> >        echo
> >
> > +       emacs_server_stop
> > +
> >        if [ "$test_failure" = "0" ]; then
> >            if [ "$test_broken" = "0" ]; then
> >                rm -rf "$remove_tmp"
> > @@ -838,24 +845,26 @@ test_done () {
> >        fi
> >  }
> >
> > -test_emacs () {
> > -       # Construct a little test script here for the benefit of the user,
> > -       # (who can easily run "run_emacs" to get the same emacs environment
> > -       # for investigating any failures).
> > -       cat <<EOF > run_emacs
> > +# Generate some scripts for running Emacs tests.  These scripts are
> > +# used by Emacs tests and help investigating failures.  The following
> > +# scripts are generated:
> > +#
> > +# * emacs_server_start - start Emacs server
> > +# * emacs_server_stop  - stop Emacs server
> > +# * emacs_start                - start Emacs
> > +# * emacs_run          - execute ELisp expressions in running Emacs server
> > +emacs_generate_scripts ()
> > +{
> > +       server_name="notmuch-test-suite-$$"
> > +
> > +       cat <<EOF > "$TMP_DIRECTORY/emacs_server_start"
> >  #!/bin/sh
> >  export PATH=$PATH
> >  export NOTMUCH_CONFIG=$NOTMUCH_CONFIG
> >
> > -# We assume that the user will give a command-line argument only if
> > -# wanting to run in batch mode.
> > -if [ \$# -gt 0 ]; then
> > -       BATCH=--batch
> > -fi
> > -
> >  # Here's what we are using here:
> >  #
> > -# --batch:             Quit after given commands and print all (messages)
> > +# --daemon             Start Emacs as a daemon
> >  #
> >  # --no-init-file       Don't load users ~/.emacs
> >  #
> > @@ -865,13 +874,90 @@ fi
> >  #
> >  # --load               Force loading of notmuch.el and test-lib.el
> >
> > -emacs \$BATCH --no-init-file --no-site-file \
> > -       --directory ../../emacs --load notmuch.el \
> > -       --directory .. --load test-lib.el \
> > -       --eval "(progn \$@)"
> > +emacs --daemon --no-init-file --no-site-file \
> > +       --directory "$TEST_DIRECTORY/../emacs" --load notmuch.el \
> > +       --directory "$TEST_DIRECTORY" --load test-lib.el \
> > +       --eval '(setq server-name "$server_name")'
> > +EOF
> > +       chmod a+x "$TMP_DIRECTORY/emacs_server_start"
> > +
> > +       cat <<EOF > "$TMP_DIRECTORY/emacs_server_stop"
> > +#!/bin/sh
> > +
> > +dir=\$(dirname "\$0")
> > +"\$dir"/emacs_run '(kill-emacs)'
> >  EOF
> > -       chmod a+x ./run_emacs
> > -       ./run_emacs "$@"
> > +       chmod a+x "$TMP_DIRECTORY/emacs_server_stop"
> > +
> > +       cat <<EOF > "$TMP_DIRECTORY/emacs_start"
> > +#!/bin/sh
> > +export PATH=$PATH
> > +export NOTMUCH_CONFIG=$NOTMUCH_CONFIG
> > +
> > +# Here's what we are using here:
> > +#
> > +# --no-init-file       Don't load users ~/.emacs
> > +#
> > +# --no-site-file       Don't load the site-wide startup stuff
> > +#
> > +# --directory          Ensure that the local elisp sources are found
> > +#
> > +# --load               Force loading of notmuch.el and test-lib.el
> > +
> > +emacs --no-init-file --no-site-file \
> > +       --directory "$TEST_DIRECTORY/../emacs" --load notmuch.el \
> > +       --directory "$TEST_DIRECTORY" --load test-lib.el
> > +EOF
> > +       chmod a+x "$TMP_DIRECTORY/emacs_start"
> > +
> > +       cat <<EOF > "$TMP_DIRECTORY/emacs_run"
> > +#!/bin/sh
> > +
> > +# Here's what we are using here:
> > +#
> > +# --socket-name                Emacs server name
> > +#
> > +# --eval               Evaluate ELisp expressions
> > +
> > +emacsclient --socket-name "$server_name" --eval "(progn \$@)"
> > +EOF
> > +       chmod a+x "$TMP_DIRECTORY/emacs_run"
> > +}
> > +
> > +# Start Emacs server if it is not running.
> > +emacs_server_start ()
> > +{
> > +       [ -n "$emacs_server_pid" ] && return
> > +
> > +       output=$("$TMP_DIRECTORY/emacs_server_start" 2>&1)
> > +       if [ "$?" -ne 0 ]; then
> > +               echo "$output"
> > +               return 1
> > +       fi
> > +
> > +       emacs_server_pid=$("$TMP_DIRECTORY/emacs_run" '(emacs-pid)')
> > +       [ "$?" -eq 0 -a -n "$emacs_server_pid" ]
> > +}
> > +
> > +# Stop Emacs server if it is running.
> > +emacs_server_stop ()
> > +{
> > +       [ -z "$emacs_server_pid" ] && return
> > +
> > +       emacs_server_pid=
> > +       output=$("$TMP_DIRECTORY/emacs_server_stop" 2>&1)
> > +       if [ "$?" -ne 0 ]; then
> > +               echo "$output"
> > +               return 1
> > +       fi
> > +}
> > +
> > +# Evaluate ELisp expressions in Emacs server.  Server is started if it
> > +# is not running.
> > +test_emacs () {
> > +       emacs_server_start || return
> > +
> > +       "$TMP_DIRECTORY/emacs_run" "$@"
> >  }
> >
> >
> > @@ -999,6 +1085,7 @@ primary_email=test_suite@notmuchmail.org
> >  other_email=test_suite_other@notmuchmail.org;test_suite@otherdomain.org
> >  EOF
> >
> > +emacs_generate_scripts
> >
> >  # Use -P to resolve symlinks in our working directory so that the cwd
> >  # in subprocesses like git equals our $PWD (for pathname comparisons).
> > --
> > 1.7.5.4
> >
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch
> >

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-27 20:22       ` Dmitry Kurochkin
@ 2011-06-27 20:32         ` Austin Clements
  2011-06-28  1:03         ` Dmitry Kurochkin
  1 sibling, 0 replies; 22+ messages in thread
From: Austin Clements @ 2011-06-27 20:32 UTC (permalink / raw)
  To: Dmitry Kurochkin; +Cc: notmuch

On Mon, Jun 27, 2011 at 4:22 PM, Dmitry Kurochkin
<dmitry.kurochkin@gmail.com> wrote:
> On Mon, 27 Jun 2011 16:02:12 -0400, Austin Clements <amdragon@mit.edu> wrote:
>>  The trap is there just to cleanly shut down in case of a normal
>> exit [1].
>
> For normal exit we should just put this into test_done.  Otherwise it is
> not a normal exit and we do not care about Emacs error message.  No?

Yes, that's good, too, and it avoids the standard pitfalls of trap.

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-27 20:22       ` Dmitry Kurochkin
  2011-06-27 20:32         ` Austin Clements
@ 2011-06-28  1:03         ` Dmitry Kurochkin
  2011-06-28  3:49           ` Austin Clements
  1 sibling, 1 reply; 22+ messages in thread
From: Dmitry Kurochkin @ 2011-06-28  1:03 UTC (permalink / raw)
  To: Austin Clements; +Cc: notmuch

Austin,

On Tue, 28 Jun 2011 00:22:41 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Mon, 27 Jun 2011 16:02:12 -0400, Austin Clements <amdragon@mit.edu> wrote:
> > This looks like a great idea!  The test suite has been getting irritating slow.
> > 
> > A few minor comments: This patch would be clearer if it the
> > setq-to-let translation were a separate patch.  It would also be worth
> > adding a big comment at the top of the test explaining why all of the
> > tests let-bind everything instead of setq'ing, primarily for the
> > benefit of people writing new tests.
> > 
> 
> Agreed, will separate and add the warning.
> 
> > I might just be having trouble reading the patch, but the difference
> > between emacs_start and emacs_server_start seems unclear.  Perhaps the
> > comments should explain how somebody would use these scripts?
> > 
> 
> emacs_start start a normal Emacs in non-daemon mode.  Something you
> might prefer when debugging.
> 
> > 
> > My bigger concern with this change is that it may leave behind stale
> > emacs daemons if the script gets interrupted.
> 
> That is an issue indeed.  I would probably do smth like a periodic check
> inside Emacs that the shell is alive, but your approach below looks more
> reliable.
> 
> >  The only way I know to
> > reliably kill a child process is to open a pipe to it and have it exit
> > on its own when it reads EOF.  Unfortunately, I couldn't find a way to
> > do this with an emacs daemon (it appears daemon mode aggressively
> > cleans up things like pipes), but here's a different approach:
> > 
> >     coproc emacs --batch --eval "(while t (eval (read)))"
> >     EMACSFD=${COPROC[1]}
> >     trap "echo '(kill-emacs)' >&$EMACSFD" EXIT
> > 
> >     echo '(message "Hi")' >&$EMACSFD
> >     # ...
> > 
> > This is, basically, a poor man's emacs server, but the coprocess pipe
> > binds it tightly to the shell.  If the shell exits for *any* reason,
> > the pipe will be closed by the kernel, emacs will read an EOF, and
> > exit.
> 
> I like this idea.
> 
> >  The trap is there just to cleanly shut down in case of a normal
> > exit [1].
> 
> For normal exit we should just put this into test_done.  Otherwise it is
> not a normal exit and we do not care about Emacs error message.  No?
> 
> >  This also has the advantage that read-from-minibuffer still
> > works:
> > 
> >     echo '(message (read-from-minibuffer ""))' >&$EMACSFD
> >     echo 'Test' >&$EMACSFD
> > 
> > Thoughts?
> > 
> 
> I like it and I will implement it.  Thanks for the idea.
> 

While implementing the idea, I stumbled upon a problem: we need to know
when Emacs finished what we echoed or failed with an error.  At the
moment tests fail because they check for OUTPUT before Emacs creates it.

We can tell Emacs to print some special marker and wait for it.  But
there may be exceptions and errors which may make it difficult.  I did
not found a good solution yet.  Would love to hear your thoughts :)

Regards,
  Dmitry

> Regards,
>   Dmitry
> 
> > [1] If you don't do this, emacs complains that it can't read from
> > stdin before it exits.  It would be nice to catch this condition in
> > the elisp code and not bother with the trap, but the error thrown is
> > just an 'error, so I don't think we can catch and ignore it without
> > catching and ignoring *all* errors.
> > 
> > On Sun, Jun 26, 2011 at 11:54 PM, Dmitry Kurochkin
> > <dmitry.kurochkin@gmail.com> wrote:
> > > Before the change, every Emacs tests ran in a separate Emacs
> > > instance.  Starting Emacs many times wastes considerable time and
> > > it gets worse as the test suite grows.  The patch solves this by
> > > using a single Emacs server and emacsclient(1) to run multiple
> > > tests.  Emacs server is started on the first test_emacs call and
> > > stopped when test_done is called or the test is killed by a
> > > signal.  Several auxiliary scripts useful for debugging and test
> > > development are generated instead of the run_emacs script:
> > >
> > >  * emacs_server_start - start Emacs server
> > >  * emacs_server_stop  - stop Emacs server
> > >  * emacs_start        - start Emacs
> > >  * emacs_run          - execute ELisp expressions in running Emacs server
> > >
> > > Since multiple tests are run in a single Emacs instance, they
> > > must not change Emacs environment because it may affect other
> > > tests.  For now, the only Emacs environment modifications done by
> > > the tests are variable settings.  Before the change, variables
> > > were set with `setq' which affected other tests.  The patch
> > > changes all variables to use `let', so the scope of the change is
> > > limited to a single test.
> > > ---
> > >  test/emacs       |   74 +++++++++++++-------------
> > >  test/test-lib.el |    6 ++
> > >  test/test-lib.sh |  149 ++++++++++++++++++++++++++++++++++++++++++-----------
> > >  3 files changed, 161 insertions(+), 68 deletions(-)
> > >
> > > diff --git a/test/emacs b/test/emacs
> > > index 4f16b41..f1939dc 100755
> > > --- a/test/emacs
> > > +++ b/test/emacs
> > > @@ -12,20 +12,20 @@ test_emacs '(notmuch-hello)
> > >  test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello
> > >
> > >  test_begin_subtest "Saved search with 0 results"
> > > -test_emacs '(setq notmuch-show-empty-saved-searches t)
> > > -           (setq notmuch-saved-searches
> > > -                 '\''(("inbox" . "tag:inbox")
> > > -                      ("unread" . "tag:unread")
> > > -                      ("empty" . "tag:doesnotexist")))
> > > -           (notmuch-hello)
> > > -           (test-output)'
> > > +test_emacs '(let ((notmuch-show-empty-saved-searches t)
> > > +                 (notmuch-saved-searches
> > > +                  '\''(("inbox" . "tag:inbox")
> > > +                       ("unread" . "tag:unread")
> > > +                       ("empty" . "tag:doesnotexist"))))
> > > +             (notmuch-hello)
> > > +             (test-output))'
> > >  test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-with-empty
> > >
> > >  test_begin_subtest "No saved searches displayed (all with 0 results)"
> > > -test_emacs '(setq notmuch-saved-searches
> > > -                 '\''(("empty" . "tag:doesnotexist")))
> > > -           (notmuch-hello)
> > > -           (test-output)'
> > > +test_emacs '(let ((notmuch-saved-searches
> > > +                  '\''(("empty" . "tag:doesnotexist"))))
> > > +             (notmuch-hello)
> > > +             (test-output))'
> > >  test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-no-saved-searches
> > >
> > >  test_begin_subtest "Basic notmuch-search view in emacs"
> > > @@ -147,9 +147,9 @@ output=$(notmuch search 'subject:"testing message sent via SMTP"' | notmuch_sear
> > >  test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; Testing message sent via SMTP (inbox)"
> > >
> > >  test_begin_subtest "notmuch-fcc-dirs set to nil"
> > > -test_emacs "(setq notmuch-fcc-dirs nil)
> > > -           (notmuch-mua-mail)
> > > -           (test-output)"
> > > +test_emacs "(let ((notmuch-fcc-dirs nil))
> > > +             (notmuch-mua-mail)
> > > +             (test-output))"
> > >  cat <<EOF >EXPECTED
> > >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > >  To:
> > > @@ -164,9 +164,9 @@ mkdir -p mail/sent-string/new
> > >  mkdir -p mail/sent-string/tmp
> > >
> > >  test_begin_subtest "notmuch-fcc-dirs set to a string"
> > > -test_emacs "(setq notmuch-fcc-dirs \"sent-string\")
> > > -           (notmuch-mua-mail)
> > > -           (test-output)"
> > > +test_emacs "(let ((notmuch-fcc-dirs \"sent-string\"))
> > > +             (notmuch-mua-mail)
> > > +             (test-output))"
> > >  cat <<EOF >EXPECTED
> > >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > >  To:
> > > @@ -185,11 +185,11 @@ mkdir -p mail/failure/new
> > >  mkdir -p mail/failure/tmp
> > >
> > >  test_begin_subtest "notmuch-fcc-dirs set to a list (with match)"
> > > -test_emacs "(setq notmuch-fcc-dirs
> > > -                 '((\"notmuchmail.org\" . \"sent-list-match\")
> > > -                   (\".*\" . \"failure\")))
> > > -           (notmuch-mua-mail)
> > > -           (test-output)"
> > > +test_emacs "(let ((notmuch-fcc-dirs
> > > +                  '((\"notmuchmail.org\" . \"sent-list-match\")
> > > +                    (\".*\" . \"failure\"))))
> > > +             (notmuch-mua-mail)
> > > +             (test-output))"
> > >  cat <<EOF >EXPECTED
> > >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > >  To:
> > > @@ -205,11 +205,11 @@ mkdir -p mail/sent-list-catch-all/new
> > >  mkdir -p mail/sent-list-catch-all/tmp
> > >
> > >  test_begin_subtest "notmuch-fcc-dirs set to a list (catch-all)"
> > > -test_emacs "(setq notmuch-fcc-dirs
> > > -                 '((\"example.com\" . \"failure\")
> > > -                   (\".*\" . \"sent-list-catch-all\")))
> > > -           (notmuch-mua-mail)
> > > -           (test-output)"
> > > +test_emacs "(let ((notmuch-fcc-dirs
> > > +                  '((\"example.com\" . \"failure\")
> > > +                    (\".*\" . \"sent-list-catch-all\"))))
> > > +             (notmuch-mua-mail)
> > > +             (test-output))"
> > >  cat <<EOF >EXPECTED
> > >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > >  To:
> > > @@ -220,11 +220,11 @@ EOF
> > >  test_expect_equal_file OUTPUT EXPECTED
> > >
> > >  test_begin_subtest "notmuch-fcc-dirs set to a list (no match)"
> > > -test_emacs "(setq notmuch-fcc-dirs
> > > -                 '((\"example.com\" . \"failure\")
> > > -                   (\"nomatchhere.net\" . \"failure\")))
> > > -           (notmuch-mua-mail)
> > > -           (test-output)"
> > > +test_emacs "(let ((notmuch-fcc-dirs
> > > +                  '((\"example.com\" . \"failure\")
> > > +                    (\"nomatchhere.net\" . \"failure\"))))
> > > +             (notmuch-mua-mail)
> > > +             (test-output))"
> > >  cat <<EOF >EXPECTED
> > >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > >  To:
> > > @@ -253,15 +253,15 @@ test_expect_equal_file OUTPUT EXPECTED
> > >
> > >  test_begin_subtest "Save attachment from within emacs using notmuch-show-save-attachments"
> > >  # save as archive to test that Emacs does not re-compress .gz
> > > -echo ./attachment1.gz |
> > > -test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
> > > -           (notmuch-show-save-attachments)' > /dev/null 2>&1
> > > +test_emacs '(let ((standard-input "\"attachment1.gz\""))
> > > +             (notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
> > > +             (notmuch-show-save-attachments))'
> > >  test_expect_equal_file "$EXPECTED/attachment" attachment1.gz
> > >
> > >  test_begin_subtest "Save attachment from within emacs using notmuch-show-save-part"
> > >  # save as archive to test that Emacs does not re-compress .gz
> > > -echo ./attachment2.gz |
> > > -test_emacs '(notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com" 5)' > /dev/null 2>&1
> > > +test_emacs '(let ((standard-input "\"attachment2.gz\""))
> > > +             (notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com" 5))' > /dev/null 2>&1
> > >  test_expect_equal_file "$EXPECTED/attachment" attachment2.gz
> > >
> > >  test_begin_subtest "View raw message within emacs"
> > > diff --git a/test/test-lib.el b/test/test-lib.el
> > > index 4e7f5cf..a5a3125 100644
> > > --- a/test/test-lib.el
> > > +++ b/test/test-lib.el
> > > @@ -23,6 +23,12 @@
> > >  ;; avoid crazy 10-column default of --batch
> > >  (set-frame-width (window-frame (get-buffer-window)) 80)
> > >
> > > +;; `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
> > > +;; `read' call.
> > > +(setq read-file-name-function (lambda (&rest _) (read)))
> > > +
> > >  (defun notmuch-test-wait ()
> > >   "Wait for process completion."
> > >   (while (get-buffer-process (current-buffer))
> > > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > > index ad1506c..1c1581b 100755
> > > --- a/test/test-lib.sh
> > > +++ b/test/test-lib.sh
> > > @@ -57,6 +57,9 @@ unset CDPATH
> > >
> > >  unset GREP_OPTIONS
> > >
> > > +# PID of running Emacs server
> > > +emacs_server_pid=
> > > +
> > >  # Convenience
> > >  #
> > >  # A regexp to match 5 and 40 hexdigits
> > > @@ -174,6 +177,7 @@ test_success=0
> > >
> > >  die () {
> > >        code=$?
> > > +       emacs_server_stop
> > >        if test -n "$GIT_EXIT_OK"
> > >        then
> > >                exit $code
> > > @@ -394,19 +398,20 @@ emacs_deliver_message ()
> > >     mkdir -p "$MAIL_DIR"/sent/{cur,new,tmp}
> > >     ../smtp-dummy sent_message &
> > >     smtp_dummy_pid=$!
> > > -    test_emacs "(setq message-send-mail-function 'message-smtpmail-send-it)
> > > -               (setq smtpmail-smtp-server \"localhost\")
> > > -               (setq smtpmail-smtp-service \"25025\")
> > > -               (notmuch-hello)
> > > -               (notmuch-mua-mail)
> > > -               (message-goto-to)
> > > -               (insert \"test_suite@notmuchmail.org\nDate: 01 Jan 2000 12:00:00 -0000\")
> > > -               (message-goto-subject)
> > > -               (insert \"${subject}\")
> > > -               (message-goto-body)
> > > -               (insert \"${body}\")
> > > -               $@
> > > -               (message-send-and-exit)" >/dev/null 2>&1
> > > +    test_emacs \
> > > +       "(let ((message-send-mail-function 'message-smtpmail-send-it)
> > > +              (smtpmail-smtp-server \"localhost\")
> > > +              (smtpmail-smtp-service \"25025\"))
> > > +          (notmuch-hello)
> > > +          (notmuch-mua-mail)
> > > +          (message-goto-to)
> > > +          (insert \"test_suite@notmuchmail.org\nDate: 01 Jan 2000 12:00:00 -0000\")
> > > +          (message-goto-subject)
> > > +          (insert \"${subject}\")
> > > +          (message-goto-body)
> > > +          (insert \"${body}\")
> > > +          $@
> > > +          (message-send-and-exit))" >/dev/null 2>&1
> > >     wait ${smtp_dummy_pid}
> > >     notmuch new >/dev/null
> > >  }
> > > @@ -828,6 +833,8 @@ test_done () {
> > >
> > >        echo
> > >
> > > +       emacs_server_stop
> > > +
> > >        if [ "$test_failure" = "0" ]; then
> > >            if [ "$test_broken" = "0" ]; then
> > >                rm -rf "$remove_tmp"
> > > @@ -838,24 +845,26 @@ test_done () {
> > >        fi
> > >  }
> > >
> > > -test_emacs () {
> > > -       # Construct a little test script here for the benefit of the user,
> > > -       # (who can easily run "run_emacs" to get the same emacs environment
> > > -       # for investigating any failures).
> > > -       cat <<EOF > run_emacs
> > > +# Generate some scripts for running Emacs tests.  These scripts are
> > > +# used by Emacs tests and help investigating failures.  The following
> > > +# scripts are generated:
> > > +#
> > > +# * emacs_server_start - start Emacs server
> > > +# * emacs_server_stop  - stop Emacs server
> > > +# * emacs_start                - start Emacs
> > > +# * emacs_run          - execute ELisp expressions in running Emacs server
> > > +emacs_generate_scripts ()
> > > +{
> > > +       server_name="notmuch-test-suite-$$"
> > > +
> > > +       cat <<EOF > "$TMP_DIRECTORY/emacs_server_start"
> > >  #!/bin/sh
> > >  export PATH=$PATH
> > >  export NOTMUCH_CONFIG=$NOTMUCH_CONFIG
> > >
> > > -# We assume that the user will give a command-line argument only if
> > > -# wanting to run in batch mode.
> > > -if [ \$# -gt 0 ]; then
> > > -       BATCH=--batch
> > > -fi
> > > -
> > >  # Here's what we are using here:
> > >  #
> > > -# --batch:             Quit after given commands and print all (messages)
> > > +# --daemon             Start Emacs as a daemon
> > >  #
> > >  # --no-init-file       Don't load users ~/.emacs
> > >  #
> > > @@ -865,13 +874,90 @@ fi
> > >  #
> > >  # --load               Force loading of notmuch.el and test-lib.el
> > >
> > > -emacs \$BATCH --no-init-file --no-site-file \
> > > -       --directory ../../emacs --load notmuch.el \
> > > -       --directory .. --load test-lib.el \
> > > -       --eval "(progn \$@)"
> > > +emacs --daemon --no-init-file --no-site-file \
> > > +       --directory "$TEST_DIRECTORY/../emacs" --load notmuch.el \
> > > +       --directory "$TEST_DIRECTORY" --load test-lib.el \
> > > +       --eval '(setq server-name "$server_name")'
> > > +EOF
> > > +       chmod a+x "$TMP_DIRECTORY/emacs_server_start"
> > > +
> > > +       cat <<EOF > "$TMP_DIRECTORY/emacs_server_stop"
> > > +#!/bin/sh
> > > +
> > > +dir=\$(dirname "\$0")
> > > +"\$dir"/emacs_run '(kill-emacs)'
> > >  EOF
> > > -       chmod a+x ./run_emacs
> > > -       ./run_emacs "$@"
> > > +       chmod a+x "$TMP_DIRECTORY/emacs_server_stop"
> > > +
> > > +       cat <<EOF > "$TMP_DIRECTORY/emacs_start"
> > > +#!/bin/sh
> > > +export PATH=$PATH
> > > +export NOTMUCH_CONFIG=$NOTMUCH_CONFIG
> > > +
> > > +# Here's what we are using here:
> > > +#
> > > +# --no-init-file       Don't load users ~/.emacs
> > > +#
> > > +# --no-site-file       Don't load the site-wide startup stuff
> > > +#
> > > +# --directory          Ensure that the local elisp sources are found
> > > +#
> > > +# --load               Force loading of notmuch.el and test-lib.el
> > > +
> > > +emacs --no-init-file --no-site-file \
> > > +       --directory "$TEST_DIRECTORY/../emacs" --load notmuch.el \
> > > +       --directory "$TEST_DIRECTORY" --load test-lib.el
> > > +EOF
> > > +       chmod a+x "$TMP_DIRECTORY/emacs_start"
> > > +
> > > +       cat <<EOF > "$TMP_DIRECTORY/emacs_run"
> > > +#!/bin/sh
> > > +
> > > +# Here's what we are using here:
> > > +#
> > > +# --socket-name                Emacs server name
> > > +#
> > > +# --eval               Evaluate ELisp expressions
> > > +
> > > +emacsclient --socket-name "$server_name" --eval "(progn \$@)"
> > > +EOF
> > > +       chmod a+x "$TMP_DIRECTORY/emacs_run"
> > > +}
> > > +
> > > +# Start Emacs server if it is not running.
> > > +emacs_server_start ()
> > > +{
> > > +       [ -n "$emacs_server_pid" ] && return
> > > +
> > > +       output=$("$TMP_DIRECTORY/emacs_server_start" 2>&1)
> > > +       if [ "$?" -ne 0 ]; then
> > > +               echo "$output"
> > > +               return 1
> > > +       fi
> > > +
> > > +       emacs_server_pid=$("$TMP_DIRECTORY/emacs_run" '(emacs-pid)')
> > > +       [ "$?" -eq 0 -a -n "$emacs_server_pid" ]
> > > +}
> > > +
> > > +# Stop Emacs server if it is running.
> > > +emacs_server_stop ()
> > > +{
> > > +       [ -z "$emacs_server_pid" ] && return
> > > +
> > > +       emacs_server_pid=
> > > +       output=$("$TMP_DIRECTORY/emacs_server_stop" 2>&1)
> > > +       if [ "$?" -ne 0 ]; then
> > > +               echo "$output"
> > > +               return 1
> > > +       fi
> > > +}
> > > +
> > > +# Evaluate ELisp expressions in Emacs server.  Server is started if it
> > > +# is not running.
> > > +test_emacs () {
> > > +       emacs_server_start || return
> > > +
> > > +       "$TMP_DIRECTORY/emacs_run" "$@"
> > >  }
> > >
> > >
> > > @@ -999,6 +1085,7 @@ primary_email=test_suite@notmuchmail.org
> > >  other_email=test_suite_other@notmuchmail.org;test_suite@otherdomain.org
> > >  EOF
> > >
> > > +emacs_generate_scripts
> > >
> > >  # Use -P to resolve symlinks in our working directory so that the cwd
> > >  # in subprocesses like git equals our $PWD (for pathname comparisons).
> > > --
> > > 1.7.5.4
> > >
> > > _______________________________________________
> > > notmuch mailing list
> > > notmuch@notmuchmail.org
> > > http://notmuchmail.org/mailman/listinfo/notmuch
> > >

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-28  1:03         ` Dmitry Kurochkin
@ 2011-06-28  3:49           ` Austin Clements
  2011-06-28  3:59             ` Dmitry Kurochkin
  2011-06-28 16:22             ` Austin Clements
  0 siblings, 2 replies; 22+ messages in thread
From: Austin Clements @ 2011-06-28  3:49 UTC (permalink / raw)
  To: Dmitry Kurochkin; +Cc: notmuch

Quoth Dmitry Kurochkin on Jun 28 at  5:03 am:
> > >  The only way I know to
> > > reliably kill a child process is to open a pipe to it and have it exit
> > > on its own when it reads EOF.  Unfortunately, I couldn't find a way to
> > > do this with an emacs daemon (it appears daemon mode aggressively
> > > cleans up things like pipes), but here's a different approach:
> > > 
> > >     coproc emacs --batch --eval "(while t (eval (read)))"
> > >     EMACSFD=${COPROC[1]}
> > >     trap "echo '(kill-emacs)' >&$EMACSFD" EXIT
> > > 
> > >     echo '(message "Hi")' >&$EMACSFD
> > >     # ...
> > > 
> > > This is, basically, a poor man's emacs server, but the coprocess pipe
> > > binds it tightly to the shell.  If the shell exits for *any* reason,
> > > the pipe will be closed by the kernel, emacs will read an EOF, and
> > > exit.
> > 
> > I like this idea.
> > 
> > >  The trap is there just to cleanly shut down in case of a normal
> > > exit [1].
> > 
> > For normal exit we should just put this into test_done.  Otherwise it is
> > not a normal exit and we do not care about Emacs error message.  No?
> > 
> > >  This also has the advantage that read-from-minibuffer still
> > > works:
> > > 
> > >     echo '(message (read-from-minibuffer ""))' >&$EMACSFD
> > >     echo 'Test' >&$EMACSFD
> > > 
> > > Thoughts?
> > > 
> > 
> > I like it and I will implement it.  Thanks for the idea.
> > 
> 
> While implementing the idea, I stumbled upon a problem: we need to know
> when Emacs finished what we echoed or failed with an error.  At the
> moment tests fail because they check for OUTPUT before Emacs creates it.
> 
> We can tell Emacs to print some special marker and wait for it.  But
> there may be exceptions and errors which may make it difficult.  I did
> not found a good solution yet.  Would love to hear your thoughts :)

Oof, yes, of course.  How about making the one-line poor man's emacs
server slightly less poor?  Use a FIFO to communicate completion.
Something like,

EMACSDONE=$TEST_DIRECTORY/emacsdone
mkfifo $EMACSDONE
coproc emacs --batch --eval '(while t (eval (read)) (write-region "\n" nil "'$EMACSDONE'" t 0))'
EMACSFD=${COPROC[1]}

test_emacs() {
    echo "$1" >&$EMACSFD
    read <$EMACSDONE
}

test_emacs '(sleep-for 2)'
test_emacs '(message "Hi")'

echo '(kill-emacs)' >&$EMACSFD

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-28  3:49           ` Austin Clements
@ 2011-06-28  3:59             ` Dmitry Kurochkin
  2011-06-28  4:17               ` Austin Clements
  2011-06-28 16:22             ` Austin Clements
  1 sibling, 1 reply; 22+ messages in thread
From: Dmitry Kurochkin @ 2011-06-28  3:59 UTC (permalink / raw)
  To: Austin Clements; +Cc: notmuch

On Mon, 27 Jun 2011 23:49:37 -0400, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Dmitry Kurochkin on Jun 28 at  5:03 am:
> > > >  The only way I know to
> > > > reliably kill a child process is to open a pipe to it and have it exit
> > > > on its own when it reads EOF.  Unfortunately, I couldn't find a way to
> > > > do this with an emacs daemon (it appears daemon mode aggressively
> > > > cleans up things like pipes), but here's a different approach:
> > > > 
> > > >     coproc emacs --batch --eval "(while t (eval (read)))"
> > > >     EMACSFD=${COPROC[1]}
> > > >     trap "echo '(kill-emacs)' >&$EMACSFD" EXIT
> > > > 
> > > >     echo '(message "Hi")' >&$EMACSFD
> > > >     # ...
> > > > 
> > > > This is, basically, a poor man's emacs server, but the coprocess pipe
> > > > binds it tightly to the shell.  If the shell exits for *any* reason,
> > > > the pipe will be closed by the kernel, emacs will read an EOF, and
> > > > exit.
> > > 
> > > I like this idea.
> > > 
> > > >  The trap is there just to cleanly shut down in case of a normal
> > > > exit [1].
> > > 
> > > For normal exit we should just put this into test_done.  Otherwise it is
> > > not a normal exit and we do not care about Emacs error message.  No?
> > > 
> > > >  This also has the advantage that read-from-minibuffer still
> > > > works:
> > > > 
> > > >     echo '(message (read-from-minibuffer ""))' >&$EMACSFD
> > > >     echo 'Test' >&$EMACSFD
> > > > 
> > > > Thoughts?
> > > > 
> > > 
> > > I like it and I will implement it.  Thanks for the idea.
> > > 
> > 
> > While implementing the idea, I stumbled upon a problem: we need to know
> > when Emacs finished what we echoed or failed with an error.  At the
> > moment tests fail because they check for OUTPUT before Emacs creates it.
> > 
> > We can tell Emacs to print some special marker and wait for it.  But
> > there may be exceptions and errors which may make it difficult.  I did
> > not found a good solution yet.  Would love to hear your thoughts :)
> 
> Oof, yes, of course.  How about making the one-line poor man's emacs
> server slightly less poor?  Use a FIFO to communicate completion.
> Something like,
> 
> EMACSDONE=$TEST_DIRECTORY/emacsdone
> mkfifo $EMACSDONE
> coproc emacs --batch --eval '(while t (eval (read)) (write-region "\n" nil "'$EMACSDONE'" t 0))'
> EMACSFD=${COPROC[1]}
> 
> test_emacs() {
>     echo "$1" >&$EMACSFD
>     read <$EMACSDONE
> }
> 
> test_emacs '(sleep-for 2)'
> test_emacs '(message "Hi")'
> 
> echo '(kill-emacs)' >&$EMACSFD

I am sure that would work, but I do not like the complexity.  How about
getting back to standard emacsclient and running a watchdog in the
emacs?  Like:

(defun orphan-watchdog (pid)
  "Periodically check that the process with id PID is still
running, quit if it terminated."
  (if (not (process-attributes pid))
      (kill-emacs)
    (run-at-time "1 min" nil orphan-watchdog pid)))

This function (or my other changes) do not work (by yet unknown reason
:)), but you get the idea.

Regards,
  Dmitry

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-28  3:59             ` Dmitry Kurochkin
@ 2011-06-28  4:17               ` Austin Clements
  2011-06-28  4:44                 ` Dmitry Kurochkin
  0 siblings, 1 reply; 22+ messages in thread
From: Austin Clements @ 2011-06-28  4:17 UTC (permalink / raw)
  To: Dmitry Kurochkin; +Cc: notmuch

Quoth Dmitry Kurochkin on Jun 28 at  7:59 am:
> I am sure that would work, but I do not like the complexity.  How about
> getting back to standard emacsclient and running a watchdog in the
> emacs?  Like:
> 
> (defun orphan-watchdog (pid)
>   "Periodically check that the process with id PID is still
> running, quit if it terminated."
>   (if (not (process-attributes pid))
>       (kill-emacs)
>     (run-at-time "1 min" nil orphan-watchdog pid)))
> 
> This function (or my other changes) do not work (by yet unknown reason
> :)), but you get the idea.

I would consider this more complex than a few file descriptors. ]:--8)
Though, I'm automatically distrustful of anything that relies on
polling (why poll when you can be notified instantly?).

It also has some problems.  For example, PID's are easily reused, so
if another process happens to take up that PID, the emacs could still
hang around for a long time.

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-28  4:17               ` Austin Clements
@ 2011-06-28  4:44                 ` Dmitry Kurochkin
  0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Kurochkin @ 2011-06-28  4:44 UTC (permalink / raw)
  To: Austin Clements; +Cc: notmuch

On Tue, 28 Jun 2011 00:17:42 -0400, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Dmitry Kurochkin on Jun 28 at  7:59 am:
> > I am sure that would work, but I do not like the complexity.  How about
> > getting back to standard emacsclient and running a watchdog in the
> > emacs?  Like:
> > 
> > (defun orphan-watchdog (pid)
> >   "Periodically check that the process with id PID is still
> > running, quit if it terminated."
> >   (if (not (process-attributes pid))
> >       (kill-emacs)
> >     (run-at-time "1 min" nil orphan-watchdog pid)))
> > 
> > This function (or my other changes) do not work (by yet unknown reason
> > :)), but you get the idea.
> 
> I would consider this more complex than a few file descriptors. ]:--8)

More shell code and more elisp code.  I do not think can be considered
simpler :)

> Though, I'm automatically distrustful of anything that relies on
> polling (why poll when you can be notified instantly?).
> 

I agree that polling is not as elegant as an instant notification.  But
IMO reinventing emacsclient just kills all the beauty of this solution.
I liked it when it was a simple read loop, but now it is too complex to
my taste.  Besides, I am a bit worried that we will face new problems in
the future that would force us to add more "features" to our
not-so-poor-man's Emacs server implementation.

> It also has some problems.  For example, PID's are easily reused, so
> if another process happens to take up that PID, the emacs could still
> hang around for a long time.

Indeed.  We may add a more complex process detection, but IMO it is an
overkill.

Anyway, I am done with reworking the patch series and will post it now.
Perhaps others would voice their opinion on this one.

Regards,
  Dmitry

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-28  3:49           ` Austin Clements
  2011-06-28  3:59             ` Dmitry Kurochkin
@ 2011-06-28 16:22             ` Austin Clements
  2011-06-28 16:42               ` Dmitry Kurochkin
  1 sibling, 1 reply; 22+ messages in thread
From: Austin Clements @ 2011-06-28 16:22 UTC (permalink / raw)
  To: Dmitry Kurochkin; +Cc: notmuch

Quoth myself on Jun 27 at 11:49 pm:
> Quoth Dmitry Kurochkin on Jun 28 at  5:03 am:
> EMACSDONE=$TEST_DIRECTORY/emacsdone
> mkfifo $EMACSDONE
> coproc emacs --batch --eval '(while t (eval (read)) (write-region "\n" nil "'$EMACSDONE'" t 0))'
> EMACSFD=${COPROC[1]}
> 
> test_emacs() {
>     echo "$1" >&$EMACSFD
>     read <$EMACSDONE
> }
> 
> test_emacs '(sleep-for 2)'
> test_emacs '(message "Hi")'
> 
> echo '(kill-emacs)' >&$EMACSFD

Oops, got a little overzealous with TEST_DIRECTORY.  For reference,
the pipe should, of course, have gone in the current directory (or
TMP_DIRECTORY).

mkfifo emacsdone
coproc emacs --batch --eval '(while t (eval (read)) (write-region "\n" nil "emacsdone" t 0))'
EMACSFD=${COPROC[1]}

test_emacs() {
    echo "$1" >&$EMACSFD
    read < emacsdone
}


(I don't really see how that could be either more shell code or more
elisp code than using emacsclient plus cleanup code [nor why it
matters for five lines of code], but I'm probably missing something.)

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-28 16:22             ` Austin Clements
@ 2011-06-28 16:42               ` Dmitry Kurochkin
  2011-06-28 20:10                 ` Carl Worth
  0 siblings, 1 reply; 22+ messages in thread
From: Dmitry Kurochkin @ 2011-06-28 16:42 UTC (permalink / raw)
  To: Austin Clements; +Cc: notmuch

On Tue, 28 Jun 2011 12:22:57 -0400, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth myself on Jun 27 at 11:49 pm:
> > Quoth Dmitry Kurochkin on Jun 28 at  5:03 am:
> > EMACSDONE=$TEST_DIRECTORY/emacsdone
> > mkfifo $EMACSDONE
> > coproc emacs --batch --eval '(while t (eval (read)) (write-region "\n" nil "'$EMACSDONE'" t 0))'
> > EMACSFD=${COPROC[1]}
> > 
> > test_emacs() {
> >     echo "$1" >&$EMACSFD
> >     read <$EMACSDONE
> > }
> > 
> > test_emacs '(sleep-for 2)'
> > test_emacs '(message "Hi")'
> > 
> > echo '(kill-emacs)' >&$EMACSFD
> 
> Oops, got a little overzealous with TEST_DIRECTORY.  For reference,
> the pipe should, of course, have gone in the current directory (or
> TMP_DIRECTORY).
> 
> mkfifo emacsdone
> coproc emacs --batch --eval '(while t (eval (read)) (write-region "\n" nil "emacsdone" t 0))'
> EMACSFD=${COPROC[1]}
> 
> test_emacs() {
>     echo "$1" >&$EMACSFD
>     read < emacsdone
> }
> 
> 
> (I don't really see how that could be either more shell code or more
> elisp code than using emacsclient plus cleanup code [nor why it
> matters for five lines of code], but I'm probably missing something.)

Well, it may not be less code if you count lines.  It is just IMHO, but
it shell code in case of emacsclient is simpler (no fifo, no coproc),
cleanup is the same (one line with kill-emacs).  More lisp code in
emacsclient variant, but that is just because you put it all on one line
:)

Code size does not matter indeed.  Your approach just feels more of a
hack to me and unnecessary complex.  Again, this is just my IMHO.

I would like to hear what other (Carl in particular) think about this.
If the consensus is for your approach, I would be happy to implement it.

Regards,
  Dmitry

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-28 16:42               ` Dmitry Kurochkin
@ 2011-06-28 20:10                 ` Carl Worth
  2011-06-28 20:47                   ` Dmitry Kurochkin
  2011-06-28 20:58                   ` Dmitry Kurochkin
  0 siblings, 2 replies; 22+ messages in thread
From: Carl Worth @ 2011-06-28 20:10 UTC (permalink / raw)
  To: Dmitry Kurochkin, Austin Clements; +Cc: notmuch

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

On Tue, 28 Jun 2011 20:42:42 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> I would like to hear what other (Carl in particular) think about this.
> If the consensus is for your approach, I would be happy to implement
> it.

In general, I love the whole series, thanks! I'm looking forward to our
future, faster test suite.

Even more, I love the constructive dialog that follows the original
series and the attention being focused on getting things right.

As for the detail of whether to use emacsclient or Austin's look-alike,
I don't have a strong attachment to either solution. I do appreciate
concrete technical things like "robust against recycled PIDs", "more
robust against leaving daemon's around for some reason", etc.

Would any of this potentially interfere with my own usage of emacsclient
and emacs server? I use them regularly and would be quite surprised (and
likely frustrated) if the test suite got mixed up with my existing emacs
server (or the other way around). Maybe that's already taken care of
with either approach? (A quick skim of the emacsclient manual age didn't
make it obvious to me how emacslcient finds its server.)

-Carl

-- 
carl.d.worth@intel.com

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

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-28 20:10                 ` Carl Worth
@ 2011-06-28 20:47                   ` Dmitry Kurochkin
  2011-06-28 22:41                     ` Carl Worth
  2011-06-28 20:58                   ` Dmitry Kurochkin
  1 sibling, 1 reply; 22+ messages in thread
From: Dmitry Kurochkin @ 2011-06-28 20:47 UTC (permalink / raw)
  To: Carl Worth, Austin Clements; +Cc: notmuch

On Tue, 28 Jun 2011 13:10:58 -0700, Carl Worth <cworth@cworth.org> wrote:
> On Tue, 28 Jun 2011 20:42:42 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > I would like to hear what other (Carl in particular) think about this.
> > If the consensus is for your approach, I would be happy to implement
> > it.
> 
> In general, I love the whole series, thanks! I'm looking forward to our
> future, faster test suite.
> 
> Even more, I love the constructive dialog that follows the original
> series and the attention being focused on getting things right.
> 
> As for the detail of whether to use emacsclient or Austin's look-alike,
> I don't have a strong attachment to either solution. I do appreciate
> concrete technical things like "robust against recycled PIDs",

This is an issue indeed.  We can take more care to better identify the
process.  But I think this is not really needed.  I am fine with leaving
a small (very tiny, really :)) possibility for leaving Emacs running for
longer than needed.  Note this only happens if test is terminated
abnormally.

> "more
> robust against leaving daemon's around for some reason", etc.
> 

Not sure I agree with this.  The watchdog is not perfect, but not less
reliable.  It reacts with a delay, but again I do not think this is so
important.

I think I found an issue with Austin's approach: if an error happens
(and is not catched), the loop would stop.  Currently, this would result
in non-working tests and also would leave the Emacs process behind.  I
guess we will have to add error catching in the loop.  Austin?

> Would any of this potentially interfere with my own usage of emacsclient
> and emacs server? I use them regularly and would be quite surprised (and
> likely frustrated) if the test suite got mixed up with my existing emacs
> server (or the other way around). Maybe that's already taken care of
> with either approach? (A quick skim of the emacsclient manual age didn't
> make it obvious to me how emacslcient finds its server.)
> 

Emacs uses `server-name' and emacsclient --socket-name to identify the
server.  Tests use "notmuch-test-suite-<TEST_SHELL_PID>" server name.
While it is not entirely impossible to mess your existing Emacs server,
it is unlikely enough IMO.

Regards,
  Dmitry

> -Carl
> 
> -- 
> carl.d.worth@intel.com

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-28 20:10                 ` Carl Worth
  2011-06-28 20:47                   ` Dmitry Kurochkin
@ 2011-06-28 20:58                   ` Dmitry Kurochkin
  2011-06-28 22:42                     ` Carl Worth
  1 sibling, 1 reply; 22+ messages in thread
From: Dmitry Kurochkin @ 2011-06-28 20:58 UTC (permalink / raw)
  To: Carl Worth; +Cc: notmuch, Austin Clements

On Tue, 28 Jun 2011 13:10:58 -0700, Carl Worth <cworth@cworth.org> wrote:
Non-text part: multipart/signed
> On Tue, 28 Jun 2011 20:42:42 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > I would like to hear what other (Carl in particular) think about this.
> > If the consensus is for your approach, I would be happy to implement
> > it.
> 
> In general, I love the whole series, thanks! I'm looking forward to our
> future, faster test suite.
> 

BTW Carl, while we continue our debate, you may consider applying the
first 9 patches from the series :)

id:"1309236311-2162-1-git-send-email-dmitry.kurochkin@gmail.com"

Regards,
  Dmitry

> Even more, I love the constructive dialog that follows the original
> series and the attention being focused on getting things right.
> 
> As for the detail of whether to use emacsclient or Austin's look-alike,
> I don't have a strong attachment to either solution. I do appreciate
> concrete technical things like "robust against recycled PIDs", "more
> robust against leaving daemon's around for some reason", etc.
> 
> Would any of this potentially interfere with my own usage of emacsclient
> and emacs server? I use them regularly and would be quite surprised (and
> likely frustrated) if the test suite got mixed up with my existing emacs
> server (or the other way around). Maybe that's already taken care of
> with either approach? (A quick skim of the emacsclient manual age didn't
> make it obvious to me how emacslcient finds its server.)
> 
> -Carl
> 
> -- 
> carl.d.worth@intel.com
Non-text part: application/pgp-signature

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-28 20:47                   ` Dmitry Kurochkin
@ 2011-06-28 22:41                     ` Carl Worth
  0 siblings, 0 replies; 22+ messages in thread
From: Carl Worth @ 2011-06-28 22:41 UTC (permalink / raw)
  To: Dmitry Kurochkin, Austin Clements; +Cc: notmuch

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

On Wed, 29 Jun 2011 00:47:37 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > "more robust against leaving daemon's around for some reason", etc.
> 
> Not sure I agree with this.

I'm sorry. I wasn't clear.

I wasn't advocating one solution over the other. I was just giving an
example of the kind of technical merits on which I would like to see the
approaches evaluated.

I care about the robustness of the approach here. I'm less inclined to
care about the "messiness" of the implementation (however that is
measured).

If both approaches are equally robust, then I'll take your word for it,
and that's great.

-Carl

-- 
carl.d.worth@intel.com

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

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] test: use emacsclient(1) for Emacs tests
  2011-06-28 20:58                   ` Dmitry Kurochkin
@ 2011-06-28 22:42                     ` Carl Worth
  0 siblings, 0 replies; 22+ messages in thread
From: Carl Worth @ 2011-06-28 22:42 UTC (permalink / raw)
  To: Dmitry Kurochkin; +Cc: notmuch, Austin Clements

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

On Wed, 29 Jun 2011 00:58:41 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> BTW Carl, while we continue our debate, you may consider applying the
> first 9 patches from the series :)

Yes, I already did that.

And I would have even installed all 10 (with the debate still going)
except that I'm seeing tests fail, (as mentioned in the other thread).

-Carl

-- 
carl.d.worth@intel.com

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

^ permalink raw reply	[flat|nested] 22+ messages in thread
end of thread, other threads:[~2011-06-28 22:42 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-26 23:52 [PATCH 1/5] test: do not set `message-signature' in test_emacs Dmitry Kurochkin
2011-06-26 23:52 ` [PATCH 2/5] test: cleanup test_emacs Dmitry Kurochkin
2011-06-26 23:52 ` [PATCH 3/5] test: wrap and indent test_emacs calls Dmitry Kurochkin
2011-06-26 23:52 ` [PATCH 4/5] test: save buffer content to file instead of printing it in Emacs tests Dmitry Kurochkin
2011-06-26 23:52 ` [PATCH 5/5] test: remove some sed(1) calls " Dmitry Kurochkin
2011-06-27  3:54 ` [PATCH 1/2] test: use emacs_deliver_message in Emacs SMTP send test Dmitry Kurochkin
2011-06-27  3:54   ` [PATCH 2/2] test: use emacsclient(1) for Emacs tests Dmitry Kurochkin
2011-06-27 20:02     ` Austin Clements
2011-06-27 20:22       ` Dmitry Kurochkin
2011-06-27 20:32         ` Austin Clements
2011-06-28  1:03         ` Dmitry Kurochkin
2011-06-28  3:49           ` Austin Clements
2011-06-28  3:59             ` Dmitry Kurochkin
2011-06-28  4:17               ` Austin Clements
2011-06-28  4:44                 ` Dmitry Kurochkin
2011-06-28 16:22             ` Austin Clements
2011-06-28 16:42               ` Dmitry Kurochkin
2011-06-28 20:10                 ` Carl Worth
2011-06-28 20:47                   ` Dmitry Kurochkin
2011-06-28 22:41                     ` Carl Worth
2011-06-28 20:58                   ` Dmitry Kurochkin
2011-06-28 22:42                     ` Carl Worth

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).