Subject: Re: bug#76446: [PATCH v2 1/5] tests: Move helper and wrapper functions into their own file

From: Morgan Smith <[email-masked]>
Date: Sun, 23 Feb 2025 14:05:17 -0500

Michael Albinus <[email-masked]> writes:

> Morgan Smith <[email-masked]> writes:
>
>>> We might be better with
>>>
>>> --8<---------------cut here---------------start------------->8---
>>> @$(EMACS) -Q --batch -L . -L ./test -l $(TESTSOURCE) -f ert-run-tests-batch-and-exit
>>> --8<---------------cut here---------------end--------------->8---
>>>
>>
>> I did the '-L ./test' part but I kept the foreach.
>>
>> Without the foreach the command looks like "... -l file1.el file2.el
>> file3.el ..." and emacs only actually loads the first file.  The command
>> needs to look like "... -l file1.el -l file2.el -l file3.el ..."
>
> My point is, that we have only the make target 'check', which runs all
> ert tests located in different files. But I would also like to run only
> the tests in, say for example, debbugs-tests.el. So we need an own
> target for every *-tests.el file.
>

I've now implemented the interface I've seen in other projects (guix,
org-mode) to enable this feature.  It looks something like this: "make
check TESTS='get-status'".

>>>> --- /dev/null
>>>> +++ b/test/debbugs-test-helpers.el
>>>> @@ -0,0 +1,146 @@
>>>> +(defconst debbugs-test--bug-status-soap-return
>>>> +(defconst debbugs-test--bug-status
>>>
>>> Perhaps we should move these data into an ert-resource file (or two).
>>>

I still don't understand why it would be beneficial to have the data in
resource files.  I also think that maybe it should be classified as an
enhancement that goes beyond the scope of this particular patch series.
If you would like to do this please feel free.  I've only got so many
hours in my day.

>From 71fd3e1a8490c038473e91fd521ced6661de56f6 Mon Sep 17 00:00:00 2001
From: Morgan Smith <[email-masked]>
Date: Fri, 21 Feb 2025 12:52:03 -0500
Subject: [PATCH v3 1/5] tests: Move helper and wrapper functions into their
 own file

* test/debbugs-test-helpers.el: New file.
* test/debbugs-tests.el: Move test helper and wrapper functions into the new
file and require it.
* Makefile(check): Load all files in the test directory.
---
 Makefile                     |   8 +-
 test/debbugs-test-helpers.el | 146 +++++++++++++++++++++++++++++++++++
 test/debbugs-tests.el        | 113 +--------------------------
 3 files changed, 154 insertions(+), 113 deletions(-)
 create mode 100644 test/debbugs-test-helpers.el

diff --git a/Makefile b/Makefile
index a7b6b0c7f3..81d7b5d2f2 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,9 @@
 EMACS ?= emacs
 MAKEINFO ?= makeinfo
 
+# regex of tests to run
+TESTS=.*
+
 SOURCE=$(wildcard *.el)
 TESTSOURCE=$(wildcard test/*.el)
 TARGET=$(filter-out debbugs-pkg.elc,$(patsubst %.el,%.elc,$(SOURCE)))
@@ -14,6 +17,9 @@ INFOMANUALS=debbugs.info debbugs-ug.info
 %.elc: %.el
 	@$(EMACS) -Q -batch -L . -f batch-byte-compile $<
 
+test/%.elc: test/%.el
+	@$(EMACS) -Q -batch -L . -L ./test -f batch-byte-compile $<
+
 %.info: %.texi
 	$(MAKEINFO) --error-limit=0 --no-split $< -o $@
 
@@ -24,7 +30,7 @@ doc: $(INFOMANUALS)
 build: $(TARGET)
 
 check: build $(TESTTARGET)
-	@$(EMACS) -Q --batch -L . -l $(TESTSOURCE) -f ert-run-tests-batch-and-exit
+	@$(EMACS) -Q --batch -L . -L ./test $(foreach file,$(TESTSOURCE), -l $(file)) --eval '(ert-run-tests-batch-and-exit "$(TESTS)")'
 
 clean:
 	-rm -f $(TARGET) $(TESTTARGET) $(INFOMANUALS)
diff --git a/test/debbugs-test-helpers.el b/test/debbugs-test-helpers.el
new file mode 100644
index 0000000000..f98bab40ed
--- /dev/null
+++ b/test/debbugs-test-helpers.el
@@ -0,0 +1,146 @@
+;;; debbugs-test-helpers.el --- Helpers for testing debbugs -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2024-2025 Free Software Foundation, Inc.
+
+;; Author: Morgan Smith <[email-masked]>
+;; Package: debbugs
+
+;; This file is not part of GNU Emacs.
+
+;; This program is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Please ensure tests don't actually make network calls.
+
+;;; Code:
+
+(require 'debbugs)
+
+;;; Helper Data:
+
+;; Generated using this:
+;; (soap-invoke debbugs-wsdl debbugs-port "get_status" [64064])
+(defconst debbugs-test--bug-status-soap-return
+  '(((item
+      (key . 64064)
+      (value
+       (package . "emacs") (found_date) (last_modified . 1689593050)
+       (affects) (date . 1686745022) (fixed_versions)
+       (originator . "Morgan Smith <[email-masked]>")
+       (blocks) (archived . 1) (found) (unarchived) (tags . "patch")
+       (severity . "normal") (location . "archive") (owner) (fixed)
+       (blockedby) (pending . "done") (keywords . "patch") (id . 64064)
+       (found_versions) (mergedwith) (summary) (forwarded)
+       (log_modified . 1689593050)
+       (done . "Michael Albinus <[email-masked]>")
+       (source . "unknown")
+       (msgid
+        . "<[email-masked]>")
+       (bug_num . 64064) (subject . "[PATCH 0/4] debbugs improvements")
+       (fixed_date)))))
+  "Mock result from `soap-invoke' for bug 64064.")
+
+;; Generated using this:
+;; (debbugs-get-status 64064)
+(defconst debbugs-test--bug-status
+  '(((cache_time . 5000) (source . "unknown") (unarchived)
+     (keywords "patch") (blocks) (pending . "done") (severity . "normal")
+     (done . "Michael Albinus <[email-masked]>") (location . "archive")
+     (log_modified . 1689593050) (subject . "[PATCH 0/4] debbugs improvements")
+     (last_modified . 1689593050) (found) (tags "patch") (package "emacs")
+     (originator . "Morgan Smith <[email-masked]>") (archived . t)
+     (blockedby) (affects) (mergedwith) (summary) (date . 1686745022)
+     (fixed_versions) (id . 64064) (fixed) (found_date) (forwarded)
+     (msgid
+      . "<[email-masked]>")
+     (owner) (found_versions) (fixed_date) (bug_num . 64064)))
+  "Mock result from `debbugs-get-status' for bug 64064.")
+
+;;; Helper Functions:
+
+(defvar debbugs-test--soap-operation-name nil)
+(defvar debbugs-test--soap-parameters nil)
+(defun debbugs-test--soap-invoke-internal
+    (callback _cbargs _wsdl _service operation-name &rest parameters)
+  "Over-ride for testing."
+  (setq debbugs-test--soap-operation-name operation-name)
+  (setq debbugs-test--soap-parameters parameters)
+  (let ((return
+         (cond ((string-equal operation-name "get_status")
+                debbugs-test--bug-status-soap-return)
+               ((string-equal operation-name "get_usertag")
+                '(((hi))))
+               (t '((0))))))
+    (if callback
+        (progn
+          (funcall callback return)
+          nil)
+      return)))
+
+(defun debbugs-test--override-float-time (func &rest rest)
+  "Override `float-time' for FUNC with args REST."
+  (cl-letf (((symbol-function #'float-time)
+             (lambda (&optional _specified-time) 5000)))
+    (apply func rest)))
+
+(defun debbugs-test--setup ()
+  "Mock network and time functions.
+These mock functions are needed to make the tests reproducible."
+  (setq debbugs-test--soap-operation-name nil)
+  (setq debbugs-test--soap-parameters nil)
+
+  (add-function
+   :override (symbol-function #'soap-invoke-internal)
+   #'debbugs-test--soap-invoke-internal)
+
+  (add-function
+   :around (symbol-function #'debbugs-get-cache)
+   #'debbugs-test--override-float-time)
+
+  (add-function
+   :around (symbol-function #'debbugs-put-cache)
+   #'debbugs-test--override-float-time))
+
+(defun debbugs-test--teardown ()
+  "Restore functions to as they where before."
+  (setq debbugs-test--soap-operation-name nil)
+  (setq debbugs-test--soap-parameters nil)
+
+  (remove-function
+   (symbol-function #'soap-invoke-internal)
+   #'debbugs-test--soap-invoke-internal)
+
+  (remove-function
+   (symbol-function #'debbugs-get-cache)
+   #'debbugs-test--override-float-time)
+
+  (remove-function
+   (symbol-function #'debbugs-put-cache)
+   #'debbugs-test--override-float-time))
+
+(defmacro ert-deftest--debbugs (name args docstring &rest body)
+  "The same as `ert-deftest' but runs setup and teardown functions."
+  (declare
+   (doc-string 3)
+   (indent 2))
+  `(ert-deftest ,name ,args ,docstring
+                (debbugs-test--setup)
+                ,@body
+                (debbugs-test--teardown)))
+
+
+(provide 'debbugs-test-helpers)
+
+;;; debbugs-test-helpers.el ends here
diff --git a/test/debbugs-tests.el b/test/debbugs-tests.el
index f3abea2837..da1b9dffe6 100644
--- a/test/debbugs-tests.el
+++ b/test/debbugs-tests.el
@@ -27,118 +27,7 @@
 ;;; Code:
 
 (require 'debbugs)
-
-;;; Helper Data:
-
-;; Generated using this:
-;; (soap-invoke debbugs-wsdl debbugs-port "get_status" [64064])
-(defconst debbugs-test--bug-status-soap-return
-  '(((item
-      (key . 64064)
-      (value
-       (package . "emacs") (found_date) (last_modified . 1689593050)
-       (affects) (date . 1686745022) (fixed_versions)
-       (originator . "Morgan Smith <[email-masked]>")
-       (blocks) (archived . 1) (found) (unarchived) (tags . "patch")
-       (severity . "normal") (location . "archive") (owner) (fixed)
-       (blockedby) (pending . "done") (keywords . "patch") (id . 64064)
-       (found_versions) (mergedwith) (summary) (forwarded)
-       (log_modified . 1689593050)
-       (done . "Michael Albinus <[email-masked]>")
-       (source . "unknown")
-       (msgid
-        . "<[email-masked]>")
-       (bug_num . 64064) (subject . "[PATCH 0/4] debbugs improvements")
-       (fixed_date)))))
-  "Mock result from `soap-invoke' for bug 64064.")
-
-;; Generated using this:
-;; (debbugs-get-status 64064)
-(defconst debbugs-test--bug-status
-  '(((cache_time . 5000) (source . "unknown") (unarchived)
-     (keywords "patch") (blocks) (pending . "done") (severity . "normal")
-     (done . "Michael Albinus <[email-masked]>") (location . "archive")
-     (log_modified . 1689593050) (subject . "[PATCH 0/4] debbugs improvements")
-     (last_modified . 1689593050) (found) (tags "patch") (package "emacs")
-     (originator . "Morgan Smith <[email-masked]>") (archived . t)
-     (blockedby) (affects) (mergedwith) (summary) (date . 1686745022)
-     (fixed_versions) (id . 64064) (fixed) (found_date) (forwarded)
-     (msgid
-      . "<[email-masked]>")
-     (owner) (found_versions) (fixed_date) (bug_num . 64064)))
-  "Mock result from `debbugs-get-status' for bug 64064.")
-
-;;; Helper Functions:
-
-(defvar debbugs-test--soap-operation-name nil)
-(defvar debbugs-test--soap-parameters nil)
-(defun debbugs-test--soap-invoke-internal
-    (callback _cbargs _wsdl _service operation-name &rest parameters)
-  "Over-ride for testing."
-  (setq debbugs-test--soap-operation-name operation-name)
-  (setq debbugs-test--soap-parameters parameters)
-  (let ((return
-         (cond ((string-equal operation-name "get_status")
-                debbugs-test--bug-status-soap-return)
-               ((string-equal operation-name "get_usertag")
-                '(((hi))))
-               (t '((0))))))
-    (if callback
-        (progn
-          (funcall callback return)
-          nil)
-      return)))
-
-(defun debbugs-test--override-float-time (func &rest rest)
-  "Override `float-time' for FUNC with args REST."
-  (cl-letf (((symbol-function #'float-time)
-             (lambda (&optional _specified-time) 5000)))
-    (apply func rest)))
-
-(defun debbugs-test--setup ()
-  "Mock network and time functions.
-These mock functions are needed to make the tests reproducible."
-  (setq debbugs-test--soap-operation-name nil)
-  (setq debbugs-test--soap-parameters nil)
-
-  (add-function
-   :override (symbol-function #'soap-invoke-internal)
-   #'debbugs-test--soap-invoke-internal)
-
-  (add-function
-   :around (symbol-function #'debbugs-get-cache)
-   #'debbugs-test--override-float-time)
-
-  (add-function
-   :around (symbol-function #'debbugs-put-cache)
-   #'debbugs-test--override-float-time))
-
-(defun debbugs-test--teardown ()
-  "Restore functions to as they where before."
-  (setq debbugs-test--soap-operation-name nil)
-  (setq debbugs-test--soap-parameters nil)
-
-  (remove-function
-   (symbol-function #'soap-invoke-internal)
-   #'debbugs-test--soap-invoke-internal)
-
-  (remove-function
-   (symbol-function #'debbugs-get-cache)
-   #'debbugs-test--override-float-time)
-
-  (remove-function
-   (symbol-function #'debbugs-put-cache)
-   #'debbugs-test--override-float-time))
-
-(defmacro ert-deftest--debbugs (name args docstring &rest body)
-  "The same as `ert-deftest' but runs setup and teardown functions."
-  (declare
-   (doc-string 3)
-   (indent 2))
-  `(ert-deftest ,name ,args ,docstring
-                (debbugs-test--setup)
-                ,@body
-                (debbugs-test--teardown)))
+(require 'debbugs-test-helpers)
 
 ;;; Tests:
 
-- 
2.48.1