GNU bug report logs - #72574
[PATCH 0/3] debbugs improvements. Add tests

Previous Next

Package: emacs;

Reported by: Morgan Smith <Morgan.J.Smith <at> outlook.com>

Date: Sun, 11 Aug 2024 12:17:02 UTC

Severity: normal

Tags: patch

Done: Michael Albinus <michael.albinus <at> gmx.de>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 72574 in the body.
You can then email your comments to 72574 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to michael.albinus <at> gmx.de, bug-gnu-emacs <at> gnu.org:
bug#72574; Package emacs. (Sun, 11 Aug 2024 12:17:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Morgan Smith <Morgan.J.Smith <at> outlook.com>:
New bug report received and forwarded. Copy sent to michael.albinus <at> gmx.de, bug-gnu-emacs <at> gnu.org. (Sun, 11 Aug 2024 12:17:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: bug-gnu-emacs <at> gnu.org
Cc: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Subject: [PATCH 0/3] debbugs improvements.  Add tests
Date: Sun, 11 Aug 2024 08:15:31 -0400
Hello!

I would like to start contributing some improvements to debbugs.  However, when
changing things I found it difficult to avoid regressions.  So I figured I'd
try to add a test suite to debbugs first.

The first two patches are not that interesting.  I was hoping to get feedback
on the "Add tests" patch.  Obviously more tests are needed but I would like to
know if I'm on the right path.

Thanks,

Morgan

Morgan Smith (3):
  * debbugs-gnu.el (debbugs-gnu-find-contributor): Make regex more
    specific
  Fix various warnings
  Add tests

 Makefile             |   4 ++
 debbugs-browse.el    |   3 +-
 debbugs-compat.el    |   3 ++
 debbugs-gnu.el       |  24 +++++-----
 debbugs-org.el       |   7 +--
 debbugs.el           |  12 ++---
 test/test-debbugs.el | 101 +++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 133 insertions(+), 21 deletions(-)
 create mode 100644 Makefile
 create mode 100644 test/test-debbugs.el

-- 
2.45.2





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72574; Package emacs. (Sun, 11 Aug 2024 12:20:02 GMT) Full text and rfc822 format available.

Message #8 received at 72574 <at> debbugs.gnu.org (full text, mbox):

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: 72574 <at> debbugs.gnu.org
Cc: Morgan Smith <Morgan.J.Smith <at> outlook.com>,
 Michael Albinus <michael.albinus <at> gmx.de>
Subject: [PATCH 1/3] * debbugs-gnu.el (debbugs-gnu-find-contributor): Make
 regex more specific
Date: Sun, 11 Aug 2024 08:18:39 -0400
---
 debbugs-gnu.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debbugs-gnu.el b/debbugs-gnu.el
index 3f5a9aebc8..6b6dd4992b 100644
--- a/debbugs-gnu.el
+++ b/debbugs-gnu.el
@@ -2702,7 +2702,7 @@ If SELECTIVELY, query the user before applying the patch."
   (let ((found 0)
 	(match (concat "^[0-9].*" string)))
     (dolist (file (directory-files-recursively
-		   debbugs-gnu-current-directory "ChangeLog\\(.[0-9]+\\)?$"))
+		   debbugs-gnu-current-directory "ChangeLog\\(\\.[0-9]+\\)?\\'"))
       (with-temp-buffer
 	(when (file-exists-p file)
 	  (insert-file-contents file))
-- 
2.45.2





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72574; Package emacs. (Sun, 11 Aug 2024 12:21:01 GMT) Full text and rfc822 format available.

Message #11 received at 72574 <at> debbugs.gnu.org (full text, mbox):

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: 72574 <at> debbugs.gnu.org
Cc: Morgan Smith <Morgan.J.Smith <at> outlook.com>,
 Michael Albinus <michael.albinus <at> gmx.de>
Subject: [PATCH 2/3] Fix various warnings
Date: Sun, 11 Aug 2024 08:18:40 -0400
* debbugs-browse.el:
* debbugs-compat.el:
* debbugs-gnu.el:
* debbugs-org.el:
* debbugs.el:
Fix various warnings and checkdoc warnings.
---
 debbugs-browse.el |  3 ++-
 debbugs-compat.el |  3 +++
 debbugs-gnu.el    | 22 +++++++++++-----------
 debbugs-org.el    |  7 ++++---
 debbugs.el        | 12 +++++++-----
 5 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/debbugs-browse.el b/debbugs-browse.el
index 3b439b08b5..900adec818 100644
--- a/debbugs-browse.el
+++ b/debbugs-browse.el
@@ -54,6 +54,7 @@ This can be either `debbugs-gnu-bugs' or `debbugs-org-bugs'."
 
 ;;;###autoload
 (defun debbugs-browse-url (url &optional _new-window)
+  "Browse bug at URL using debbugs."
   (when (and (stringp url)
 	     (string-match debbugs-browse-url-regexp url))
     (funcall debbugs-browse-function (string-to-number (match-string 3 url)))
@@ -67,7 +68,7 @@ This can be either `debbugs-gnu-bugs' or `debbugs-org-bugs'."
 
 ;;;###autoload
 (define-minor-mode debbugs-browse-mode
-  "Browse GNU Debbugs bug URLs with debbugs-gnu or debbugs-org.
+  "Browse GNU Debbugs bug URLs with `debbugs-gnu' or `debbugs-org'.
 With a prefix argument ARG, enable Debbugs Browse mode if ARG is
 positive, and disable it otherwise.  If called from Lisp, enable
 the mode if ARG is omitted or nil.
diff --git a/debbugs-compat.el b/debbugs-compat.el
index 29509fa6d2..96628a1921 100644
--- a/debbugs-compat.el
+++ b/debbugs-compat.el
@@ -21,6 +21,9 @@
 ;; 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:
+;;; Code:
+
 ;; Function `string-replace' is new in Emacs 28.1.
 (defalias 'debbugs-compat-string-replace
   (if (fboundp 'string-replace)
diff --git a/debbugs-gnu.el b/debbugs-gnu.el
index 6b6dd4992b..68599cff7f 100644
--- a/debbugs-gnu.el
+++ b/debbugs-gnu.el
@@ -33,7 +33,7 @@
 ;;
 ;;   M-x debbugs-gnu
 
-;; It asks for the severities, for which bugs shall be shown. This can
+;; It asks for the severities, for which bugs shall be shown.  This can
 ;; be either just one severity, or a list of severities, separated by
 ;; comma.  Valid severities are "serious", "important", "normal",
 ;; "minor" or "wishlist".  Severities "critical" and "grave" are not
@@ -243,7 +243,7 @@
 (defvar debbugs-gnu-limit)
 
 (defgroup debbugs-gnu ()
-  "UI for the debbugs.gnu.org bug tracker."
+  "UI for the debbugs bug tracker."
   :group 'debbugs
   :version "24.1")
 
@@ -426,7 +426,7 @@ They haven't been touched more than a week.")
 
 (defvar debbugs-gnu-persistency-file
   (expand-file-name (locate-user-emacs-file "debbugs"))
-  "File name of a persistency store for debbugs variables")
+  "File name of a persistency store for debbugs variables.")
 
 (defun debbugs-gnu-dump-persistency-file ()
   "Function to store debbugs variables persistently."
@@ -698,7 +698,7 @@ depend on PHRASE being a string, or nil.  See Info node
 
 ;;;###autoload
 (defun debbugs-gnu-package (&optional packages)
-  "List the bug reports of default packages, divided by severity."
+  "List the bug reports of PACKAGES or default packages, divided by severity."
   (interactive
      (list
       (if current-prefix-arg
@@ -2548,7 +2548,7 @@ If given a prefix, patch in the branch directory instead.
 If SELECTIVELY, query the user before applying the patch."
   (interactive "P")
   (unless (eq debbugs-gnu-mail-backend 'gnus)
-    (error "This function only works with Gnus."))
+    (error "This function only works with Gnus"))
   (add-hook 'diff-mode-hook #'debbugs-gnu-diff-mode)
   (debbugs-gnu-init-current-directory branch)
   (let ((rej (expand-file-name "debbugs-gnu.rej" temporary-file-directory))
@@ -2695,12 +2695,12 @@ If SELECTIVELY, query the user before applying the patch."
 		       nil t)))
     (forward-line 2)))
 
-(defun debbugs-gnu-find-contributor (string)
-  "Search through ChangeLogs to find contributors."
+(defun debbugs-gnu-find-contributor (contributor)
+  "Search through ChangeLogs to find CONTRIBUTOR."
   (interactive "sContributor match: ")
   (debbugs-gnu-init-current-directory)
   (let ((found 0)
-	(match (concat "^[0-9].*" string)))
+	(match (concat "^[0-9].*" contributor)))
     (dolist (file (directory-files-recursively
 		   debbugs-gnu-current-directory "ChangeLog\\(\\.[0-9]+\\)?\\'"))
       (with-temp-buffer
@@ -2710,7 +2710,7 @@ If SELECTIVELY, query the user before applying the patch."
 	(while (and (re-search-forward match nil t)
 		    (not (looking-at ".*tiny change")))
 	  (cl-incf found))))
-    (message "%s is a contributor %d times" string found)
+    (message "%s is a contributor %d times" contributor found)
     found))
 
 (defvar debbugs-gnu-patch-subject nil)
@@ -2719,7 +2719,7 @@ If SELECTIVELY, query the user before applying the patch."
   "Add a ChangeLog from a recently applied patch from a third party."
   (interactive)
   (unless (eq debbugs-gnu-mail-backend 'gnus)
-    (error "This function only works with Gnus."))
+    (error "This function only works with Gnus"))
   (let (from subject patch-subject changelog
 	     patch-from)
     (with-current-buffer gnus-article-buffer
@@ -2848,7 +2848,7 @@ If SELECTIVELY, query the user before applying the patch."
     map))
 
 (define-minor-mode debbugs-gnu-log-edit-mode
-  "Minor mode for providing a debbugs interface in log-edit buffers.
+  "Minor mode for providing a debbugs interface in `log-edit' buffers.
 
 \\{debbugs-gnu-log-edit-mode-map}"
   :lighter " Debbugs" :keymap debbugs-gnu-log-edit-mode-map)
diff --git a/debbugs-org.el b/debbugs-org.el
index c8a94c32dc..0a3a5b2a65 100644
--- a/debbugs-org.el
+++ b/debbugs-org.el
@@ -31,7 +31,7 @@
 ;;
 ;;   M-x debbugs-org
 
-;; It asks for the severities, for which bugs shall be shown. This can
+;; It asks for the severities, for which bugs shall be shown.  This can
 ;; be either just one severity, or a list of severities, separated by
 ;; comma.  Valid severities are "serious", "important", "normal",
 ;; "minor" or "wishlist".  Severities "critical" and "grave" are not
@@ -140,7 +140,7 @@
   "Mapping of debbugs severities to TODO priorities.")
 
 (defun debbugs-org-get-severity-priority (state)
-  "Returns the TODO priority of STATE."
+  "Return the TODO priority of STATE."
   (or (cdr (assoc (alist-get 'severity state) debbugs-org-severity-priority))
       (cdr (assoc "minor" debbugs-org-severity-priority))))
 
@@ -319,7 +319,7 @@ the corresponding buffer (e.g. by closing Emacs)."
 
 ;;;###autoload
 (define-minor-mode debbugs-org-mode
-  "Minor mode for providing a debbugs interface in org-mode buffers.
+  "Minor mode for providing a debbugs interface in `org-mode' buffers.
 
 \\{debbugs-org-mode-map}"
   :lighter " Debbugs" :keymap debbugs-org-mode-map
@@ -374,3 +374,4 @@ defined."
 ;; - Sort according to different TODO properties.
 
 (provide 'debbugs-org)
+;;; debbugs-org.el ends here
diff --git a/debbugs.el b/debbugs.el
index c90fb1008c..627cdf163d 100644
--- a/debbugs.el
+++ b/debbugs.el
@@ -44,7 +44,7 @@
 (eval-when-compile (require 'cl-lib))
 
 (defgroup debbugs nil
-  "Debbugs library"
+  "Debbugs library."
   :group 'hypermedia)
 
 (defcustom debbugs-servers
@@ -56,10 +56,10 @@
      :bugreport-url "https://bugs.debian.org/cgi-bin/bugreport.cgi"))
   "*List of Debbugs server specifiers.
 Each entry is a list that contains a string identifying the port
-name and the server parameters in keyword-value form. Allowed
+name and the server parameters in keyword-value form.  Allowed
 keywords are:
 
-`:wsdl' -- Location of WSDL. The value is a string with URL that
+`:wsdl' -- Location of WSDL.  The value is a string with URL that
 should return the WSDL specification of Debbugs/SOAP service.
 
 `:bugreport-url' -- URL of the server script that returns mboxes
@@ -121,7 +121,9 @@ t or 0 disables caching, nil disables expiring."
   "The object manipulated by `debbugs-soap-invoke-async'.")
 
 (defun debbugs-soap-invoke-async (operation-name &rest parameters)
-  "Invoke the SOAP connection asynchronously."
+  "Invoke the SOAP connection asynchronously.
+
+OPERATION-NAME and PARAMETERS are as described in `soap-invoke'."
   (apply
    #'soap-invoke-async
    (lambda (response &rest _args)
@@ -381,7 +383,7 @@ Every returned entry is an association list with the following attributes:
 
   `package': A list of package names the bug belongs to.
 
-  `severity': The severity of the bug report. This can be
+  `severity': The severity of the bug report.  This can be
   \"critical\", \"grave\", \"serious\", \"important\",
   \"normal\", \"minor\" or \"wishlist\".
 
-- 
2.45.2





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72574; Package emacs. (Sun, 11 Aug 2024 12:21:02 GMT) Full text and rfc822 format available.

Message #14 received at 72574 <at> debbugs.gnu.org (full text, mbox):

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: 72574 <at> debbugs.gnu.org
Cc: Morgan Smith <Morgan.J.Smith <at> outlook.com>,
 Michael Albinus <michael.albinus <at> gmx.de>
Subject: [PATCH 3/3] Add tests
Date: Sun, 11 Aug 2024 08:18:41 -0400
* Makefile: New file.
* test/test-debbugs.el: New file.
---
 Makefile             |   4 ++
 test/test-debbugs.el | 101 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+)
 create mode 100644 Makefile
 create mode 100644 test/test-debbugs.el

diff --git a/Makefile b/Makefile
new file mode 100644
index 0000000000..3a4b06a76e
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,4 @@
+.PHONY: check
+
+check:
+	emacs -Q --batch -L . -l test/* -f ert-run-tests-batch-and-exit
diff --git a/test/test-debbugs.el b/test/test-debbugs.el
new file mode 100644
index 0000000000..8eca3fe3a8
--- /dev/null
+++ b/test/test-debbugs.el
@@ -0,0 +1,101 @@
+;;; test-debbugs.el --- tests for debbugs.el -*- lexical-binding: t; -*-
+
+;;; 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])
+(defvar test-bug-status-soap-return
+  '(((item (key . 64064)
+        (value (package . "emacs") (found_date) (last_modified . 1689593050)
+               (affects) (date . 1686745022) (fixed_versions)
+               (originator . "Morgan Smith <Morgan.J.Smith <at> outlook.com>")
+               (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 <michael.albinus <at> gmx.de>")
+               (source . "unknown")
+               (msgid
+                . "<DM5PR03MB31632E3A4FE170C62E7D4B0CC55AA <at> DM5PR03MB3163.namprd03.prod.outlook.com>")
+               (bug_num . 64064) (subject . "[PATCH 0/4] debbugs improvements")
+               (fixed_date))))))
+
+;; Generated using this:
+;; (debbugs-get-status 64064)
+(defvar test-bug-status
+  '(((cache_time . 5000) (source . "unknown") (unarchived)
+     (keywords "patch") (blocks) (pending . "done") (severity . "normal") (found)
+     (done . "Michael Albinus <michael.albinus <at> gmx.de>") (location . "archive")
+     (log_modified . 1689593050) (subject . "[PATCH 0/4] debbugs improvements")
+     (last_modified . 1689593050)
+     (originator . "Morgan Smith <Morgan.J.Smith <at> outlook.com>") (archived . t)
+     (blockedby) (affects) (mergedwith) (summary) (date . 1686745022)
+     (fixed_versions) (id . 64064) (fixed) (found_date) (forwarded) (tags "patch")
+     (msgid
+      . "<DM5PR03MB31632E3A4FE170C62E7D4B0CC55AA <at> DM5PR03MB3163.namprd03.prod.outlook.com>")
+     (owner) (found_versions) (fixed_date) (bug_num . 64064) (package "emacs"))))
+
+;;; Helper Functions:
+
+(defvar test-soap-operation-name nil)
+(defvar test-soap-parameters nil)
+(defun soap-invoke-internal (callback _cbargs _wsdl _service operation-name &rest parameters)
+  "Over-ride for testing"
+  (setq test-soap-operation-name operation-name)
+  (setq test-soap-parameters parameters)
+  (let ((return
+         (cond ((string-equal operation-name "get_status") test-bug-status-soap-return)
+               ((string-equal operation-name "get_usertag") '(((hi))))
+               (t '((0))))))
+    (if callback
+        (progn
+          (funcall callback return)
+          nil)
+      return)))
+
+;;; Tests:
+
+(ert-deftest test-debbugs-get-bugs ()
+  (let (test-soap-operation-name test-soap-parameters)
+    (debbugs-get-bugs
+     :tag "patch"
+     :severity "critical"
+     :status "open"
+     :status "forwarded")
+    (should (string-equal test-soap-operation-name "get_bugs"))
+    (should (equal test-soap-parameters '(["tag" "patch" "severity" "critical"
+                                           "status" "open" "status" "forwarded"])))))
+
+(ert-deftest test-debbugs-newest-bugs ()
+  (let (test-soap-operation-name test-soap-parameters)
+    (debbugs-newest-bugs 4)
+    (should (string-equal test-soap-operation-name "newest_bugs"))
+    (should (equal test-soap-parameters '(4)))))
+
+(ert-deftest test-debbugs-get-status ()
+  (let ((original-float-time (symbol-function 'float-time))
+        test-soap-operation-name test-soap-parameters)
+    (fset 'float-time (lambda (&optional _specified-time) 5000))
+    (should (= (float-time) 5000))
+    (should (equal (sort (car (debbugs-get-status 64064))) (sort (car test-bug-status))))
+    (should (string-equal test-soap-operation-name "get_status"))
+    (should (equal test-soap-parameters '([64064])))
+    (fset 'float-time original-float-time)))
+
+(ert-deftest test-debbugs-get-usertag ()
+  (let (test-soap-operation-name test-soap-parameters)
+    (should (equal (debbugs-get-usertag :user "emacs") '("hi")))
+    (should (string-equal test-soap-operation-name "get_usertag"))
+    (should (equal test-soap-parameters '("emacs")))))
+
+(provide 'test-debbugs)
+;;; test-debbugs.el ends here
-- 
2.45.2





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72574; Package emacs. (Sun, 11 Aug 2024 12:40:01 GMT) Full text and rfc822 format available.

Message #17 received at 72574 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Cc: 72574 <at> debbugs.gnu.org
Subject: Re: bug#72574: [PATCH 0/3] debbugs improvements.  Add tests
Date: Sun, 11 Aug 2024 14:38:25 +0200
Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:

> Hello!

Hi Morgan,

Great to see you, again!

> I would like to start contributing some improvements to debbugs.  However, when
> changing things I found it difficult to avoid regressions.  So I figured I'd
> try to add a test suite to debbugs first.
>
> The first two patches are not that interesting.

I've scanned them rougly, and they seem to be OK. Will commit them later
today, with a more precise reading.

> I was hoping to get feedback on the "Add tests" patch.  Obviously more
> tests are needed but I would like to know if I'm on the right path.

It looks like the right path, yes. Will comment this patch.

> Thanks,
>
> Morgan

Best regards, Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72574; Package emacs. (Sun, 11 Aug 2024 13:55:02 GMT) Full text and rfc822 format available.

Message #20 received at 72574 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Cc: 72574 <at> debbugs.gnu.org
Subject: Re: [PATCH 3/3] Add tests
Date: Sun, 11 Aug 2024 15:53:20 +0200
Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:

Hi Morgan,

here are my comments:

> * test/test-debbugs.el: New file.

I propose to follow Emacs naming conventions. A file PACKAGE.el has a
corresponding file PACKAGE-tests.el. So we might call the file now
debbugs-tests.el. If more test functions arrive, we might split this
file into the different debbugs-*-tests.el files.

> +++ b/Makefile
> +.PHONY: check
> +
> +check:
> +	emacs -Q --batch -L . -l test/* -f ert-run-tests-batch-and-exit

Well, you run the tests on the source file. Usually, batch tests are
run on compiled files. So you might add rules for compiling the
debbugs*.el, and the test/debbugs-tests*.el files first.

You call the program 'emacs', hardcoded. This is OK as default, but
people (me!) might want to test with different Emacs versions on the
same machine. Please add a variable 'EMACS ?= emacs', and use it.

Don't use '-l test/*', better is -l 'test/*.el'. Perhaps you want to add
something else in that directory, like a README?

> +++ b/test/test-debbugs.el
> +;;; test-debbugs.el --- tests for debbugs.el -*- lexical-binding: t; -*-

The Copyright notice and the GNU GPL text is missing. And also the
Author: and Package: header lines.

> +;; Generated using this:
> +;; (soap-invoke debbugs-wsdl debbugs-port "get_status" [64064])
> +(defvar test-bug-status-soap-return

Please prefix helper objects with 'debbugs-test--'. Yes, two hyphens,
'cos they are helpers.

It has a fixed value, so it might be a defconst.

> +;; Generated using this:
> +;; (debbugs-get-status 64064)
> +(defvar test-bug-status

Same comments as above.

> +(defvar test-soap-operation-name nil)
> +(defvar test-soap-parameters nil)

Please adapt the names.

> +(defun soap-invoke-internal (callback _cbargs _wsdl _service operation-name &rest parameters)
> +  "Over-ride for testing"
> +  (setq test-soap-operation-name operation-name)
> +  (setq test-soap-parameters parameters)
> +  (let ((return
> +         (cond ((string-equal operation-name "get_status") test-bug-status-soap-return)
> +               ((string-equal operation-name "get_usertag") '(((hi))))
> +               (t '((0))))))
> +    (if callback
> +        (progn
> +          (funcall callback return)
> +          nil)
> +      return)))

Don't override the function this way. It is better to write a helper
function debbugs-test--soap-invoke-internal, and to override the
original function with something like

--8<---------------cut here---------------start------------->8---
(add-function
 :override (symbol-function #'soap-invoke-internal)
 #'debbugs-test--soap-invoke-internal)
--8<---------------cut here---------------end--------------->8---

Another technique is to use cl-letf, see example below.

> +(ert-deftest test-debbugs-get-bugs ()

Please rename all tests to debbugs-test-*. This is not formalized in the
Emacs source tree, but good practice.

> +(ert-deftest test-debbugs-get-status ()
> +  (let ((original-float-time (symbol-function 'float-time))
> +        test-soap-operation-name test-soap-parameters)
> +    (fset 'float-time (lambda (&optional _specified-time) 5000))
> +    (should (= (float-time) 5000))
> +    (should (equal (sort (car (debbugs-get-status 64064))) (sort (car test-bug-status))))
> +    (should (string-equal test-soap-operation-name "get_status"))
> +    (should (equal test-soap-parameters '([64064])))
> +    (fset 'float-time original-float-time)))

Please respect max line width of 80.

Please use cl-letf instead of fset. Something like

--8<---------------cut here---------------start------------->8---
(ert-deftest debbugs-test-get-status ()
  (cl-letf (((symbol-function #'float-time)
	     (lambda (&optional _specified-time) 5000)))
    (let (test-soap-operation-name test-soap-parameters)
      (should (= (float-time) 5000))
      (should (equal (sort (car (debbugs-get-status 64064)))
                     (sort (car test-bug-status))))
      (should (string-equal test-soap-operation-name "get_status"))
      (should (equal test-soap-parameters '([64064]))))))
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72574; Package emacs. (Sun, 11 Aug 2024 13:55:02 GMT) Full text and rfc822 format available.

Message #23 received at 72574 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Cc: 72574 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/3] * debbugs-gnu.el (debbugs-gnu-find-contributor):
 Make regex more specific
Date: Sun, 11 Aug 2024 15:54:03 +0200
Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:

> ---
>  debbugs-gnu.el | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

pushed.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72574; Package emacs. (Sun, 11 Aug 2024 13:56:02 GMT) Full text and rfc822 format available.

Message #26 received at 72574 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Cc: 72574 <at> debbugs.gnu.org
Subject: Re: [PATCH 2/3] Fix various warnings
Date: Sun, 11 Aug 2024 15:54:33 +0200
Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:

> * debbugs-browse.el:
> * debbugs-compat.el:
> * debbugs-gnu.el:
> * debbugs-org.el:
> * debbugs.el:
> Fix various warnings and checkdoc warnings.
> ---
>  debbugs-browse.el |  3 ++-
>  debbugs-compat.el |  3 +++
>  debbugs-gnu.el    | 22 +++++++++++-----------
>  debbugs-org.el    |  7 ++++---
>  debbugs.el        | 12 +++++++-----
>  5 files changed, 27 insertions(+), 20 deletions(-)

Pushed.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72574; Package emacs. (Sun, 11 Aug 2024 14:17:02 GMT) Full text and rfc822 format available.

Message #29 received at 72574 <at> debbugs.gnu.org (full text, mbox):

From: Morgan Smith <morgan.j.smith <at> outlook.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 72574 <at> debbugs.gnu.org
Subject: Re: [PATCH 3/3] Add tests
Date: Sun, 11 Aug 2024 10:15:26 -0400
Hi Michael,

I really appreciate you taking the time let me know all of this.  It 
would have taken me a while to figure out all of the best practices on 
my own.  This saves me a lot of time!  I'll get you an updated patch soon.

Thank you!

Morgan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72574; Package emacs. (Sun, 11 Aug 2024 22:10:02 GMT) Full text and rfc822 format available.

Message #32 received at 72574 <at> debbugs.gnu.org (full text, mbox):

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: 72574 <at> debbugs.gnu.org
Cc: Morgan Smith <Morgan.J.Smith <at> outlook.com>,
 Michael Albinus <michael.albinus <at> gmx.de>
Subject: [PATCH v2] Add tests
Date: Sun, 11 Aug 2024 18:05:30 -0400
* Makefile: New file.
* test/test-debbugs.el: New file.
---

I made all the changes you requested.

I hope you like it!

 Makefile             |  20 +++++++
 test/test-debbugs.el | 132 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 152 insertions(+)
 create mode 100644 Makefile
 create mode 100644 test/test-debbugs.el

diff --git a/Makefile b/Makefile
new file mode 100644
index 0000000000..86cc575a2b
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,20 @@
+EMACS ?= emacs
+
+SOURCE=$(wildcard *.el)
+TESTSOURCE=$(wildcard test/*.el)
+TARGET=$(patsubst %.el,%.elc,$(SOURCE))
+TESTTARGET=$(patsubst %.el,%.elc,$(TESTSOURCE))
+
+.PHONY: check clean
+.PRECIOUS: %.elc
+
+%.elc: %.el
+	@$(EMACS) -Q -batch -L . -f batch-byte-compile $<
+
+build: $(TARGET)
+
+check: build $(TESTTARGET)
+	emacs -Q --batch -L . -l $(TESTSOURCE) -f ert-run-tests-batch-and-exit
+
+clean:
+	-rm -f $(TARGET) $(TESTTARGET)
diff --git a/test/test-debbugs.el b/test/test-debbugs.el
new file mode 100644
index 0000000000..e22aadc14d
--- /dev/null
+++ b/test/test-debbugs.el
@@ -0,0 +1,132 @@
+;;; debbugs-tests.el --- tests for debbugs.el -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2024 Free Software Foundation, Inc.
+
+;; Author: Morgan Smith <Morgan.J.Smith <at> outlook.com>
+;; 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 <Morgan.J.Smith <at> outlook.com>")
+       (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 <michael.albinus <at> gmx.de>")
+       (source . "unknown")
+       (msgid
+        . "<DM5PR03MB31632E3A4FE170C62E7D4B0CC55AA <at> DM5PR03MB3163.namprd03.prod.outlook.com>")
+       (bug_num . 64064) (subject . "[PATCH 0/4] debbugs improvements")
+       (fixed_date))))))
+
+;; 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 <michael.albinus <at> gmx.de>") (location . "archive")
+     (log_modified . 1689593050) (subject . "[PATCH 0/4] debbugs improvements")
+     (last_modified . 1689593050) (found) (tags "patch") (package "emacs")
+     (originator . "Morgan Smith <Morgan.J.Smith <at> outlook.com>") (archived . t)
+     (blockedby) (affects) (mergedwith) (summary) (date . 1686745022)
+     (fixed_versions) (id . 64064) (fixed) (found_date) (forwarded)
+     (msgid
+      . "<DM5PR03MB31632E3A4FE170C62E7D4B0CC55AA <at> DM5PR03MB3163.namprd03.prod.outlook.com>")
+     (owner) (found_versions) (fixed_date) (bug_num . 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)))
+
+(add-function
+ :override (symbol-function #'soap-invoke-internal)
+ #'debbugs-test--soap-invoke-internal)
+
+;;; Tests:
+
+(ert-deftest debbugs-test-get-bugs ()
+  (let (debbugs-test--soap-operation-name debbugs-test--soap-parameters)
+    (debbugs-get-bugs
+     :tag "patch"
+     :severity "critical"
+     :status "open"
+     :status "forwarded")
+    (should (string-equal debbugs-test--soap-operation-name "get_bugs"))
+    (should (equal debbugs-test--soap-parameters
+                   '(["tag" "patch" "severity" "critical"
+                      "status" "open" "status" "forwarded"])))))
+
+(ert-deftest debbugs-test-newest-bugs ()
+  (let (debbugs-test--soap-operation-name debbugs-test--soap-parameters)
+    (debbugs-newest-bugs 4)
+    (should (string-equal debbugs-test--soap-operation-name "newest_bugs"))
+    (should (equal debbugs-test--soap-parameters '(4)))))
+
+(ert-deftest debbugs-test-get-status ()
+  (let (debbugs-test--soap-operation-name debbugs-test--soap-parameters)
+    (cl-letf (((symbol-function #'float-time)
+               (lambda (&optional _specified-time) 5000)))
+      (should (= (float-time) 5000))
+      (should (equal (sort (car (debbugs-get-status 64064)))
+                     (sort (car debbugs-test--bug-status))))
+      (should (string-equal debbugs-test--soap-operation-name "get_status"))
+      (should (equal debbugs-test--soap-parameters '([64064]))))))
+
+(ert-deftest debbugs-test-get-usertag ()
+  (let (debbugs-test--soap-operation-name debbugs-test--soap-parameters)
+    (should (equal (debbugs-get-usertag :user "emacs") '("hi")))
+    (should (string-equal debbugs-test--soap-operation-name "get_usertag"))
+    (should (equal debbugs-test--soap-parameters '("emacs")))))
+
+(provide 'debbugs-tests)
+;;; debbugs-tests.el ends here
-- 
2.45.2





Reply sent to Michael Albinus <michael.albinus <at> gmx.de>:
You have taken responsibility. (Mon, 12 Aug 2024 09:59:01 GMT) Full text and rfc822 format available.

Notification sent to Morgan Smith <Morgan.J.Smith <at> outlook.com>:
bug acknowledged by developer. (Mon, 12 Aug 2024 09:59:02 GMT) Full text and rfc822 format available.

Message #37 received at 72574-done <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Cc: 72574-done <at> debbugs.gnu.org
Subject: Re: [PATCH v2] Add tests
Date: Mon, 12 Aug 2024 11:57:26 +0200
Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:

Hi Morgan,

> * Makefile: New file.
> * test/test-debbugs.el: New file.
> ---
>
> I made all the changes you requested.
>
> I hope you like it!

Yes, thank you! I've pushed them to the ELPA repository. I made also some
small changes afterwards, nothing serious.

Closing the bug.

Best regards, Michael.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 09 Sep 2024 11:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 343 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.