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

Previous Next

Full log

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

Previous Next

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