[PATCH 2/2] test: use emacsclient(1) for Emacs tests
Austin Clements
amdragon at mit.edu
Mon Jun 27 13:02:12 PDT 2011
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 at 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 at 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 at 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 at 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 at 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 at 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 at mail.gmail.com")
> - (notmuch-show-save-attachments)' > /dev/null 2>&1
> +test_emacs '(let ((standard-input "\"attachment1.gz\""))
> + (notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a at 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 at mail.gmail.com" 5)' > /dev/null 2>&1
> +test_emacs '(let ((standard-input "\"attachment2.gz\""))
> + (notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a at 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 at 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 at 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 at notmuchmail.org
> other_email=test_suite_other at notmuchmail.org;test_suite at 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 at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
>
More information about the notmuch
mailing list