; Remove some forward declarations from ERC tests

* lisp/erc/erc.el (erc--insert-before-markers-transplanting-hidden):
Don't deliberately return anything because the return value is
undefined.  The only existing call site, in `erc-insert-line', invokes
the function indirectly via `erc--insert-line-function' and discards the
result.
* test/lisp/erc/erc-scenarios-join-display-context.el: Require
`erc-join'.
* test/lisp/erc/erc-scenarios-log.el: Remove forward declarations and
require `erc-stamp'.
* test/lisp/erc/erc-scenarios-match.el: Require `erc-match'.
* test/lisp/erc/erc-scenarios-misc.el
(erc-scenarios-base-kill-server-track): Suppress unwanted newline in
test output.
* test/lisp/erc/erc-scenarios-sasl.el (erc-scenarios-sasl--plain-fail):
Don't redundantly bind `erc--warnings-buffer-name' or create a buffer
for it.  Use literal value assigned by test fixture instead.
* test/lisp/erc/erc-scenarios-services-misc.el: Require `erc-services'.
* test/lisp/erc/erc-tests.el (erc-handle-irc-url): Remove redundant
`save-excursion'.
(erc-tests--modules): Reflow into single column for easier management of
divergent WIP patch sets.  This should help minimize manual surgery when
applying them atop one another.
* test/lisp/erc/resources/erc-scenarios-common.el: Remove unused
`require's and forward declarations from top of file.
(erc-scenarios-common--make-bindings): Remove the non-existent
`erc-auth-source-parameters-join-function' and add
`erc--warnings-buffer-name'.  The latter's buffer, if created, will be
killed automatically when the test body exits.
(erc-scenarios-common-with-cleanup): Generate locally scoped defvars to
be used in a manner similar to `dlet' for user options yet to be loaded.
(erc-scenarios-common--assert-date-stamps): Don't run when `erc-stamp'
isn't loaded.

8 files changed, 117 insertions, 77 deletions

diff --git a/test/lisp/erc/erc-scenarios-join-display-context.el b/test/lisp/erc/erc-scenarios-join-display-context.el
index 07d7ff1519e..e525eb59f82 100644
--- a/test/lisp/erc/erc-scenarios-join-display-context.el
+++ b/test/lisp/erc/erc-scenarios-join-display-context.el
@@ -26,6 +26,8 @@
   (let ((load-path (cons (ert-resource-directory) load-path)))
     (require 'erc-scenarios-common)))
 
+(require 'erc-join)
+
 ;; This module uses the list `erc-join--requested-channels' to detect
 ;; whether a JOIN response was likely triggered by an outgoing JOIN
 ;; emitted on behalf of `erc-autojoin-channels-alist'.  When a related
diff --git a/test/lisp/erc/erc-scenarios-log.el b/test/lisp/erc/erc-scenarios-log.el
index 7452062e3c5..4f190ae898e 100644
--- a/test/lisp/erc/erc-scenarios-log.el
+++ b/test/lisp/erc/erc-scenarios-log.el
@@ -28,8 +28,7 @@
 
 (require 'erc-log)
 (require 'erc-truncate)
-
-(defvar erc-timestamp-format-left)
+(require 'erc-stamp)
 
 (ert-deftest erc-scenarios-log--kill-hook ()
   :tags '(:expensive-test)
@@ -327,9 +326,6 @@
        (funcall expect 1 "loathed enemy")
        (funcall expect -0.001 "please your lordship")))))
 
-(defvar erc-insert-timestamp-function)
-(declare-function erc-insert-timestamp-left "erc-stamp" (string))
-
 (ert-deftest erc-scenarios-log--save-buffer-in-logs/truncate-on-save ()
   :tags '(:expensive-test)
   (with-suppressed-warnings ((obsolete erc-truncate-buffer-on-save))
diff --git a/test/lisp/erc/erc-scenarios-match.el b/test/lisp/erc/erc-scenarios-match.el
index bc5ebe27ce9..b22cff18c46 100644
--- a/test/lisp/erc/erc-scenarios-match.el
+++ b/test/lisp/erc/erc-scenarios-match.el
@@ -24,10 +24,7 @@
   (let ((load-path (cons (ert-resource-directory) load-path)))
     (require 'erc-scenarios-common)))
 
-(eval-when-compile
-  (require 'erc-join)
-  (require 'erc-match))
-
+(require 'erc-match)
 (require 'erc-stamp)
 (require 'erc-fill)
 
diff --git a/test/lisp/erc/erc-scenarios-misc.el b/test/lisp/erc/erc-scenarios-misc.el
index e973c912cf9..ef6e2fd3392 100644
--- a/test/lisp/erc/erc-scenarios-misc.el
+++ b/test/lisp/erc/erc-scenarios-misc.el
@@ -236,7 +236,8 @@
         (set-process-query-on-exit-flag erc-server-process nil)
         (kill-buffer))
       (should-not (eq (current-buffer) (get-buffer "#chan"))) ; *temp*
-      (ert-simulate-command '(erc-track-switch-buffer 1)) ; No longer signals
+      (let ((inhibit-message noninteractive))
+        (ert-simulate-command '(erc-track-switch-buffer 1))) ; doesn't signal
       (should (eq (current-buffer) (get-buffer "#chan"))))))
 
 ;;; erc-scenarios-misc.el ends here
diff --git a/test/lisp/erc/erc-scenarios-sasl.el b/test/lisp/erc/erc-scenarios-sasl.el
index c7ea3e46997..edd0dcf3fa0 100644
--- a/test/lisp/erc/erc-scenarios-sasl.el
+++ b/test/lisp/erc/erc-scenarios-sasl.el
@@ -149,8 +149,6 @@
        (erc-modules (cons 'sasl erc-modules))
        (erc-sasl-password "wrong")
        (erc-sasl-mechanism 'plain)
-       (erc--warnings-buffer-name  "*ERC test warnings*")
-       (warnings-buffer (get-buffer-create erc--warnings-buffer-name))
        (inhibit-message noninteractive)
        (expect (erc-d-t-make-expecter)))
 
@@ -164,7 +162,7 @@
       (funcall expect 20 "Connection failed!")
       (should-not (erc-server-process-alive)))
 
-    (with-current-buffer warnings-buffer
+    (with-current-buffer "*ERC test warnings*"
       (funcall expect 10 "please review SASL settings")))
 
   (when noninteractive
diff --git a/test/lisp/erc/erc-scenarios-services-misc.el b/test/lisp/erc/erc-scenarios-services-misc.el
index bc6522ae110..6a7d3c2f882 100644
--- a/test/lisp/erc/erc-scenarios-services-misc.el
+++ b/test/lisp/erc/erc-scenarios-services-misc.el
@@ -24,8 +24,7 @@
   (let ((load-path (cons (ert-resource-directory) load-path)))
     (require 'erc-scenarios-common)))
 
-(eval-when-compile (require 'erc-join)
-                   (require 'erc-services))
+(require 'erc-services)
 
 (ert-deftest erc-scenarios-services-password ()
   :tags '(:expensive-test)
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index dd439e73fc9..2b8e6b3ecca 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -3602,8 +3602,7 @@
         (should-not calls))
 
       (ert-info ("Known network, existing chan with key")
-        (save-excursion
-          (with-current-buffer "foonet" (erc--open-target "#chan")))
+        (with-current-buffer "foonet" (erc--open-target "#chan"))
         (erc-handle-irc-url "irc.foonet.org" nil "#chan?sec" nil nil "irc")
         (should (equal '("#chan" "sec") (pop calls)))
         (should-not calls))
@@ -3685,12 +3684,45 @@
       (should (= 0 (erc-channel-user-status u))))))
 
 (defconst erc-tests--modules
-  '( autoaway autojoin bufbar button capab-identify
-     command-indicator completion dcc fill identd
-     imenu irccontrols keep-place list log match menu move-to-prompt netsplit
-     networks nickbar nicks noncommands notifications notify page readonly
-     replace ring sasl scrolltobottom services smiley sound
-     spelling stamp track truncate unmorse xdcc))
+  '(autoaway
+    autojoin
+    bufbar
+    button
+    capab-identify
+    command-indicator
+    completion
+    dcc
+    fill
+    identd
+    imenu
+    irccontrols
+    keep-place
+    list log
+    match
+    menu
+    move-to-prompt
+    netsplit
+    networks
+    nickbar
+    nicks
+    noncommands
+    notifications
+    notify
+    page
+    readonly
+    replace
+    ring
+    sasl
+    scrolltobottom
+    services
+    smiley
+    sound
+    spelling
+    stamp
+    track
+    truncate
+    unmorse
+    xdcc))
 
 ;; Ensure that `:initialize' doesn't change the ordering of the
 ;; members because otherwise the widget's state is "edited".
diff --git a/test/lisp/erc/resources/erc-scenarios-common.el b/test/lisp/erc/resources/erc-scenarios-common.el
index fa187cc59f7..aa18bbf7b21 100644
--- a/test/lisp/erc/resources/erc-scenarios-common.el
+++ b/test/lisp/erc/resources/erc-scenarios-common.el
@@ -93,13 +93,6 @@
 
 (require 'erc)
 
-(eval-when-compile (require 'erc-join)
-                   (require 'erc-services)
-                   (require 'erc-fill))
-
-(declare-function erc-network "erc-networks")
-(defvar erc-network)
-
 (defvar erc-scenarios-common--resources-dir
   (expand-file-name "../" (ert-resource-directory)))
 
@@ -149,76 +142,96 @@
       (auth-source-do-cache nil)
       (timer-list (copy-sequence timer-list))
       (timer-idle-list (copy-sequence timer-idle-list))
-      (erc-auth-source-parameters-join-function nil)
+      ;; This binding exists to protect the default value because ERC
+      ;; adds all joined channels automatically.
       (erc-autojoin-channels-alist nil)
       (erc-server-auto-reconnect nil)
       (erc-after-connect nil)
       (erc-last-input-time 0)
       (erc-d-linger-secs 10)
+      ;; This buffer, if created by `erc--lwarn', will be killed before
+      ;; `erc-scenarios-common-with-cleanup' exits.
+      (erc--warnings-buffer-name  "*ERC test warnings*")
       ,@bindings)))
 
 (defmacro erc-scenarios-common-with-cleanup (bindings &rest body)
   "Provide boilerplate cleanup tasks after calling BODY with BINDINGS.
+Shadow various options and variables used by ERC with values more
+suitable for test purposes.  These can be overridden in the \"varlist\"
+BINDINGS.  Upon exiting, kill buffers and delete processes created by
+ERC, as well as any bound to variables in BINDINGS.  However, adding
+items not referenced in BODY for this purpose alone can confuse readers.
 
-If an `erc-d' process exists, wait for it to start before running BODY.
-If `erc-autojoin-mode' mode is bound, restore it during cleanup if
-disabled by BODY.  Other defaults common to these test cases are added
-below and can be overridden, except when wanting the \"real\" default
-value, which must be looked up or captured outside of the calling form.
+Avoid taking special care to restore the effect of activating global
+modules in BODY.  However, as a special case, if the variable
+`erc-autojoin-mode' mode is bound, restore its minor-mode activation
+state during teardown if modified by BODY.
 
-When running tests tagged as serially runnable while interactive
-and the flag `erc-scenarios-common--graphical-p' is non-nil, run
-teardown tasks normally inhibited when interactive.  That is,
-behave almost as if `noninteractive' were also non-nil, and
-ensure buffers and other resources are destroyed on completion.
+Additionally, prepare the environment for an `erc-d' test server.  If an
+`erc-d' process exists, wait for it to start before running BODY.
+Locate dialog resource directories by expanding the variable
+`erc-scenarios-common-dialog' or its value in BINDINGS.
 
-Dialog resource directories are located by expanding the variable
-`erc-scenarios-common-dialog' or its value in BINDINGS."
+If the flag `erc-scenarios-common--graphical-p' is non-nil and a test is
+tagged as interactive-aware, usually via the \"ERC_TESTS_GRAPHICAL\"
+environment variable, run teardown tasks normally inhibited when
+interactive.  That is, behave almost as if `noninteractive' were also
+non-nil, and ensure buffers and other resources are destroyed on
+completion."
   (declare (indent 1))
 
   (let* ((orig-autojoin-mode (make-symbol "orig-autojoin-mode"))
          (combined `((,orig-autojoin-mode (bound-and-true-p erc-autojoin-mode))
-                     ,@(erc-scenarios-common--make-bindings bindings))))
+                     ,@(erc-scenarios-common--make-bindings bindings)))
+         (dynvars ()))
 
-    `(erc-d-t-with-cleanup (,@combined)
+    ;; Declare "erc-" variables dynamic in test scope.
+    (dolist (binder combined)
+      (setq binder (ensure-list binder))
+      (when (and (string-prefix-p "erc-" (symbol-name (car binder)))
+                 (not (special-variable-p (car binder))))
+        (push `(defvar ,(car binder)) dynvars)))
+    `(let (_)
+       ,@dynvars
+       (erc-d-t-with-cleanup (,@combined)
 
-         (ert-info ("Restore autojoin, etc., kill ERC buffers")
-           (dolist (buf (buffer-list))
-             (when-let* ((erc-d-u--process-buffer)
-                         (proc (get-buffer-process buf)))
-               (delete-process proc)))
+           (ert-info ("Restore autojoin, etc., kill ERC buffers")
+             (dolist (buf (buffer-list))
+               (when-let* ((erc-d-u--process-buffer)
+                           (proc (get-buffer-process buf)))
+                 (delete-process proc)))
 
-           (erc-scenarios-common--remove-silence)
+             (erc-scenarios-common--remove-silence)
 
-           (when erc-scenarios-common-extra-teardown
-             (ert-info ("Running extra teardown")
-               (funcall erc-scenarios-common-extra-teardown)))
+             (when erc-scenarios-common-extra-teardown
+               (ert-info ("Running extra teardown")
+                 (funcall erc-scenarios-common-extra-teardown)))
 
-           (erc-buffer-do #'erc-scenarios-common--assert-date-stamps)
-           (when (and (boundp 'erc-autojoin-mode)
-                      (not (eq erc-autojoin-mode ,orig-autojoin-mode)))
-             (erc-autojoin-mode (if ,orig-autojoin-mode +1 -1)))
+             (erc-buffer-do #'erc-scenarios-common--assert-date-stamps)
+             (when (and (boundp 'erc-autojoin-mode)
+                        (not (eq erc-autojoin-mode ,orig-autojoin-mode)))
+               (erc-autojoin-mode (if ,orig-autojoin-mode +1 -1)))
 
-           (when (or noninteractive erc-scenarios-common--graphical-p)
-             (when noninteractive
-               (erc-scenarios-common--print-trace))
-             (erc-d-t-kill-related-buffers)
-             (delete-other-windows)))
+             (when (or noninteractive erc-scenarios-common--graphical-p)
+               (when noninteractive
+                 (erc-scenarios-common--print-trace))
+               (erc-d-t-kill-related-buffers)
+               (delete-other-windows)))
 
-       (erc-scenarios-common--add-silence)
+         (erc-scenarios-common--add-silence)
 
-       (ert-info ("Wait for dumb server")
-         (dolist (buf (buffer-list))
-           (with-current-buffer buf
-             (when erc-d-u--process-buffer
-               (erc-d-t-search-for 3 "Starting")))))
+         (ert-info ("Wait for dumb server")
+           (dolist (buf (buffer-list))
+             (with-current-buffer buf
+               (when erc-d-u--process-buffer
+                 (erc-d-t-search-for 3 "Starting")))))
 
-       (ert-info ("Activate erc-debug-irc-protocol")
-         (unless (and (or noninteractive erc-scenarios-common--graphical-p)
-                      (not erc-debug-irc-protocol))
-           (erc-toggle-debug-irc-protocol)))
+         (ert-info ("Activate erc-debug-irc-protocol")
+           (unless (and (or noninteractive erc-scenarios-common--graphical-p)
+                        (not erc-debug-irc-protocol))
+             (erc-toggle-debug-irc-protocol)))
 
-       ,@body)))
+         ,@body))))
 
 (defvar erc-scenarios-common--term-size '(34 . 80))
 (declare-function term-char-mode "term" nil)
@@ -336,9 +349,11 @@ See Info node `(emacs) Term Mode' for the various commands."
 
 (defun erc-scenarios-common--assert-date-stamps ()
   "Ensure all date stamps are accounted for."
-  (dolist (stamp erc-stamp--date-stamps)
-    (should (eq 'datestamp (get-text-property (erc-stamp--date-marker stamp)
-                                              'erc--msg)))))
+  (defvar erc-stamp--date-stamps)
+  (when (fboundp 'erc-stamp--date-marker)
+    (dolist (stamp erc-stamp--date-stamps)
+      (should (eq 'datestamp (get-text-property (erc-stamp--date-marker stamp)
+                                                'erc--msg))))))
 
 (defun erc-scenarios-common-assert-initial-buf-name (id port)
   ;; Assert no limbo period when explicit ID given