bug#38813: Make comint-tests more robust

bug#38813: Make comint-tests more robust

[Mattias Engdegård]

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

There seem to be races in the new comint-tests; they frequently hang and/or fail when run (make check) on macOS.
This patch should improve matters. OK for emacs-27?


[-- Attachment #2: 0001-Make-comint-tests-more-robust.patch --]
[-- Type: application/octet-stream, Size: 3303 bytes --]

From b5ec344dc37408bc97385476767df953a47d0061 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Mon, 30 Dec 2019 14:10:02 +0100
Subject: [PATCH] Make comint-tests more robust

* test/lisp/comint-tests.el (comint-test-no-password-function)
(comint-test-password-function-with-value)
(comint-test-password-function-with-nil):
Call accept-process-output twice, with a slightly more generous
timeout (100 ms), after sending the Password: prompt to the
process, since there must be time for some back-and-forth
communication.  Also clear the process-query-on-exit flag, since
it doesn't go well with noninteractive tests.
---
 test/lisp/comint-tests.el | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/test/lisp/comint-tests.el b/test/lisp/comint-tests.el
index c04134599f..9960f5e65d 100644
--- a/test/lisp/comint-tests.el
+++ b/test/lisp/comint-tests.el
@@ -64,10 +64,11 @@ comint-test-no-password-function
         (with-temp-buffer
           (make-comint-in-buffer "test-comint-password" (current-buffer) cat)
           (let ((proc (get-buffer-process (current-buffer))))
+            (set-process-query-on-exit-flag proc nil)
             (comint-send-string proc "Password: ")
-            (accept-process-output proc 0 1 t)
             (comint-send-eof)
-            (accept-process-output proc 0 1 t)
+            (accept-process-output proc 0 100 t)
+            (accept-process-output proc 0 100 t)
             (should (string-equal (buffer-substring-no-properties (point-min) (point-max))
                                   "Password: PaSsWoRd123\n"))
             (when (process-live-p proc)
@@ -87,10 +88,11 @@ comint-test-password-function-with-value
         (with-temp-buffer
           (make-comint-in-buffer "test-comint-password" (current-buffer) cat)
           (let ((proc (get-buffer-process (current-buffer))))
+            (set-process-query-on-exit-flag proc nil)
             (comint-send-string proc "Password: ")
-            (accept-process-output proc 0 1 t)
             (comint-send-eof)
-            (accept-process-output proc 0 1 t)
+            (accept-process-output proc 0 100 t)
+            (accept-process-output proc 0 100 t)
             (should (string-equal (buffer-substring-no-properties (point-min) (point-max))
                                   "Password: MaGiC-PaSsWoRd789\n"))
             (when (process-live-p proc)
@@ -110,10 +112,11 @@ comint-test-password-function-with-nil
         (with-temp-buffer
           (make-comint-in-buffer "test-comint-password" (current-buffer) cat)
           (let ((proc (get-buffer-process (current-buffer))))
+            (set-process-query-on-exit-flag proc nil)
             (comint-send-string proc "Password: ")
-            (accept-process-output proc 0 1 t)
             (comint-send-eof)
-            (accept-process-output proc 0 1 t)
+            (accept-process-output proc 0 100 t)
+            (accept-process-output proc 0 100 t)
             (should (string-equal (buffer-substring-no-properties (point-min) (point-max))
                                   "Password: PaSsWoRd456\n"))
             (when (process-live-p proc)
-- 
2.21.0 (Apple Git-122.2)

bug#38813: Make comint-tests more robust

[Michael Albinus]

Mattias Engdegård <mattiase@acm.org> writes:

> There seem to be races in the new comint-tests; they frequently hang
> and/or fail when run (make check) on macOS.
> This patch should improve matters. OK for emacs-27?

> -            (accept-process-output proc 0 1 t)
> +            (accept-process-output proc 0 100 t)
> +            (accept-process-output proc 0 100 t)

The usual pattern is (while (accept-process-output proc 0 nil t))
That's what I do in tramp-tests.el

Best regards, Michael.

bug#38813: Make comint-tests more robust

[Mattias Engdegård]

30 dec. 2019 kl. 16.20 skrev Michael Albinus <michael.albinus@gmx.de>:

> The usual pattern is (while (accept-process-output proc 0 nil t))
> That's what I do in tramp-tests.el

Thank you, but that only works if inside another loop, and then it is a busy-wait, right?
We don't really have a predicate to tell us when we need to stop waiting, unless we use the test condition for that.
We could do

 (while (accept-process-output proc 0.1 nil t))

which isn't strictly necessary since the test logic guarantees that only two accept-process-output calls should be needed, but it doesn't hurt (much).

bug#38813: Make comint-tests more robust

[Michael Albinus]

Mattias Engdegård <mattiase@acm.org> writes:

>> The usual pattern is (while (accept-process-output proc 0 nil t))
>> That's what I do in tramp-tests.el
>
> Thank you, but that only works if inside another loop, and then it is
> a busy-wait, right?

I'm not sure. Some months (years?) ago there was a discussion about on
emacs-devel, and AFAIR this form was proposed by Stefan.

If needed, I could check the archives.

Best regards, Michael.

bug#38813: Make comint-tests more robust

[Mattias Engdegård]

30 dec. 2019 kl. 19.28 skrev Michael Albinus <michael.albinus@gmx.de>:

> I'm not sure. Some months (years?) ago there was a discussion about on
> emacs-devel, and AFAIR this form was proposed by Stefan.
> 
> If needed, I could check the archives.

Thank you, I don't think that's necessary. The improved patch has been pushed to emacs-27.