[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