Perl and Emacs: Developing tests for progmodes

Perl and Emacs: Developing tests for progmodes

[Harald Jörg]

Hi emacs-devel,

Emacs comes with two modes to edit Perl sources: the rather
lightweight perl-mode, and the not-so-lightweight cperl-mode. Two
weeks ago I wrote a first ERT test for a bug fix in cperl-mode in
test/lisp/progmodes/cperl-mode-tests.el.

This test has been rewritten so that it can be applied to both
cperl-mode and perl-mode, which is nice.

However, there are bugs in cperl-mode which perl-mode doesn't have,
mostly because perl-mode doesn't offer the functions which fail in the
first place.  So, tests for these bugs need to (require 'cperl-mode),
and they make no sense when testing perl-mode.  I expect most of the
upcoming tests to fall into that category.

To solve this, we could:

 - Split the tests into two and manually copy over tests where this
   makes sense or:

 - Split the tests into a "common" set plus two sets for the specific
   modes (how would the files be named in that case?) or:

 - Keep all tests in one file and tag the individual tests.

Are there any similar cases in the set of Emacs packages, or
conventions how to do it?
-- 
Cheers,
haj

Re: Perl and Emacs: Developing tests for progmodes

[Stefan Monnier]

> However, there are bugs in cperl-mode which perl-mode doesn't have,
> mostly because perl-mode doesn't offer the functions which fail in the
> first place.  So, tests for these bugs need to (require 'cperl-mode),
> and they make no sense when testing perl-mode.  I expect most of the
> upcoming tests to fall into that category.
>
> To solve this, we could:
>
>  - Split the tests into two and manually copy over tests where this
>    makes sense or:
>
>  - Split the tests into a "common" set plus two sets for the specific
>    modes (how would the files be named in that case?) or:
>
>  - Keep all tests in one file and tag the individual tests.

I think for now we can tag the tests individually with

    (ert-deftest (...)
      (skip-unless (eq cperl-test-mode #'cperl-mode))
      ...)

We can change later if that becomes inconvenient (e.g. almost all the
tests only apply to cperl-mode).


        Stefan

Re: Perl and Emacs: Developing tests for progmodes

[Stefan Kangas]

[-- Attachment #1: Type: text/plain, Size: 334 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I think for now we can tag the tests individually with
>
>     (ert-deftest (...)
>       (skip-unless (eq cperl-test-mode #'cperl-mode))
>       ...)

How about something like the attached to make the tests run
automatically also for perl-mode?  It's a bit of a hack, admittedly.

[-- Attachment #2: 0001-test-lisp-progmodes-perl-mode-tests.el-New-file.patch --]
[-- Type: text/x-diff, Size: 1853 bytes --]

From dc8f879c40c124db4211d0b0579672367936019b Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Thu, 3 Sep 2020 21:59:53 +0200
Subject: [PATCH] * test/lisp/progmodes/perl-mode-tests.el: New file.

---
 test/lisp/progmodes/perl-mode-tests.el | 33 ++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 test/lisp/progmodes/perl-mode-tests.el

diff --git a/test/lisp/progmodes/perl-mode-tests.el b/test/lisp/progmodes/perl-mode-tests.el
new file mode 100644
index 0000000000..a2ea972c10
--- /dev/null
+++ b/test/lisp/progmodes/perl-mode-tests.el
@@ -0,0 +1,33 @@
+;;; perl-mode-tests --- Test for perl-mode  -*- lexical-binding: t -*-
+
+;; Copyright (C) 2020 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs 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.
+
+;; GNU Emacs 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/>.
+
+;;; Code:
+
+(require 'perl-mode)
+
+;;;; Re-use cperl-mode tests
+
+(defvar cperl-test-mode)
+(setq cperl-test-mode #'perl-mode)
+(load-file (expand-file-name "cperl-mode-tests.el"
+                             (file-truename
+                              (file-name-directory (or load-file-name
+                                                       buffer-file-name)))))
+
+;;; perl-mode-tests.el ends here
-- 
2.28.0

Re: Perl and Emacs: Developing tests for progmodes

[Stefan Kangas]

Stefan Kangas <stefankangas@gmail.com> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> I think for now we can tag the tests individually with
>>
>>     (ert-deftest (...)
>>       (skip-unless (eq cperl-test-mode #'cperl-mode))
>>       ...)
>
> How about something like the attached to make the tests run
> automatically also for perl-mode?  It's a bit of a hack, admittedly.

This never got a reply.  Is the idea reasonable, or should it just be
scrapped?  I personally don't have a strong opinion, but I didn't want to
throw it away without at least asking first.

(It is sometimes hard for me to understand if no reply means "sure, go
ahead" or "this is too dumb to even comment on"...  ;-)

Of course, it will introduce some potential friction in developing new
cperl-mode tests, but on the other hand perl-mode.el will see some
automated testing.

> diff --git a/test/lisp/progmodes/perl-mode-tests.el b/test/lisp/progmodes/perl-mode-tests.el
> new file mode 100644
> index 0000000000..a2ea972c10
> --- /dev/null
> +++ b/test/lisp/progmodes/perl-mode-tests.el
> @@ -0,0 +1,33 @@
[...]
> +(require 'perl-mode)
> +
> +;;;; Re-use cperl-mode tests
> +
> +(defvar cperl-test-mode)
> +(setq cperl-test-mode #'perl-mode)
> +(load-file (expand-file-name "cperl-mode-tests.el"
> +                             (file-truename
> +                              (file-name-directory (or load-file-name
> +                                                       buffer-file-name)))))
> +
> +;;; perl-mode-tests.el ends here

Re: Perl and Emacs: Developing tests for progmodes

[Stefan Monnier]

>> How about something like the attached to make the tests run
>> automatically also for perl-mode?  It's a bit of a hack, admittedly.
> This never got a reply.  Is the idea reasonable, or should it just be
> scrapped?

Yes, very reasonable.  I was wondering why you never installed it ;-)


        Stefan

Re: Perl and Emacs: Developing tests for progmodes

[Stefan Kangas]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> How about something like the attached to make the tests run
>>> automatically also for perl-mode?  It's a bit of a hack, admittedly.
>> This never got a reply.  Is the idea reasonable, or should it just be
>> scrapped?
>
> Yes, very reasonable.  I was wondering why you never installed it ;-)

:-)

Installed on master as commit 6937408c08.

Re: Perl and Emacs: Developing tests for progmodes

[Harald Jörg]

Stefan Kangas <stefankangas@gmail.com> writes:

> Stefan Kangas <stefankangas@gmail.com> writes:
>
>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>
>>> I think for now we can tag the tests individually with
>>>
>>>     (ert-deftest (...)
>>>       (skip-unless (eq cperl-test-mode #'cperl-mode))
>>>       ...)
>>
>> How about something like the attached to make the tests run
>> automatically also for perl-mode?  It's a bit of a hack, admittedly.
>
> This never got a reply.  Is the idea reasonable, or should it just be
> scrapped?  I personally don't have a strong opinion, but I didn't want to
> throw it away without at least asking first.

Stefan (Monnier) already committed this as a patch (7a7847d7ad5;
Fri Aug 14) :)

> (It is sometimes hard for me to understand if no reply means "sure, go
> ahead" or "this is too dumb to even comment on"...  ;-)
>
> Of course, it will introduce some potential friction in developing new
> cperl-mode tests, but on the other hand perl-mode.el will see some
> automated testing.

Yes, there is some friction, but as the test suite is still very small.
It can easily be split into two files whenever this seems appropriate.
I don't have a strong opinion either.

In any case, it makes sense to share resources (Perl sources), providing
a nice edge case for the new ert-resource-file function. :)

Here's the experience so far:

Together with his suggestion above, Stefan (M.) rewrote one of the tests
so that it would work with both modes.  It turned out that perl-mode
showed the same bug, which he fixed.  So this was one of the cases where
a common test absolutely makes sense.

Tests which are specific for a function which only exists in cperl-mode
make no sense with perl-mode.  We have a few of those tests, and right
now I found one where I forgot to skip it when testing perl-mode.

Things get murky for tests where cperl-mode intentionally behaves
differently than perl-mode: A test makes sense, but the
"should"-condition needs to be different.  There's more friction in
those tests (Example: the modes populate the "imenu"-Index quite
differently).

My recent addition to the tests (fb26dc13: cperl-mode: Delete a
misleading comment, add tests for verification; Mon Oct 19) shows
another issue: The test (fontification of \$") makes sense for both
cperl-mode and perl-mode.  In cperl-mode, the fontification was fixed in
1997 (I think), in perl-mode it is still wrong.  I'm not deep enough in
either of the modes (yet) to figure out what to do to fix this in
perl-mode, so I decided to only run the test for cperl-mode.

Finally, perl-mode comes with its own list of ancient open bugs, many of
those don't occur in cperl-mode.  I wonder whether the authors of these
bugs would accept "use cperl-mode instead" as a workaround?

At some time, it might make sense to merge those two modes into one.
Perl continues to evolve, and upgrading two modes to support that
doesn't seem to be an economic use of time.
--
Cheers,
haj

Re: Perl and Emacs: Developing tests for progmodes

[Stefan Kangas]

haj@posteo.de (Harald Jörg) writes:

> Stefan (Monnier) already committed this as a patch (7a7847d7ad5;
> Fri Aug 14) :)

I was referring to my separate patch, now committed in 754a2f11b8.
Sorry for being unclear.

> My recent addition to the tests (fb26dc13: cperl-mode: Delete a
> misleading comment, add tests for verification; Mon Oct 19) shows
> another issue: The test (fontification of \$") makes sense for both
> cperl-mode and perl-mode.  In cperl-mode, the fontification was fixed in
> 1997 (I think), in perl-mode it is still wrong.  I'm not deep enough in
> either of the modes (yet) to figure out what to do to fix this in
> perl-mode, so I decided to only run the test for cperl-mode.
>
> Finally, perl-mode comes with its own list of ancient open bugs, many of
> those don't occur in cperl-mode.  I wonder whether the authors of these
> bugs would accept "use cperl-mode instead" as a workaround?
>
> At some time, it might make sense to merge those two modes into one.
> Perl continues to evolve, and upgrading two modes to support that
> doesn't seem to be an economic use of time.

Some interesting points.  I'm interested to hear what others have to
say about them.

Re: Perl and Emacs: Developing tests for progmodes

[Eli Zaretskii]

> From: Harald Jörg <haj@posteo.de>
> Date: Thu, 3 Sep 2020 15:42:47 +0200
> Cc: Stefan Kangas <stefankangas@gmail.com>,
>  Stefan Monnier <monnier@iro.umontreal.ca>
> 
> To solve this, we could:
> 
>  - Split the tests into two and manually copy over tests where this
>    makes sense or:
> 
>  - Split the tests into a "common" set plus two sets for the specific
>    modes (how would the files be named in that case?) or:
> 
>  - Keep all tests in one file and tag the individual tests.
> 
> Are there any similar cases in the set of Emacs packages, or
> conventions how to do it?

Don't perl-mode and cperl-mode live on 2 separate files?  We generally
have a structure under test/ that mirrors the file hierarchy of the
sources being tested, so it would be natural to have 2 separate files,
test/lisp/progmodes/perl-mode-tests.el and
test/lisp/progmodes/cperl-mode-tests.el.

Does that answer your question?

Re: Perl and Emacs: Developing tests for progmodes

[Harald Jörg]

On 9/3/20 7:34 PM, Eli Zaretskii wrote:
>> From: Harald Jörg <haj@posteo.de>
>> Date: Thu, 3 Sep 2020 15:42:47 +0200
>> Cc: Stefan Kangas <stefankangas@gmail.com>,
>>  Stefan Monnier <monnier@iro.umontreal.ca>
>>
>> To solve this, we could:
>>
>>  - Split the tests into two and manually copy over tests where this
>>    makes sense or:
>>
>>  - Split the tests into a "common" set plus two sets for the specific
>>    modes (how would the files be named in that case?) or:
>>
>>  - Keep all tests in one file and tag the individual tests.
>>
>> Are there any similar cases in the set of Emacs packages, or
>> conventions how to do it?
>
> Don't perl-mode and cperl-mode live on 2 separate files?

They do.

> We generally
> have a structure under test/ that mirrors the file hierarchy of the
> sources being tested, so it would be natural to have 2 separate files,
> test/lisp/progmodes/perl-mode-tests.el and
> test/lisp/progmodes/cperl-mode-tests.el.

Such was my understanding of test/file-organization.org, too.

> Does that answer your question?

Not really: As far as I understand, Stefan Monnier augmented the first
version of cperl-mode-tests.el so that it is also applicable for
perl-mode.el (commit 7a7847d7ad5).  So, a (not-yet-existing)
perl-mode-tests.el would just (let ((cperl-test-mode #'perl-mode)) and
then run the tests in cperl-mode-tests.pl.

For that to work, the tests in cperl-mode-tests.el need to co-operate.
My recent test written for (Bug#16368) calls a function which is only
available in cperl-mode, and the tests for (Bug#10483) exercise
cperl-mode's indentation by calling `cperl-indent-exp' and expect
cperl-mode's indentation results.  The simple approach from above
would then either give false negative results (whenever perl-mode runs
against cperl-mode's expectations) or just run the same test twice
(when the test explicitly calls a cperl-mode function).

Adding a line (skip-unless (eq cperl-test-mode #'cperl-mode)), as
suggested by Stefan Monnier, will do the trick, and will allow me to
continue chasing old bugs or even add new bugs, eerm, features.
-- 
Cheers,
haj

Re: Perl and Emacs: Developing tests for progmodes

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org, stefankangas@gmail.com, monnier@iro.umontreal.ca
> From: Harald Jörg <haj@posteo.de>
> Date: Thu, 3 Sep 2020 22:00:22 +0200
> 
> > We generally
> > have a structure under test/ that mirrors the file hierarchy of the
> > sources being tested, so it would be natural to have 2 separate files,
> > test/lisp/progmodes/perl-mode-tests.el and
> > test/lisp/progmodes/cperl-mode-tests.el.
> 
> Such was my understanding of test/file-organization.org, too.
> 
> > Does that answer your question?
> 
> Not really: As far as I understand, Stefan Monnier augmented the first
> version of cperl-mode-tests.el so that it is also applicable for
> perl-mode.el (commit 7a7847d7ad5).  So, a (not-yet-existing)
> perl-mode-tests.el would just (let ((cperl-test-mode #'perl-mode)) and
> then run the tests in cperl-mode-tests.pl.

Then I guess I misunderstand the problem you are trying to solve,
sorry.