[PATCH 2/2] test: use emacsclient(1) for Emacs tests
Dmitry Kurochkin
dmitry.kurochkin at gmail.com
Mon Jun 27 13:22:41 PDT 2011
On Mon, 27 Jun 2011 16:02:12 -0400, Austin Clements <amdragon at 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 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