GNU bug report logs - #76446
[Patch Debbugs] Infrastructure Improvements

Previous Next

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

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

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
Subject: [Patch Debbugs] Infrastructure Improvements
Date: Thu, 20 Feb 2025 14:01:03 -0500

[Message part 1 (text/plain, inline)]

Hello!

Nothing too exciting in this series.  Just adjusting things so we can
split the tests into multiple files.  Also adding a makefile target to
run the "checkdoc" stuff with the spell checker enabled.

Thanks,

Morgan

[0001-tests-Move-helper-and-wrapper-functions-into-their-o.patch (text/x-patch, attachment)]

[0002-tests-Add-tests-for-debbugs-gnu.patch (text/x-patch, attachment)]

[0003-Makefile-Add-target-to-run-checkdoc-on-all-files.patch (text/x-patch, attachment)]

Message #8 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [Patch Debbugs] Infrastructure Improvements
Date: Fri, 21 Feb 2025 12:41:14 +0100

Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:

> Hello!

Hi Morgan,

> Nothing too exciting in this series.  Just adjusting things so we can
> split the tests into multiple files.  Also adding a makefile target to
> run the "checkdoc" stuff with the spell checker enabled.

Thanks for this! I'm happy that you's started to provide also a
debbugs-gnu-tests.el file.

I didn't test the patches yet, starting with my comments on the code
reading first. See next messages.

> Thanks,
>
> Morgan

Best regards, Michael.

Message #11 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446:
 0001-tests-Move-helper-and-wrapper-functions-into-their-o.patch
Date: Fri, 21 Feb 2025 12:52:17 +0100

Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:

> --- a/Makefile
> +++ b/Makefile
> @@ -24,7 +24,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 . $(foreach file,$(TESTSOURCE), -l $(file)) -f ert-run-tests-batch-and-exit

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---

> --- /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).

> +(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)))

In order to ensure that debbugs-test--teardown is called even in case of
errors, we could use

(unwind-protect
    (progn
      (debbugs-test--setup)
      ,@body)
  (debbugs-test--teardown))

> --- a/test/debbugs-tests.el
> +++ b/test/debbugs-tests.el
> @@ -27,118 +27,7 @@
>  ;;; Code:
>
>  (require 'debbugs)
> +(require 'debbugs-test-helpers "./test/debbugs-test-helpers")

(require 'debbugs-test-helpers) should be sufficient with the proposed
change in the Makefile.

Best regards, Michael.

Message #14 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: 0002-tests-Add-tests-for-debbugs-gnu.patch
Date: Fri, 21 Feb 2025 12:56:43 +0100

Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:

> --- /dev/null
> +++ b/test/debbugs-gnu-tests.el
> +(require 'debbugs)
> +(require 'debbugs-gnu)
> +(require 'debbugs-test-helpers "./test/debbugs-test-helpers")

(require 'debbugs-test-helpers) is sufficient.

> +(ert-deftest--debbugs debbugs-test-debbugs-gnu-search ()
> +  "Test `debbugs-gnu-search'.

Isn't it rather a test of `debbugs-gnu'?

> +Also test `debbugs-gnu-patches' and `debbugs-gnu-tagged' which are
> +simple wrappers for `debbugs-gnu-search'."

Nope. They are wrappers of `debbugs-gnu'.

Best regards, Michael.

Message #17 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446:
 0003-Makefile-Add-target-to-run-checkdoc-on-all-files.patch
Date: Fri, 21 Feb 2025 13:02:20 +0100

Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:

> --- /dev/null
> +++ b/.dir-locals.el
> @@ -0,0 +1,18 @@
> +;;; Directory Local Variables            -*- no-byte-compile: t -*-
> +;;; For more information see (info "(emacs) Directory Variables")
> +
> +((emacs-lisp-mode . ((checkdoc-package-keywords-flag . t)
> +                     (checkdoc-ispell-lisp-words
> +                      .
> +                      ("ChangeLog" "ChangeLogs" "UTF" "alist" "args"
> +                       "armstrong" "backend" "bcc" "bugreport" "cdate" "cedet"
> +                       "coreutils" "cygwin" "debbugs" "debian" "el" "emacs"
> +                       "etags" "freemail" "fsf" "guix" "gw" "henoch" "hu"
> +                       "hyperestraier" "keymap" "magit" "magnus" "maint"
> +                       "maintainer" "maintainer's" "mbox" "mboxes" "minibuffer"
> +                       "moreinfo" "multibyte" "notabug" "paren" "persistency"
> +                       "regexp" "rescan" "rgm" "rmail" "severities" "sexp"
> +                       "solaris" "src" "sublist" "submitter" "submitter's"
> +                       "subproduct" "subqueries" "subquery" "teardown"
> +                       "unarchived" "unibyte" "unreproducible" "url" "util"
> +                       "wishlist" "wontfix" "wsdl" "www" "xsd" "zltuz")))))

This is needed for the checkdoc target in make only, isn't it? Shouldn't
we activate it for just this case?

> --- a/Makefile
> +++ b/Makefile
> +checkdoc: $(SOURCE) $(TESTSOURCE)
> +	@$(EMACS) -Q --batch "--eval=(setq enable-local-variables :all \
> +                                       checkdoc-spellcheck-documentation-flag t)" \
> +    -L . $(foreach file,$^,"--eval=(checkdoc-file \"$(file)\")")

Do we need '-L .'?

Best regards, Michael.

Message #20 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446:
 0001-tests-Move-helper-and-wrapper-functions-into-their-o.patch
Date: Fri, 21 Feb 2025 13:08:40 -0500

[Message part 1 (text/plain, inline)]

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

> Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:
>
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -24,7 +24,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 . $(foreach file,$(TESTSOURCE), -l $(file)) -f ert-run-tests-batch-and-exit
>
> 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 ..."

>> --- /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 read the info manual and source code pertaining to 'ert-resource' and
I fail to see any benefit from using it.

>> +(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)))
>
> In order to ensure that debbugs-test--teardown is called even in case of
> errors, we could use
>
> (unwind-protect
>     (progn
>       (debbugs-test--setup)
>       ,@body)
>   (debbugs-test--teardown))
>

Good catch!  Thank you!  I don't want to move things and change them at
the same time so now this is two patches.

>> --- a/test/debbugs-tests.el
>> +++ b/test/debbugs-tests.el
>> @@ -27,118 +27,7 @@
>>  ;;; Code:
>>
>>  (require 'debbugs)
>> +(require 'debbugs-test-helpers "./test/debbugs-test-helpers")
>
> (require 'debbugs-test-helpers) should be sufficient with the proposed
> change in the Makefile.

Done!

>
> Best regards, Michael.

[v2-0001-tests-Move-helper-and-wrapper-functions-into-thei.patch (text/x-patch, attachment)]

[v2-0002-tests-Use-unwind-protect-to-ensure-test-cleanup-a.patch (text/x-patch, attachment)]

Message #23 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: 0002-tests-Add-tests-for-debbugs-gnu.patch
Date: Fri, 21 Feb 2025 13:11:51 -0500

[Message part 1 (text/plain, inline)]

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

> Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:
>
>> --- /dev/null
>> +++ b/test/debbugs-gnu-tests.el
>> +(require 'debbugs)
>> +(require 'debbugs-gnu)
>> +(require 'debbugs-test-helpers "./test/debbugs-test-helpers")
>
> (require 'debbugs-test-helpers) is sufficient.
>

Done!

>> +(ert-deftest--debbugs debbugs-test-debbugs-gnu-search ()
>> +  "Test `debbugs-gnu-search'.
>
> Isn't it rather a test of `debbugs-gnu'?
>

Nope! The intention is to test `debbugs-gnu-search'.  In fact I stub out
`debbugs-gnu' with #'list so that I can test how `debbugs-gnu is
being calling by `debbugs-gnu-search''.

>> +Also test `debbugs-gnu-patches' and `debbugs-gnu-tagged' which are
>> +simple wrappers for `debbugs-gnu-search'."
>
> Nope. They are wrappers of `debbugs-gnu'.

Oops!  You're right.  I have removed those tests.

> Best regards, Michael.

[v2-0003-tests-Add-tests-for-debbugs-gnu.patch (text/x-patch, attachment)]

Message #26 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446:
 0003-Makefile-Add-target-to-run-checkdoc-on-all-files.patch
Date: Fri, 21 Feb 2025 13:15:53 -0500

[Message part 1 (text/plain, inline)]

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

> Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:
>
>> --- /dev/null
>> +++ b/.dir-locals.el
>> @@ -0,0 +1,18 @@
>> +;;; Directory Local Variables            -*- no-byte-compile: t -*-
>> +;;; For more information see (info "(emacs) Directory Variables")
>> +
>> +((emacs-lisp-mode . ((checkdoc-package-keywords-flag . t)
>> +                     (checkdoc-ispell-lisp-words
>> +                      .
>> +                      ("ChangeLog" "ChangeLogs" "UTF" "alist" "args"
>> +                       "armstrong" "backend" "bcc" "bugreport" "cdate" "cedet"
>> +                       "coreutils" "cygwin" "debbugs" "debian" "el" "emacs"
>> +                       "etags" "freemail" "fsf" "guix" "gw" "henoch" "hu"
>> +                       "hyperestraier" "keymap" "magit" "magnus" "maint"
>> +                       "maintainer" "maintainer's" "mbox" "mboxes" "minibuffer"
>> +                       "moreinfo" "multibyte" "notabug" "paren" "persistency"
>> +                       "regexp" "rescan" "rgm" "rmail" "severities" "sexp"
>> +                       "solaris" "src" "sublist" "submitter" "submitter's"
>> +                       "subproduct" "subqueries" "subquery" "teardown"
>> +                       "unarchived" "unibyte" "unreproducible" "url" "util"
>> +                       "wishlist" "wontfix" "wsdl" "www" "xsd" "zltuz")))))
>
> This is needed for the checkdoc target in make only, isn't it? Shouldn't
> we activate it for just this case?
>

Ya this seems a little inelegant.  See the attached patch for an attempt
to do this a different way.

>> --- a/Makefile
>> +++ b/Makefile
>> +checkdoc: $(SOURCE) $(TESTSOURCE)
>> +	@$(EMACS) -Q --batch "--eval=(setq enable-local-variables :all \
>> +                                       checkdoc-spellcheck-documentation-flag t)" \
>> +    -L . $(foreach file,$^,"--eval=(checkdoc-file \"$(file)\")")
>
> Do we need '-L .'?

We do not!

>
> Best regards, Michael.

[v2-0004-Makefile-Add-target-to-run-checkdoc-on-all-files.patch (text/x-patch, attachment)]

Message #29 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [Patch Debbugs] Infrastructure Improvements
Date: Fri, 21 Feb 2025 13:20:41 -0500

[Message part 1 (text/plain, inline)]

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

> Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:
>
>> Hello!
>
> Hi Morgan,
>
>> Nothing too exciting in this series.  Just adjusting things so we can
>> split the tests into multiple files.  Also adding a makefile target to
>> run the "checkdoc" stuff with the spell checker enabled.
>
> Thanks for this! I'm happy that you's started to provide also a
> debbugs-gnu-tests.el file.
>
> I didn't test the patches yet, starting with my comments on the code
> reading first. See next messages.
>

Thanks for reviewing my patches!  I appreciate the insightful comments
you've provided.

I've attached the relevant patches to the other email threads but I'll
dump them all in this email as well in case that's easier.  You'll
notice I've also sneakily included an extra 5th patch here :).

>> Thanks,
>>
>> Morgan
>
> Best regards, Michael.

[v2-0001-tests-Move-helper-and-wrapper-functions-into-thei.patch (text/x-patch, attachment)]

[v2-0002-tests-Use-unwind-protect-to-ensure-test-cleanup-a.patch (text/x-patch, attachment)]

[v2-0003-tests-Add-tests-for-debbugs-gnu.patch (text/x-patch, attachment)]

[v2-0004-Makefile-Add-target-to-run-checkdoc-on-all-files.patch (text/x-patch, attachment)]

[v2-0005-tests-Support-older-Emacs-sorting-conventions.patch (text/x-patch, attachment)]

Message #32 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [PATCH v2 1/5] tests: Move helper and wrapper
 functions into their own file
Date: Sat, 22 Feb 2025 16:56:09 +0100

Morgan Smith <morgan.j.smith <at> outlook.com> 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.

>>> --- /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 read the info manual and source code pertaining to 'ert-resource' and
> I fail to see any benefit from using it.

Both defconsts contain data. Those data belong into a ressource file,
and shall be read from there. Something like

--8<---------------cut here---------------start------------->8---
;; -*- lisp-data -*-
(((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)))
--8<---------------cut here---------------end--------------->8---

With this, we could reuse tests with the sam logic, but different data
files. In the future.

> --- /dev/null
> +++ b/test/debbugs-test-helpers.el
> +(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)))))
> +  "Mock result from `soap-invoke' for bug 64064.")

Read from ressource file.

> +(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)))
> +  "Mock result from `debbugs-get-status' for bug 64064.")

Read from ressource file.

Message #35 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [PATCH v2 3/5] tests: Add tests for debbugs-gnu
Date: Sat, 22 Feb 2025 16:59:52 +0100

Morgan Smith <morgan.j.smith <at> outlook.com> writes:

>>> +Also test `debbugs-gnu-patches' and `debbugs-gnu-tagged' which are
>>> +simple wrappers for `debbugs-gnu-search'."
>>
>> Nope. They are wrappers of `debbugs-gnu'.
>
> Oops!  You're right.  I have removed those tests.

Perhaps you can give them own tests? In the futeure?

> --- /dev/null
> +++ b/test/debbugs-gnu-tests.el
> +(require 'debbugs)
> +(require 'debbugs-gnu)

You don't need to require 'debbugs'; debbugs-gnu does it for you.

Best regards, Michael.

Message #38 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [PATCH v2 4/5] Makefile: Add target to run checkdoc
 on all files
Date: Sat, 22 Feb 2025 17:05:04 +0100

Morgan Smith <morgan.j.smith <at> outlook.com> writes:

> --- a/Makefile
> +++ b/Makefile
> @@ -8,7 +8,7 @@ TESTTARGET=$(patsubst %.el,%.elc,$(TESTSOURCE))
>
>  INFOMANUALS=debbugs.info debbugs-ug.info
>
> -.PHONY: all build check clean
> +.PHONY: all build check clean checkdoc
>  .PRECIOUS: %.elc
>
>  %.elc: %.el
> @@ -26,6 +26,9 @@ doc: $(INFOMANUALS)
>
>  build: $(TARGET)
>
> +checkdoc: $(SOURCE) $(TESTSOURCE)
> +	@$(EMACS) -Q --batch -l test/debbugs-checkdoc-config.el $(foreach file,$^,"--eval=(checkdoc-file \"$(file)\")")
> +
>  check: build $(TESTTARGET)
>  	@$(EMACS) -Q --batch -L . -L ./test $(foreach file,$(TESTSOURCE), -l $(file)) -f ert-run-tests-batch-and-exit

This would be wrong. You don't need to load debbugs-helpers.el (it is
required in *-tests.el) and debbugs-checkdoc-config.el (it isn't related
to ert tests.

> --- /dev/null
> +++ b/test/debbugs-checkdoc-config.el
> +  (setq checkdoc-ispell-lisp-words
> +        '("ChangeLog" "ChangeLogs" "UTF" "alist" "args"
> +          "armstrong" "backend" "bcc" "bugreport" "cdate" "cedet"
> +          "coreutils" "cygwin" "debbugs" "debian" "el" "emacs"
> +          "etags" "freemail" "fsf" "guix" "gw" "henoch" "hu"
> +          "hyperestraier" "keymap" "magit" "magnus" "maint"
> +          "maintainer" "maintainer's" "mbox" "mboxes" "minibuffer"
> +          "moreinfo" "multibyte" "notabug" "paren" "persistency"
> +          "regexp" "rescan" "rgm" "rmail" "severities" "sexp"
> +          "solaris" "src" "sublist" "submitter" "submitter's"
> +          "subproduct" "subqueries" "subquery" "teardown"
> +          "unarchived" "unibyte" "unreproducible" "url" "util"
> +          "wishlist" "wontfix" "wsdl" "www" "xsd" "zltuz")))

Same comment here: couldn't it be a ressource file?

And, more general: Could this file transformed into an ert test?

Best regards, Michael.

Message #41 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [PATCH v2 5/5] tests: Support older Emacs sorting
 conventions
Date: Sat, 22 Feb 2025 17:13:16 +0100

Morgan Smith <morgan.j.smith <at> outlook.com> writes:

> --- a/test/debbugs-tests.el
> +++ b/test/debbugs-tests.el
> +  (eval-and-compile
> +    (defun alist-sort (x y)
> +      (string< (car x) (car y))))

I would define this function in debbugs-test-helpers.el. And perhaps
give it a debbugs-test- prefix; we don't want to pollute the Emacs name
space.

Best regards, Michael.

Message #46 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [PATCH v2 1/5] tests: Move helper and wrapper
 functions into their own file
Date: Sun, 23 Feb 2025 14:05:17 -0500

[Message part 1 (text/plain, inline)]

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

> Morgan Smith <morgan.j.smith <at> outlook.com> 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.

[v3-0001-tests-Move-helper-and-wrapper-functions-into-thei.patch (text/x-patch, attachment)]

Message #49 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [PATCH v2 3/5] tests: Add tests for debbugs-gnu
Date: Sun, 23 Feb 2025 14:08:31 -0500

[Message part 1 (text/plain, inline)]

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

> Morgan Smith <morgan.j.smith <at> outlook.com> writes:
>
>>>> +Also test `debbugs-gnu-patches' and `debbugs-gnu-tagged' which are
>>>> +simple wrappers for `debbugs-gnu-search'."
>>>
>>> Nope. They are wrappers of `debbugs-gnu'.
>>
>> Oops!  You're right.  I have removed those tests.
>
> Perhaps you can give them own tests? In the futeure?
>

The tests I removed tested absolutely nothing.  In the future I will
likely add tests for that.  The current testing is pretty bare bones as
I'm just trying to get the test infrastructure up and running first.

>> --- /dev/null
>> +++ b/test/debbugs-gnu-tests.el
>> +(require 'debbugs)
>> +(require 'debbugs-gnu)
>
> You don't need to require 'debbugs'; debbugs-gnu does it for you.

Done!

>
> Best regards, Michael.

[v3-0003-tests-Add-tests-for-debbugs-gnu.patch (text/x-patch, attachment)]

Message #52 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [PATCH v2 4/5] Makefile: Add target to run checkdoc
 on all files
Date: Sun, 23 Feb 2025 14:17:34 -0500

[Message part 1 (text/plain, inline)]

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

> Morgan Smith <morgan.j.smith <at> outlook.com> writes:
>
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -8,7 +8,7 @@ TESTTARGET=$(patsubst %.el,%.elc,$(TESTSOURCE))
>>
>>  INFOMANUALS=debbugs.info debbugs-ug.info
>>
>> -.PHONY: all build check clean
>> +.PHONY: all build check clean checkdoc
>>  .PRECIOUS: %.elc
>>
>>  %.elc: %.el
>> @@ -26,6 +26,9 @@ doc: $(INFOMANUALS)
>>
>>  build: $(TARGET)
>>
>> +checkdoc: $(SOURCE) $(TESTSOURCE)
>> +	@$(EMACS) -Q --batch -l test/debbugs-checkdoc-config.el $(foreach file,$^,"--eval=(checkdoc-file \"$(file)\")")
>> +
>>  check: build $(TESTTARGET)
>>  	@$(EMACS) -Q --batch -L . -L ./test $(foreach file,$(TESTSOURCE), -l $(file)) -f ert-run-tests-batch-and-exit
>
> This would be wrong. You don't need to load debbugs-helpers.el (it is
> required in *-tests.el) and debbugs-checkdoc-config.el (it isn't related
> to ert tests.
>

I've decided to move debbugs-checkdoc-config.el into a new resources
directory so that's no longer an issue.

I understand that loading 'debbugs-test-helpers.el' isn't needed but I
don't see any harm in doing so.

>> --- /dev/null
>> +++ b/test/debbugs-checkdoc-config.el
>> +  (setq checkdoc-ispell-lisp-words
>> +        '("ChangeLog" "ChangeLogs" "UTF" "alist" "args"
>> +          "armstrong" "backend" "bcc" "bugreport" "cdate" "cedet"
>> +          "coreutils" "cygwin" "debbugs" "debian" "el" "emacs"
>> +          "etags" "freemail" "fsf" "guix" "gw" "henoch" "hu"
>> +          "hyperestraier" "keymap" "magit" "magnus" "maint"
>> +          "maintainer" "maintainer's" "mbox" "mboxes" "minibuffer"
>> +          "moreinfo" "multibyte" "notabug" "paren" "persistency"
>> +          "regexp" "rescan" "rgm" "rmail" "severities" "sexp"
>> +          "solaris" "src" "sublist" "submitter" "submitter's"
>> +          "subproduct" "subqueries" "subquery" "teardown"
>> +          "unarchived" "unibyte" "unreproducible" "url" "util"
>> +          "wishlist" "wontfix" "wsdl" "www" "xsd" "zltuz")))
>
> Same comment here: couldn't it be a ressource file?

Again, why?

> And, more general: Could this file transformed into an ert test?

I mean it could be but then the build dependencies would expand to
include ispell and an appropriate dictionary.

In my opinion linting is not testing and should be optional.

> Best regards, Michael.

[v3-0004-Makefile-Add-target-to-run-checkdoc-on-all-files.patch (text/x-patch, attachment)]

Message #55 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [PATCH v2 5/5] tests: Support older Emacs sorting
 conventions
Date: Sun, 23 Feb 2025 14:21:49 -0500

[Message part 1 (text/plain, inline)]

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

> Morgan Smith <morgan.j.smith <at> outlook.com> writes:
>
>> --- a/test/debbugs-tests.el
>> +++ b/test/debbugs-tests.el
>> +  (eval-and-compile
>> +    (defun alist-sort (x y)
>> +      (string< (car x) (car y))))
>
> I would define this function in debbugs-test-helpers.el. And perhaps
> give it a debbugs-test- prefix; we don't want to pollute the Emacs name
> space.

Ah, my bad.  I wrongly assumed Emacs had lexical scoping.

I've attached an alternative solution.  It would help stabilize the
return value of `debbugs-get-status'.  But also users should not be
relying on `debbugs-get-status' to have a stable return so maybe this is
a bad idea.

> Best regards, Michael.

[v3-0005-Sort-return-value-of-debbugs-get-status.patch (text/x-patch, attachment)]

Message #58 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [Patch Debbugs v3] Infrastructure Improvements
Date: Sun, 23 Feb 2025 14:26:19 -0500

[Message part 1 (text/plain, inline)]

I'm still not 100% sure what the ideal way to send in patches is.  Here
is the entire v3 patch series

[v3-0001-tests-Move-helper-and-wrapper-functions-into-thei.patch (text/x-patch, attachment)]

[v3-0002-tests-Use-unwind-protect-to-ensure-test-cleanup-a.patch (text/x-patch, attachment)]

[v3-0003-tests-Add-tests-for-debbugs-gnu.patch (text/x-patch, attachment)]

[v3-0004-Makefile-Add-target-to-run-checkdoc-on-all-files.patch (text/x-patch, attachment)]

[v3-0005-Sort-return-value-of-debbugs-get-status.patch (text/x-patch, attachment)]

Message #61 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [Patch Debbugs v3] Infrastructure Improvements
Date: Mon, 24 Feb 2025 19:52:12 +0100

[Message part 1 (text/plain, inline)]

Morgan Smith <morgan.j.smith <at> outlook.com> writes:

Hi Morgan,

> I'm still not 100% sure what the ideal way to send in patches is.  Here
> is the entire v3 patch series

This is fine. Since most of the changes are not controversial, I've
committed all your patches to the debbugs repo (plus some very minor
cleanups from me).

The last point I like to discuss is the Makefile. Attached is a patch I
propose. It changes the handling of the tests more like Emacs does. Now
we have different calls:

--8<---------------cut here---------------start------------->8---
# make check
# make debbugs-tests
# make debbugs-gnu-tests
--8<---------------cut here---------------end--------------->8---

What's missing is the handling of SELECTORs like Emacs does. This we
could add later.

WDYT?

Best regards, Michael.

[Message part 2 (text/x-patch, attachment)]

Message #64 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [Patch Debbugs v3] Infrastructure Improvements
Date: Tue, 25 Feb 2025 13:17:52 -0500

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

> Morgan Smith <morgan.j.smith <at> outlook.com> writes:
>
> Hi Morgan,
>
>> I'm still not 100% sure what the ideal way to send in patches is.  Here
>> is the entire v3 patch series
>
> This is fine. Since most of the changes are not controversial, I've
> committed all your patches to the debbugs repo (plus some very minor
> cleanups from me).

I am still curious.  The guix people seem to prefer 'git send-email' but
I was told not to do that by some emacs devel people.

> The last point I like to discuss is the Makefile. Attached is a patch I
> propose. It changes the handling of the tests more like Emacs does. Now
> we have different calls:
>
> --8<---------------cut here---------------start------------->8---
> # make check
> # make debbugs-tests
> # make debbugs-gnu-tests
> --8<---------------cut here---------------end--------------->8---
>
> What's missing is the handling of SELECTORs like Emacs does. This we
> could add later.
>
> WDYT?

I started looking at what emacs does.  It looks complicated.  I don't
entirely understand what they are doing.  However, I would argue that
what is currently in the debbugs makefile is quite powerful and I would
be interested in what use cases your solution provides.

Are you trying to improve performance of running the tests?  Here are the
performance numbers for running the current tests on my laptop:

make clean
time make build
0.590s
time make check
0.472s
time make check TESTS=get-status
0.148s
time make check TESTS=no-tests
0.122s
time emacs -Q --batch
0.033s

As we can see, if we only selectivly load the test files instead of loading all
of the test files, we can maybe save 1/10 of a second.  I personally am not
that worried about this.

Are you trying to selectively run tests so we can debug tests one at a time?
That is already entirely doable with the current setup.  Running 'make check
TESTS=get-status' only runs a singular test.

The patch you have provided actually removes the ability to run one test at a
time as we'd have to run entire files.


I'm not certain what the SELECTORs do but perhaps you're thinking that
in the future we would have groups of tests (like "expensive" or
"network") that we could select.  In the interest of not
over-engineering on hypotheticals, I would say that is a future problem.
One that I would likely solve by putting those test files in
sub-directories labeled "expensive" and "network".


>
> Best regards, Michael.
>
> diff --git a/Makefile b/Makefile
> index da0764ea36..bb69ebe9da 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,13 +1,11 @@
>  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)))
>  TESTTARGET=$(patsubst %.el,%.elc,$(TESTSOURCE))
> +TESTS=$(patsubst test/%.el,%,$(wildcard test/*-tests.el))
>
>  INFOMANUALS=debbugs.info debbugs-ug.info
>
> @@ -30,10 +28,14 @@ doc: $(INFOMANUALS)
>  build: $(TARGET)
>
>  checkdoc: $(SOURCE) $(TESTSOURCE)
> -	@$(EMACS) -Q --batch -l resources/debbugs-checkdoc-config.el $(foreach file,$^,"--eval=(checkdoc-file \"$(file)\")")
> +	@$(EMACS) -Q --batch -l resources/debbugs-checkdoc-config.el \
> +	  $(foreach file,$^,"--eval=(checkdoc-file \"$(file)\")")
> +
> +check: $(TESTS)
>
> -check: build $(TESTTARGET)
> -	@$(EMACS) -Q --batch -L . -L ./test $(foreach file,$(TESTSOURCE), -l $(file)) --eval '(ert-run-tests-batch-and-exit "$(TESTS)")'
> +%-tests: build $(TESTTARGET)
> +	@$(EMACS) -Q --batch -L . -L ./test -l $@ \
> +	  --eval '(ert-run-tests-batch-and-exit t)'
>
>  clean:
>  	-rm -f $(TARGET) $(TESTTARGET) $(INFOMANUALS)

Message #67 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [Patch Debbugs v3] Infrastructure Improvements
Date: Tue, 25 Feb 2025 20:29:19 +0100

Morgan Smith <morgan.j.smith <at> outlook.com> writes:

Hi Morgan,

> I started looking at what emacs does.  It looks complicated.  I don't
> entirely understand what they are doing.  However, I would argue that
> what is currently in the debbugs makefile is quite powerful and I would
> be interested in what use cases your solution provides.
>
> Are you trying to improve performance of running the tests?  Here are the
> performance numbers for running the current tests on my laptop:

No. It isn't performance. What I'm trying to do is to run only one test
file by a given make call. This wasn't possible; your version of the
Makefile has always loaded everything.

> Are you trying to selectively run tests so we can debug tests one at a time?
> That is already entirely doable with the current setup.  Running 'make check
> TESTS=get-status' only runs a singular test.

This is exactly what is called SELECTOR in the Emacs make. And this is
the same name used in the ERT manual.

I said already in my last message that this is missing, and I will add this.

> The patch you have provided actually removes the ability to run one test at a
> time as we'd have to run entire files.

No, with adding SELECTOR to the Makefile we'll have it. Read test/README
from the Emacs source tree for a better understanding how Emacs does it.

Disclaimer: I have written a large part of that README.

> I'm not certain what the SELECTORs do but perhaps you're thinking that
> in the future we would have groups of tests (like "expensive" or
> "network") that we could select.  In the interest of not
> over-engineering on hypotheticals, I would say that is a future problem.
> One that I would likely solve by putting those test files in
> sub-directories labeled "expensive" and "network".

A SELECTOR can be anything. A regexp, a test name a tag, you name it.
See (info "(ert) Test Selectors")

Best regards, Michael.

Message #70 received at 76446 <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: 76446 <at> debbugs.gnu.org
Subject: Re: bug#76446: [Patch Debbugs v3] Infrastructure Improvements
Date: Fri, 28 Feb 2025 13:04:23 +0100

[Message part 1 (text/plain, inline)]

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

Hi Morgan,

>> Are you trying to selectively run tests so we can debug tests one at a time?
>> That is already entirely doable with the current setup.  Running 'make check
>> TESTS=get-status' only runs a singular test.
>
> This is exactly what is called SELECTOR in the Emacs make. And this is
> the same name used in the ERT manual.
>
> I said already in my last message that this is missing, and I will add this.
>
>> The patch you have provided actually removes the ability to run one test at a
>> time as we'd have to run entire files.
>
> No, with adding SELECTOR to the Makefile we'll have it. Read test/README
> from the Emacs source tree for a better understanding how Emacs does it.
>
> Disclaimer: I have written a large part of that README.
>
>> I'm not certain what the SELECTORs do but perhaps you're thinking that
>> in the future we would have groups of tests (like "expensive" or
>> "network") that we could select.  In the interest of not
>> over-engineering on hypotheticals, I would say that is a future problem.
>> One that I would likely solve by putting those test files in
>> sub-directories labeled "expensive" and "network".
>
> A SELECTOR can be anything. A regexp, a test name a tag, you name it.
> See (info "(ert) Test Selectors")

I have extended the Makefile to support also SELECTOR, see
appended. Comments?

Best regards, Michael.

[Message part 2 (text/x-patch, attachment)]

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 76446 <at> debbugs.gnu.org, Morgan Smith <morgan.j.smith <at> outlook.com>
Subject: Re: bug#76446: [Patch Debbugs] Infrastructure Improvements
Date: Sun, 02 Mar 2025 00:44:41 +0900

Hi,

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

[...]

> +## SELECTOR discrimination (see ERT manual for more possibilities):
> +##
> +## SELECTOR='"regexp"': Run all tests which name match "regexp"
> +## SELECTOR='test-name': Run test with name test-name

The use of different quotes here appear somewhat misleading to me;
someone may think extra quoting is needed when using a regexp, but it
isn't, IIUC.

> +
> +### Code:
> +
>  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)))
> +TESTTARGET = $(patsubst %.el,%.elc,$(TESTSOURCE))
>
> -SOURCE=$(wildcard *.el)
> -TESTSOURCE=$(wildcard test/*.el)
> -TARGET=$(filter-out debbugs-pkg.elc,$(patsubst %.el,%.elc,$(SOURCE)))
> -TESTTARGET=$(patsubst %.el,%.elc,$(TESTSOURCE))
> +TESTS = $(patsubst test/%.el,%,$(wildcard test/*-tests.el))
> +SELECTOR ?= (not (tag :unstable))
>
> -INFOMANUALS=debbugs.info debbugs-ug.info
> +INFOMANUALS = debbugs.info debbugs-ug.info
>
>  .PHONY: all build check clean checkdoc
>  .PRECIOUS: %.elc
> @@ -30,10 +63,14 @@ doc: $(INFOMANUALS)
>  build: $(TARGET)
>
>  checkdoc: $(SOURCE) $(TESTSOURCE)
> -	@$(EMACS) -Q --batch -l resources/debbugs-checkdoc-config.el $(foreach file,$^,"--eval=(checkdoc-file \"$(file)\")")
> +	@$(EMACS) -Q --batch -l resources/debbugs-checkdoc-config.el \
> +	  $(foreach file,$^,"--eval=(checkdoc-file \"$(file)\")")
> +
> +check: $(TESTS)
>
> -check: build $(TESTTARGET)
> -	@$(EMACS) -Q --batch -L . -L ./test $(foreach file,$(TESTSOURCE), -l $(file)) --eval '(ert-run-tests-batch-and-exit "$(TESTS)")'
> +%-tests: build $(TESTTARGET)
> +	@$(EMACS) -Q --batch -L . -L ./test -l $@ \
> +	  --eval '(ert-run-tests-batch-and-exit (quote ${SELECTOR}))'

The above LGTM; I agree being able to select a single test is useful.

-- 
Thanks,
Maxim

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 76446 <at> debbugs.gnu.org, Morgan Smith <morgan.j.smith <at> outlook.com>
Subject: Re: bug#76446: [Patch Debbugs] Infrastructure Improvements
Date: Sat, 01 Mar 2025 19:47:33 +0100

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

> Hi,

Hi Maxim,

>> +## SELECTOR discrimination (see ERT manual for more possibilities):
>> +##
>> +## SELECTOR='"regexp"': Run all tests which name match "regexp"
>> +## SELECTOR='test-name': Run test with name test-name
>
> The use of different quotes here appear somewhat misleading to me;
> someone may think extra quoting is needed when using a regexp, but it
> isn't, IIUC.

This is needed due to regexp quoting and shell quoting.

A regexp must be a string. However, the following calls don't work:

--8<---------------cut here---------------start------------->8---
# make debbugs-tests SELECTOR=get
# make debbugs-tests SELECTOR='get'
# make debbugs-tests SELECTOR="get"
--8<---------------cut here---------------end--------------->8---

So we must use

--8<---------------cut here---------------start------------->8---
# make debbugs-tests SELECTOR='"get"'
--8<---------------cut here---------------end--------------->8---

If a selector isn't a regexp, we don't need quoting at all. Both works:

--8<---------------cut here---------------start------------->8---
# make debbugs-tests SELECTOR=debbugs-test-get-status
# make debbugs-tests SELECTOR='debbugs-test-get-status'
--8<---------------cut here---------------end--------------->8---

However, if the selector is more complex, we must quote:

--8<---------------cut here---------------start------------->8---
# make debbugs-tests SELECTOR='(not debbugs-test-get-status)'
--8<---------------cut here---------------end--------------->8---

Summary: I have the attitude to quote every selector with '...'. This works.

Best regards, Michael.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 76446 <at> debbugs.gnu.org, Morgan Smith <morgan.j.smith <at> outlook.com>
Subject: Re: bug#76446: [Patch Debbugs] Infrastructure Improvements
Date: Sun, 02 Mar 2025 14:20:22 +0900

Hello,

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

[...]

> If a selector isn't a regexp, we don't need quoting at all. Both works:
>
> # make debbugs-tests SELECTOR=debbugs-test-get-status
> # make debbugs-tests SELECTOR='debbugs-test-get-status'
>
>
> However, if the selector is more complex, we must quote:
>
> # make debbugs-tests SELECTOR='(not debbugs-test-get-status)'
>
> Summary: I have the attitude to quote every selector with '...'. This works.

Thanks for explaining; I also see this is standard behavior from ERT
(info '(ert) Test Selectors)').

Reviewed-by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>

--
Thanks,
Maxim

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 76446-done <at> debbugs.gnu.org, Morgan Smith <morgan.j.smith <at> outlook.com>
Subject: Re: bug#76446: [Patch Debbugs] Infrastructure Improvements
Date: Fri, 07 Mar 2025 09:14:17 +0100

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

> Hello,

Hi,

> Thanks for explaining; I also see this is standard behavior from ERT
> (info '(ert) Test Selectors)').
>
> Reviewed-by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>

No further comment, so I've pushed the change of the Makefile. Debbugs
0.44 is released; I'm closing the bug.

> Thanks,
> Maxim

Best regards, Michael.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 76446-done <at> debbugs.gnu.org, Morgan Smith <morgan.j.smith <at> outlook.com>
Subject: Re: bug#76446: [Patch Debbugs] Infrastructure Improvements
Date: Sat, 08 Mar 2025 23:08:50 +0900

Hi,

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

> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:
>
>> Hello,
>
> Hi,
>
>> Thanks for explaining; I also see this is standard behavior from ERT
>> (info '(ert) Test Selectors)').
>>
>> Reviewed-by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
>
> No further comment, so I've pushed the change of the Makefile. Debbugs
> 0.44 is released; I'm closing the bug.

Excellent; the emacs-debbugs package in Guix has already been updated to
it.

Many thanks!

-- 
Maxim

Previous Next

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