From fd0fed210fca48cc8a7f754011b1e1aaabc4d9f4 Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Mon, 30 Oct 2023 23:36:54 -0700 Subject: [PATCH 1/2] [5.6] ; Fix concurrency bug in erc-buffer-display test * test/lisp/erc/erc-fill-tests.el (erc-fill-tests--time-vals, erc-fill-tests--current-time-value): Rename former to latter and change type from function to natnum. (erc-fill-tests--wrap-populate, erc-fill-wrap--merge, erc-fill-wrap--merge-action): Use `erc-fill-tests--current-time-value' instead of function `erc-fill-tests--time-vals'. * test/lisp/erc/erc-scenarios-base-association.el (erc-scenarios-common--base-association-multi-net): Extend timeout. * test/lisp/erc/erc-scenarios-base-buffer-display.el (erc-scenarios-base-buffer-display--reconnect-common): Move some common assertions here from callers. (erc-scenarios-base-buffer-display--defwin-recbury-intbuf, erc-scenarios-base-buffer-display--count-reset-timeout): Factor out a couple common assertions. Clarify some comments. (erc-scenarios-base-buffer-display--defwino-recbury-intbuf): Factor out a couple common assertions. Clarify some comments. Account for possible concurrency bug leading to intermittent test failures. * test/lisp/erc/erc-scenarios-base-misc-regressions.el (erc-scenarios-base-gapless-connect, erc-scenarios-base-channel-buffer-revival): Extend timeouts. * test/lisp/erc/resources/dcc/chat/accept.eld: Extend timeout. * test/lisp/erc/resources/base/reconnect/options-again.eld: Extend timeouts. * test/lisp/erc/resources/erc-d/erc-d.el (erc-d--m): Prevent possible wrong-type error. * test/lisp/erc/resources/erc-d/resources/dynamic-foonet.eld: Extend timeouts. * test/lisp/erc/resources/erc-scenarios-common.el (erc-scenarios-common--base-network-id-bouncer): Extend timeout. --- test/lisp/erc/erc-fill-tests.el | 10 +- .../erc/erc-scenarios-base-association.el | 2 +- .../erc/erc-scenarios-base-buffer-display.el | 104 ++++++++++-------- .../erc-scenarios-base-misc-regressions.el | 4 +- .../base/reconnect/options-again.eld | 4 +- test/lisp/erc/resources/dcc/chat/accept.eld | 2 +- test/lisp/erc/resources/erc-d/erc-d.el | 2 +- .../erc-d/resources/dynamic-foonet.eld | 2 +- .../erc/resources/erc-scenarios-common.el | 2 +- 9 files changed, 73 insertions(+), 59 deletions(-) diff --git a/test/lisp/erc/erc-fill-tests.el b/test/lisp/erc/erc-fill-tests.el index 92424d1e556..8179cbda2cb 100644 --- a/test/lisp/erc/erc-fill-tests.el +++ b/test/lisp/erc/erc-fill-tests.el @@ -27,7 +27,7 @@ (require 'erc-fill) (defvar erc-fill-tests--buffers nil) -(defvar erc-fill-tests--time-vals (lambda () 0)) +(defvar erc-fill-tests--current-time-value 0) (defun erc-fill-tests--insert-privmsg (speaker &rest msg-parts) (declare (indent 1)) @@ -49,7 +49,7 @@ erc-fill-tests--wrap-populate extended-command-history erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook) (cl-letf (((symbol-function 'erc-stamp--current-time) - (lambda () (funcall erc-fill-tests--time-vals))) + (lambda () erc-fill-tests--current-time-value)) ((symbol-function 'erc-server-connect) (lambda (&rest _) (setq erc-server-process @@ -261,7 +261,7 @@ erc-fill-wrap--merge ;; Set this here so that the first few messages are from 1970. ;; Following the current date stamp, the speaker isn't merged ;; even though it's continued: " zero." - (let ((erc-fill-tests--time-vals (lambda () 1680332400))) + (let ((erc-fill-tests--current-time-value 1680332400)) (erc-fill-tests--insert-privmsg "bob" "zero.") (erc-fill-tests--insert-privmsg "alice" "one.") (erc-fill-tests--insert-privmsg "alice" "two.") @@ -297,8 +297,8 @@ erc-fill-wrap--merge-action (erc-fill-tests--wrap-populate (lambda () - ;; Set this here so that the first few messages are from 1970 - (let ((erc-fill-tests--time-vals (lambda () 1680332400))) + ;; Allow prior messages to be from 1970. + (let ((erc-fill-tests--current-time-value 1680332400)) (erc-fill-tests--insert-privmsg "bob" "zero.") (erc-fill-tests--insert-privmsg "bob" "0.5") diff --git a/test/lisp/erc/erc-scenarios-base-association.el b/test/lisp/erc/erc-scenarios-base-association.el index a40a4cb7550..10abe14c43b 100644 --- a/test/lisp/erc/erc-scenarios-base-association.el +++ b/test/lisp/erc/erc-scenarios-base-association.el @@ -78,7 +78,7 @@ erc-scenarios-common--base-association-multi-net (with-current-buffer "#chan@foonet" (funcall expect 3 "bob") (funcall expect 3 "was created on") - (funcall expect 3 "prosperous"))) + (funcall expect 10 "prosperous"))) (ert-info ("All #chan@barnet output consumed") (with-current-buffer "#chan@barnet" diff --git a/test/lisp/erc/erc-scenarios-base-buffer-display.el b/test/lisp/erc/erc-scenarios-base-buffer-display.el index df292a8c113..6a80baeaaa9 100644 --- a/test/lisp/erc/erc-scenarios-base-buffer-display.el +++ b/test/lisp/erc/erc-scenarios-base-buffer-display.el @@ -27,7 +27,10 @@ (eval-when-compile (require 'erc-join)) ;; These first couple `erc-auto-reconnect-display' tests used to live -;; in erc-scenarios-base-reconnect but have since been renamed. +;; in erc-scenarios-base-reconnect but have since been renamed. Note +;; that these are somewhat difficult to reason about because the user +;; joins a second channel after reconnecting, and the first is +;; controlled by `autojoin'. (defun erc-scenarios-base-buffer-display--reconnect-common (assert-server assert-chan assert-rest) @@ -55,6 +58,7 @@ erc-scenarios-base-buffer-display--reconnect-common (ert-info ("Wait for some output in channels") (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan")) (funcall assert-chan expect) + (funcall expect 10 "welcome") (funcall expect 10 "welcome"))) (ert-info ("Server buffer shows connection failed") @@ -68,6 +72,10 @@ erc-scenarios-base-buffer-display--reconnect-common (ert-info ("Wait for auto reconnect") (with-current-buffer "FooNet" (funcall expect 10 "still in debug mode"))) + (ert-info ("Lone window still shows messages buffer") + (should (eq (window-buffer) (messages-buffer))) + (should (frame-root-window-p (selected-window)))) + (funcall assert-rest expect) (ert-info ("Wait for activity to recommence in both channels") @@ -76,40 +84,50 @@ erc-scenarios-base-buffer-display--reconnect-common (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#spam")) (funcall expect 10 "her elves come here anon"))))) +;; Interactively issuing a slash command resets the auto-reconnect +;; count, making ERC ignore the option `erc-auto-reconnect-display' +;; when next displaying a newly set up buffer. In the case of a +;; /JOIN, the option `erc-interactive-display' takes precedence. (ert-deftest erc-scenarios-base-buffer-display--defwin-recbury-intbuf () :tags '(:expensive-test) (should (eq erc-buffer-display 'bury)) (should (eq erc-interactive-display 'window)) (should-not erc-auto-reconnect-display) - (let ((erc-buffer-display 'window) - (erc-interactive-display 'buffer) - (erc-auto-reconnect-display 'bury)) + (let ((erc-buffer-display 'window) ; defwin + (erc-interactive-display 'buffer) ; intbuf + (erc-auto-reconnect-display 'bury)) ; recbury (erc-scenarios-base-buffer-display--reconnect-common (lambda (_) - (should (eq (window-buffer) (current-buffer))) - (should-not (frame-root-window-p (selected-window)))) + (ert-info ("New server buffer appears in a selected split") + (should (eq (window-buffer) (current-buffer))) + (should-not (frame-root-window-p (selected-window))))) (lambda (_) - (should (eq (window-buffer) (current-buffer))) - (should (equal (get-buffer "FooNet") (window-buffer (next-window))))) + (ert-info ("New channel buffer appears in other window") + (should (eq (window-buffer) (current-buffer))) ; selected + (should (equal (get-buffer "FooNet") (window-buffer (next-window)))))) + + (lambda (expect) + ;; If we /JOIN #spam now, we'll cancel the auto-reconnect + ;; timer, and "#chan" may well pop up in a split before we can + ;; verify that the lone window displays #spam (a race, IOW). + (ert-info ("Autojoined channel #chan buried on JOIN") + (with-current-buffer "#chan" + (funcall expect 10 "You have joined channel #chan")) + (should (frame-root-window-p (selected-window))) + (should (eq (window-buffer) (messages-buffer)))) - (lambda (_) - (with-current-buffer "FooNet" - (should (eq (window-buffer) (messages-buffer))) - (should (frame-root-window-p (selected-window)))) - - ;; A manual /JOIN command tells ERC we're done auto-reconnecting (with-current-buffer "FooNet" (erc-scenarios-common-say "/JOIN #spam")) - (ert-info ("#spam ignores `erc-auto-reconnect-display'") - ;; Uses `erc-interactive-display' instead. + (ert-info ("A /JOIN ignores `erc-auto-reconnect-display'") (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#spam")) (should (eq (window-buffer) (get-buffer "#spam"))) - ;; Option `buffer' replaces entire window (no split) - (erc-d-t-wait-for 5 (frame-root-window-p (selected-window))))))))) + ;; Option `erc-interactive-display' being `buffer' means + ;; Emacs reuses the selected window (no split). + (should (frame-root-window-p (selected-window))))))))) (ert-deftest erc-scenarios-base-buffer-display--defwino-recbury-intbuf () :tags '(:expensive-test) @@ -117,7 +135,7 @@ erc-scenarios-base-buffer-display--defwino-recbury-intbuf (should (eq erc-interactive-display 'window)) (should-not erc-auto-reconnect-display) - (let ((erc-buffer-display 'window-noselect) + (let ((erc-buffer-display 'window-noselect) ; defwino (erc-auto-reconnect-display 'bury) (erc-interactive-display 'buffer)) (erc-scenarios-base-buffer-display--reconnect-common @@ -139,26 +157,24 @@ erc-scenarios-base-buffer-display--defwino-recbury-intbuf (should (eq (current-buffer) (window-buffer (next-window))))) (lambda (_) - (with-current-buffer "FooNet" - (should (eq (window-buffer) (messages-buffer))) - (should (frame-root-window-p (selected-window)))) - - ;; A non-interactive JOIN command doesn't signal that we're - ;; done auto-reconnecting, and `erc-interactive-display' is - ;; ignored, so `erc-buffer-display' is again in charge (here, - ;; that means `window-noselect'). - (ert-info ("Join chan noninteractively and open a /QUERY") + ;; A JOIN command sent from lisp code is "non-interactive" and + ;; doesn't reset the auto-reconnect count, so ERC treats the + ;; response as possibly server-initiated or otherwise the + ;; result of an autojoin and continues to favor + ;; `erc-auto-reconnect-display'. + (ert-info ("Join chan non-interactively and open a /QUERY") (with-current-buffer "FooNet" - (erc-cmd-JOIN "#spam") - ;; However this will reset the option. - (erc-scenarios-common-say "/QUERY bob") + (erc-cmd-JOIN "#spam") ; "non-interactive" according to ERC + (erc-scenarios-common-say "/QUERY bob") ; resets count (should (eq (window-buffer) (get-buffer "bob"))) (should (frame-root-window-p (selected-window))))) + ;; The /QUERY above resets the count, and `erc-buffer-display' + ;; again decides how #spam is displayed. (ert-info ("Newly joined chan ignores `erc-auto-reconnect-display'") (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#spam")) (should (eq (window-buffer) (get-buffer "bob"))) - (should-not (frame-root-window-p (selected-window))) + (should-not (frame-root-window-p (selected-window))) ; noselect (should (eq (current-buffer) (window-buffer (next-window)))))))))) (ert-deftest erc-scenarios-base-buffer-display--count-reset-timeout () @@ -177,24 +193,22 @@ erc-scenarios-base-buffer-display--count-reset-timeout (lambda (_) (with-current-buffer "FooNet" - (should erc--server-reconnect-display-timer) - (should (eq (window-buffer) (messages-buffer))) - (should (frame-root-window-p (selected-window)))) + (should erc--server-reconnect-display-timer)) ;; A non-interactive JOIN command doesn't signal that we're - ;; done auto-reconnecting - (ert-info ("Join chan noninteractively") + ;; done auto-reconnecting. + (ert-info ("Join channel #spam non-interactively") (with-current-buffer "FooNet" (erc-d-t-wait-for 1 (null erc--server-reconnect-display-timer)) - (erc-cmd-JOIN "#spam"))) + (erc-cmd-JOIN "#spam"))) ; not processed as a /JOIN - (ert-info ("Newly joined chan ignores `erc-auto-reconnect-display'") - (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#spam")) - (should (eq (window-buffer) (messages-buffer))) - ;; If `erc-auto-reconnect-display-timeout' were left alone, this - ;; would be (frame-root-window-p #). - (should-not (frame-root-window-p (selected-window))) - (should (eq (current-buffer) (window-buffer (next-window)))))))))) + (ert-info ("Option `erc-auto-reconnect-display' ignored w/o timer") + (should (eq (window-buffer) (messages-buffer))) + (erc-d-t-wait-for 10 (get-buffer "#spam")) + ;; If `erc-auto-reconnect-display-timeout' were left alone, + ;; this would be (frame-root-window-p #). + (should-not (frame-root-window-p (selected-window))) + (should (eq (get-buffer "#spam") (window-buffer (next-window))))))))) ;; This shows that the option `erc-interactive-display' overrides ;; `erc-join-buffer' during cold opens and interactive /JOINs. diff --git a/test/lisp/erc/erc-scenarios-base-misc-regressions.el b/test/lisp/erc/erc-scenarios-base-misc-regressions.el index c1915d088a0..42d7653d3ec 100644 --- a/test/lisp/erc/erc-scenarios-base-misc-regressions.el +++ b/test/lisp/erc/erc-scenarios-base-misc-regressions.el @@ -77,7 +77,7 @@ erc-scenarios-base-gapless-connect (with-current-buffer (erc-d-t-wait-for 20 (get-buffer "#bar")) (funcall expect 10 "was created on") - (funcall expect 2 "his second fit")) + (funcall expect 10 "his second fit")) (with-current-buffer (erc-d-t-wait-for 20 (get-buffer "#foo")) (funcall expect 10 "was created on") @@ -108,7 +108,7 @@ erc-scenarios-base-channel-buffer-revival (should (string= (buffer-name) (format "127.0.0.1:%d" port))))) (ert-info ("Server buffer is unique and temp name is absent") - (erc-d-t-wait-for 1 (get-buffer "FooNet")) + (erc-d-t-wait-for 10 (get-buffer "FooNet")) (should-not (erc-scenarios-common-buflist "127.0.0.1")) (with-current-buffer erc-server-buffer-foo (erc-cmd-JOIN "#chan"))) diff --git a/test/lisp/erc/resources/base/reconnect/options-again.eld b/test/lisp/erc/resources/base/reconnect/options-again.eld index f1fcc439cc3..8a3264fda9c 100644 --- a/test/lisp/erc/resources/base/reconnect/options-again.eld +++ b/test/lisp/erc/resources/base/reconnect/options-again.eld @@ -32,13 +32,13 @@ (0 ":irc.foonet.org 353 tester = #spam :alice tester @bob") (0 ":irc.foonet.org 366 tester #spam :End of NAMES list")) -((~mode-chan 4 "MODE #chan") +((~mode-chan 10 "MODE #chan") (0 ":irc.foonet.org 324 tester #chan +nt") (0 ":irc.foonet.org 329 tester #chan 1620104779") (0.1 ":bob!~u@rz2v467q4rwhy.irc PRIVMSG #chan :alice: But, as it seems, did violence on herself.") (0.1 ":alice!~u@rz2v467q4rwhy.irc PRIVMSG #chan :bob: Well, this is the forest of Arden.")) -((mode-spam 4 "MODE #spam") +((mode-spam 20 "MODE #spam") (0 ":irc.foonet.org 324 tester #spam +nt") (0 ":irc.foonet.org 329 tester #spam 1620104779") (0.1 ":bob!~u@rz2v467q4rwhy.irc PRIVMSG #spam :alice: Signior Iachimo will not from it. Pray, let us follow 'em.") diff --git a/test/lisp/erc/resources/dcc/chat/accept.eld b/test/lisp/erc/resources/dcc/chat/accept.eld index a23e9580bcc..463f931d26f 100644 --- a/test/lisp/erc/resources/dcc/chat/accept.eld +++ b/test/lisp/erc/resources/dcc/chat/accept.eld @@ -17,7 +17,7 @@ (0 ":irc.foonet.org 266 tester 4 4 :Current global users 4, max 4") (0 ":irc.foonet.org 422 tester :MOTD File is missing")) -((mode-user 1.2 "MODE tester +i") +((mode-user 10 "MODE tester +i") ;; No mode answer (0 ":irc.foonet.org NOTICE tester :This server is in debug mode and is logging all user I/O. If you do not wish for everything you send to be readable by the server owner(s), please disconnect.") (0.2 ":dummy!~u@34n9brushbpj2.irc PRIVMSG tester :\C-aDCC CHAT chat 2130706433 " port "\C-a")) diff --git a/test/lisp/erc/resources/erc-d/erc-d.el b/test/lisp/erc/resources/erc-d/erc-d.el index f072c6b93b2..a87904e5830 100644 --- a/test/lisp/erc/resources/erc-d/erc-d.el +++ b/test/lisp/erc/resources/erc-d/erc-d.el @@ -297,7 +297,7 @@ erc-d--m (when erc-d--m-debug (setq format-string (concat (format-time-string "%s.%N: ") format-string))) (let ((insertp (and process erc-d--in-process)) - (buffer (process-buffer (process-get process :server)))) + (buffer (and process (process-buffer (process-get process :server))))) (when (and insertp (buffer-live-p buffer)) (princ (concat (apply #'format format-string args) "\n") buffer)) (when (or erc-d--m-debug (not insertp)) diff --git a/test/lisp/erc/resources/erc-d/resources/dynamic-foonet.eld b/test/lisp/erc/resources/erc-d/resources/dynamic-foonet.eld index e5532980644..2db750e49da 100644 --- a/test/lisp/erc/resources/erc-d/resources/dynamic-foonet.eld +++ b/test/lisp/erc/resources/erc-d/resources/dynamic-foonet.eld @@ -17,7 +17,7 @@ (0. ":irc.foonet.org 266 tester 3 3 :Current global users 3, max 3") (0. ":irc.foonet.org 422 tester :MOTD File is missing")) -((mode-user 2 "MODE tester +i") +((mode-user 4 "MODE tester +i") (0. ":irc.foonet.org 221 tester +Zi") (0. ":irc.foonet.org 306 tester :You have been marked as being away") (0 ":tester!~u@awyxgybtkx7uq.irc JOIN #chan") diff --git a/test/lisp/erc/resources/erc-scenarios-common.el b/test/lisp/erc/resources/erc-scenarios-common.el index 9e134e6932f..802ccaeedaa 100644 --- a/test/lisp/erc/resources/erc-scenarios-common.el +++ b/test/lisp/erc/resources/erc-scenarios-common.el @@ -455,7 +455,7 @@ erc-scenarios-common--base-network-id-bouncer :id foo-id)) (setq erc-server-process-foo erc-server-process) (erc-scenarios-common-assert-initial-buf-name foo-id port) - (erc-d-t-wait-for 3 (eq (erc-network) 'foonet)) + (erc-d-t-wait-for 6 (eq (erc-network) 'foonet)) (erc-d-t-wait-for 3 (string= (buffer-name) serv-buf-foo)) (funcall expect 5 "foonet"))) -- 2.41.0