Re: bug#48598: 28.0.50; buffer-naming collisions involving bouncers in ERC

"J.P." <[email protected]> writes:

Hi,

>> Could you provide the (changed) test files together with the erc
>> package? This makes it more simple to puzzle all changes together,
>> instead of accessing different web locations.
>
> I've added the tests to the (fake) ELPA package you installed earlier.

Thanks. I'm now at erc 5.4.1.48598.0.20220422.667.

Starting slowly, just some first comments. It would be great to
distinguish in your package the same way as between the lisp/erc/ and
test/lisp/erc directories in the Emacs git repo. In your package, you
might use a test/ subdirectory, which contains evrything located in
test/lisp/erc of the Emacs git repo.

I'm not sure about the erc-d/ subdirectory. Is it something which is
also in the lisp/erc Emacs directory, or is it only in test/lisp/erc/?
In the latter case, I believe the directory shall contain just a file
test/lisp/erc/erc-d/erc-d-tests.el (currently called erc-d-self.el), and
*all* other files shall be located at test/lisp/erc/erc-d/erc-d-resources/.

Loading the related resource files, you use a pattern like

--8<---------------cut here---------------start------------->8---
(eval-and-compile (let ((dir (getenv "EMACS_TEST_DIRECTORY")))
                    (when dir
                      (load (concat dir "/lisp/erc/erc-d/erc-d") nil t)
                      (load (concat dir "/lisp/erc/erc-d/erc-d-t") nil t))))
--8<---------------cut here---------------end--------------->8---

That looks too complicate. There are the functions ert-resource-directory
and ert-resource-file. You could use something like (untested)

--8<---------------cut here---------------start------------->8---
(eval-and-compile (let ((load-path (cons (ert-resource-directory) load-path)))
                      (load "erc-d" nil t)
                      (load "erc-d-t" nil t)))
--8<---------------cut here---------------end--------------->8---

In erc-services-tests.el, I would call the tests
erc--auth-source-search--{standard,announced,overrides}
erc--auth-source-search--netrc-{standard,announced,overrides}, because
they are about this backend. The body of these functions is almost the
same for the different backends; perhaps you can factor it out.

I would mark the tests erc--auth-source-search--pass-* with
":tags '(:unstable)" until the problems in auth-source-pass are solved.

Again, all of this is just cursory reading, I haven't started yet to
read the real code.

> Appreciate the counsel, as always.
>
> J.P.

Best regards, Michael.