[PATCH] test: test-lib.el: replace sleep-for with sit-for in notmuch-test-wait

[PATCH] test: test-lib.el: replace sleep-for with sit-for in notmuch-test-wait

[Tomi Ollila]

The function `notmuch-test-wait` called `get-buffer-process` and
`sleep-for` in a loop. On some emacses neither of these cause emacs
to check whether the process has exited and update it's status
accordingly. In this case the loop does not exit.

The function `sit-for` goes into event loop via `read-event` function
call. `read-event` does not return when process exits but the event
loop used to determine whether there is keyboard, mouse, etc. event
updates the process status as a side effect of the (more generic)
event loop. `sit-for` is used here to restore the event into queue
in the improbable case `read-event` consumes an event.
---
 test/test-lib.el |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index 5dd6271..d14246a 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -38,7 +38,8 @@
 (defun notmuch-test-wait ()
   "Wait for process completion."
   (while (get-buffer-process (current-buffer))
-    (sleep-for 0.1)))
+    ;; sit-for visits event loop for process exit notification.
+    (sit-for 0.1)))
 
 (defun test-output (&optional filename)
   "Save current buffer to file FILENAME.  Default FILENAME is OUTPUT."
-- 
1.7.1

Re: [PATCH] test: test-lib.el: replace sleep-for with sit-for in notmuch-test-wait

[Austin Clements]

Code LGTM, but maybe the comment should explain that this is a
workaround for a bug in Emacs, given that both sleep-for and sit-for
are supposed to process process output and run sentinels (which has
been stated in the Elisp reference at least as far back as Emacs 21).
In light of this (and our better understanding of sleep-for and
sit-for from IRC discussions), the commit message is also misleading.

Quoth Tomi Ollila on Aug 03 at  3:16 pm:
> The function `notmuch-test-wait` called `get-buffer-process` and
> `sleep-for` in a loop. On some emacses neither of these cause emacs
> to check whether the process has exited and update it's status
> accordingly. In this case the loop does not exit.
> 
> The function `sit-for` goes into event loop via `read-event` function
> call. `read-event` does not return when process exits but the event
> loop used to determine whether there is keyboard, mouse, etc. event
> updates the process status as a side effect of the (more generic)
> event loop. `sit-for` is used here to restore the event into queue
> in the improbable case `read-event` consumes an event.
> ---
>  test/test-lib.el |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/test/test-lib.el b/test/test-lib.el
> index 5dd6271..d14246a 100644
> --- a/test/test-lib.el
> +++ b/test/test-lib.el
> @@ -38,7 +38,8 @@
>  (defun notmuch-test-wait ()
>    "Wait for process completion."
>    (while (get-buffer-process (current-buffer))
> -    (sleep-for 0.1)))
> +    ;; sit-for visits event loop for process exit notification.
> +    (sit-for 0.1)))
>  
>  (defun test-output (&optional filename)
>    "Save current buffer to file FILENAME.  Default FILENAME is OUTPUT."

Re: [PATCH] test: test-lib.el: replace sleep-for with sit-for in notmuch-test-wait

[Tomi Ollila]

On Fri, Aug 03 2012, Austin Clements <amdragon@MIT.EDU> wrote:

> Code LGTM, but maybe the comment should explain that this is a
> workaround for a bug in Emacs, given that both sleep-for and sit-for
> are supposed to process process output and run sentinels (which has
> been stated in the Elisp reference at least as far back as Emacs 21).
> In light of this (and our better understanding of sleep-for and
> sit-for from IRC discussions), the commit message is also misleading.

Indeed...

Tomi

[PATCH v2] test: test-lib.el: replace sleep-for with sit-for in notmuch-test-wait

[Tomi Ollila]

When running emacs tests using emacs 23.1.1 the tests block (until timeout)
when emacs function (notmuch-test-wait) is called.

There is an emacs bug #2930 titled:
23.0.92; `accept-process-output' and `sleep-for' do not run sentinels

It seems this is still active in emacs 23.1; replacing sleep-for with
sit-for makes the tests work as expected.

The other function in loop executed in notmuch-test-wait: get-buffer-process
just loops through process list, returning matching process object (or nil
if no such process found) without doing any further processing.

By comparing Emacs 23.1 and 23.2 function wait_reading_process_output ()
(changes in do_display variable usage) it seems that this bug has been
fixed in 23.2.
---
 test/test-lib.el |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index 5dd6271..30d6eb7 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -38,7 +38,11 @@
 (defun notmuch-test-wait ()
   "Wait for process completion."
   (while (get-buffer-process (current-buffer))
-    (sleep-for 0.1)))
+    ;; It seems in Emacs 23.1 `accept-process-output' and `sleep-for' do not
+    ;; run sentinels (bug#2930 (bug-gnu-emacs)). `sit-for' works as documented.
+    ;; `sleep-for` may already work in Emacs 23.2 as function
+    ;; wait_reading_process_output() in src/process.c has related change.
+    (sit-for 0.1)))
 
 (defun test-output (&optional filename)
   "Save current buffer to file FILENAME.  Default FILENAME is OUTPUT."
-- 
1.7.1