[PATCH 1/2] test: emacs: call accept-process-output in notmuch-test-wait

[PATCH 1/2] test: emacs: call accept-process-output in notmuch-test-wait

[Tomi Ollila]

notmuch-test-wait called sleep-for in a loop to wait unconditionally 0.1
seconds while waiting for process to exit.
accept-process-output returns as soon as there is any data available
from process, so using it avoids unnecessary fixed delays.
Both of these functions run process sentinels.
---

This 2 patch series is an alternative to 
id:"1344122222-14344-1-git-send-email-tomi.ollila@iki.fi"
which speeds up execution when Emacs version is not 23.1.
(so that users of newer emacs doesn't need to suffer the
workaround made for emacs 23.1 users in second patch in
this series)

during testing of the feature I had
+    (accept-process-output nil 10)))
there -- it did not wait 10 seconds.

I also tested the following function:

(defun notmuch-test-wait () t)

i.e. dropping wait altogether -- this makes tests fail in different ways...

 test/test-lib.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index 5dd6271..52d9936 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -38,7 +38,7 @@
 (defun notmuch-test-wait ()
   "Wait for process completion."
   (while (get-buffer-process (current-buffer))
-    (sleep-for 0.1)))
+    (accept-process-output nil 0.1)))
 
 (defun test-output (&optional filename)
   "Save current buffer to file FILENAME.  Default FILENAME is OUTPUT."
-- 
1.7.1

[PATCH 2/2] test: emacs: run list-processes after accept-process-output in emacs 23.1

[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 sentinel

It seems this is present in emacs 23.1.

Calling list-processes after accept-process-output seems work around
this problem; in case Emacs version is 23.1 a defadvice is activated
to do just that.
---

Thanks to Austin for the comments and IRC discussions on the matter.

 test/test-lib.el |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index 52d9936..4330352 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -35,6 +35,16 @@
     "Disable yes-or-no-p before executing kill-emacs"
     (defun yes-or-no-p (prompt) t)))
 
+;; Emacs bug #2930:
+;;	23.0.92; `accept-process-output' and `sleep-for' do not run sentinels
+;; seems to be present in Emacs 23.1.
+;; Running `list-processes' after `accept-process-output' seems to work
+;; around this problem.
+(if (and (= emacs-major-version 23) (= emacs-minor-version 1))
+  (defadvice accept-process-output (after run-list-processes activate)
+    "run list-processes after executing accept-process-output"
+    (list-processes)))
+
 (defun notmuch-test-wait ()
   "Wait for process completion."
   (while (get-buffer-process (current-buffer))
-- 
1.7.1

Re: [PATCH 2/2] test: emacs: run list-processes after accept-process-output in emacs 23.1

[Austin Clements]

Quoth Tomi Ollila on Aug 05 at  2:13 pm:
> 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 sentinel
> 
> It seems this is present in emacs 23.1.
> 
> Calling list-processes after accept-process-output seems work around
> this problem; in case Emacs version is 23.1 a defadvice is activated
> to do just that.

Should this workaround perhaps go in notmuch-test-wait directly,
instead of being implemented as advice?  If we do want to keep it as
advice, should it go in notmuch-lib.el along with the few other
compatibility functions?

> ---
> 
> Thanks to Austin for the comments and IRC discussions on the matter.
> 
>  test/test-lib.el |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/test/test-lib.el b/test/test-lib.el
> index 52d9936..4330352 100644
> --- a/test/test-lib.el
> +++ b/test/test-lib.el
> @@ -35,6 +35,16 @@
>      "Disable yes-or-no-p before executing kill-emacs"
>      (defun yes-or-no-p (prompt) t)))
>  
> +;; Emacs bug #2930:
> +;;	23.0.92; `accept-process-output' and `sleep-for' do not run sentinels
> +;; seems to be present in Emacs 23.1.
> +;; Running `list-processes' after `accept-process-output' seems to work
> +;; around this problem.
> +(if (and (= emacs-major-version 23) (= emacs-minor-version 1))
> +  (defadvice accept-process-output (after run-list-processes activate)
> +    "run list-processes after executing accept-process-output"
> +    (list-processes)))
> +
>  (defun notmuch-test-wait ()
>    "Wait for process completion."
>    (while (get-buffer-process (current-buffer))

Re: [PATCH 2/2] test: emacs: run list-processes after accept-process-output in emacs 23.1

[Tomi Ollila]

On Mon, Aug 13 2012, Austin Clements <amdragon@MIT.EDU> wrote:

> Quoth Tomi Ollila on Aug 05 at  2:13 pm:
>> 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 sentinel
>> 
>> It seems this is present in emacs 23.1.
>> 
>> Calling list-processes after accept-process-output seems work around
>> this problem; in case Emacs version is 23.1 a defadvice is activated
>> to do just that.
>
> Should this workaround perhaps go in notmuch-test-wait directly,
> instead of being implemented as advice?  

This way the fact that this is workaround for bug that exists only in
23.1(*) is emphasized; The notmuch-test-wait can exist in a (simpler)
format where it doesn't need to know about this bug.

(*) This bug probably exists in emacs 22 but the MUA has not worked
on emacs 22 at least for a year now.

> If we do want to keep it as advice, should it go in notmuch-lib.el
> along with the few other compatibility functions?

In tests we have fixed environment where we can write workarounds
as "global" advices. If we did this in notmuch-lib.el we'd be changing
users' environment, possibly causing surprises...

... but neither sleep-for or accept-process-output are used in 
any of the notmuch elisp code so such "fixes" are not needed now...

... and this would ever be a problem, we might already support only
emacs 24 or newer >;) ...

Tomi

>> ---
>> 
>> Thanks to Austin for the comments and IRC discussions on the matter.
>> 
>>  test/test-lib.el |    8 ++++++++
>>  1 files changed, 8 insertions(+), 0 deletions(-)
>> 
>> diff --git a/test/test-lib.el b/test/test-lib.el
>> index 52d9936..4330352 100644
>> --- a/test/test-lib.el
>> +++ b/test/test-lib.el
>> @@ -35,6 +35,16 @@
>>      "Disable yes-or-no-p before executing kill-emacs"
>>      (defun yes-or-no-p (prompt) t)))
>>  
>> +;; Emacs bug #2930:
>> +;;	23.0.92; `accept-process-output' and `sleep-for' do not run sentinels
>> +;; seems to be present in Emacs 23.1.
>> +;; Running `list-processes' after `accept-process-output' seems to work
>> +;; around this problem.
>> +(if (and (= emacs-major-version 23) (= emacs-minor-version 1))
>> +  (defadvice accept-process-output (after run-list-processes activate)
>> +    "run list-processes after executing accept-process-output"
>> +    (list-processes)))
>> +
>>  (defun notmuch-test-wait ()
>>    "Wait for process completion."
>>    (while (get-buffer-process (current-buffer))
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/2] test: emacs: run list-processes after accept-process-output in emacs 23.1

[Austin Clements]

Quoth Tomi Ollila on Aug 16 at 10:44 am:
> On Mon, Aug 13 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> 
> > Quoth Tomi Ollila on Aug 05 at  2:13 pm:
> >> 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 sentinel
> >> 
> >> It seems this is present in emacs 23.1.
> >> 
> >> Calling list-processes after accept-process-output seems work around
> >> this problem; in case Emacs version is 23.1 a defadvice is activated
> >> to do just that.
> >
> > Should this workaround perhaps go in notmuch-test-wait directly,
> > instead of being implemented as advice?  
> 
> This way the fact that this is workaround for bug that exists only in
> 23.1(*) is emphasized; The notmuch-test-wait can exist in a (simpler)
> format where it doesn't need to know about this bug.

Okay.

> (*) This bug probably exists in emacs 22 but the MUA has not worked
> on emacs 22 at least for a year now.
> 
> > If we do want to keep it as advice, should it go in notmuch-lib.el
> > along with the few other compatibility functions?
> 
> In tests we have fixed environment where we can write workarounds
> as "global" advices. If we did this in notmuch-lib.el we'd be changing
> users' environment, possibly causing surprises...

Oh, right, of course.  I'd forgotten that this was in test-lib.el and
not in the general notmuch Emacs code.

> ... but neither sleep-for or accept-process-output are used in 
> any of the notmuch elisp code so such "fixes" are not needed now...
> 
> ... and this would ever be a problem, we might already support only
> emacs 24 or newer >;) ...
> 
> Tomi

Series LGTM.

> >> ---
> >> 
> >> Thanks to Austin for the comments and IRC discussions on the matter.
> >> 
> >>  test/test-lib.el |    8 ++++++++
> >>  1 files changed, 8 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/test/test-lib.el b/test/test-lib.el
> >> index 52d9936..4330352 100644
> >> --- a/test/test-lib.el
> >> +++ b/test/test-lib.el
> >> @@ -35,6 +35,16 @@
> >>      "Disable yes-or-no-p before executing kill-emacs"
> >>      (defun yes-or-no-p (prompt) t)))
> >>  
> >> +;; Emacs bug #2930:
> >> +;;	23.0.92; `accept-process-output' and `sleep-for' do not run sentinels
> >> +;; seems to be present in Emacs 23.1.
> >> +;; Running `list-processes' after `accept-process-output' seems to work
> >> +;; around this problem.
> >> +(if (and (= emacs-major-version 23) (= emacs-minor-version 1))
> >> +  (defadvice accept-process-output (after run-list-processes activate)
> >> +    "run list-processes after executing accept-process-output"
> >> +    (list-processes)))
> >> +
> >>  (defun notmuch-test-wait ()
> >>    "Wait for process completion."
> >>    (while (get-buffer-process (current-buffer))

Re: [PATCH 1/2] test: emacs: call accept-process-output in notmuch-test-wait

[Jameson Graef Rollins]

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

On Sun, Aug 05 2012, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> notmuch-test-wait called sleep-for in a loop to wait unconditionally 0.1
> seconds while waiting for process to exit.
> accept-process-output returns as soon as there is any data available
> from process, so using it avoids unnecessary fixed delays.
> Both of these functions run process sentinels.

If this and the following function fix the emacs test crashes on the
buildds then LGTM+++.

jamie.

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

Re: [PATCH 1/2] test: emacs: call accept-process-output in notmuch-test-wait

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:
>
> This 2 patch series is an alternative to 
> id:"1344122222-14344-1-git-send-email-tomi.ollila@iki.fi"
> which speeds up execution when Emacs version is not 23.1.

pushed, 

d