Re: ERT indentation testing

Re: ERT indentation testing

[Stefan Monnier]

> This can be done with ERT, right?  A bunch of .cc files with file-local
> variables could be indented "properly" for those variables.  Then the
> test logic for each file is 
[...]
> Plus, of course, it's not limited to .cc files.  It would work just as
> well for .c, .pl, .el, whatever.

Yup, we already have such files in test/indent.

> I can write the ERT wrapper if this sounds useful.

If you could integrate test/indent with ERT's automated tests, that
would be nice.


        Stefan

Re: ERT indentation testing

[Ted Zlatanov]

On Wed, 18 May 2011 19:19:25 -0300 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>> This can be done with ERT, right?  A bunch of .cc files with file-local
>> variables could be indented "properly" for those variables.  Then the
>> test logic for each file is 
SM> [...]
>> Plus, of course, it's not limited to .cc files.  It would work just as
>> well for .c, .pl, .el, whatever.

SM> Yup, we already have such files in test/indent.

Yes, I see that now (I didn't see the commit for those, cool!)

We should have .c and .cc files with the major indentation styles
specified as file-local variables, plus .ini, Perl, Java, Python, and of
course Lisp (CL and ELisp).  Should I add those?

>> I can write the ERT wrapper if this sounds useful.

SM> If you could integrate test/indent with ERT's automated tests, that
SM> would be nice.

OK, I'll put it on my TODO list.

Ted

Re: ERT indentation testing

[Stefan Monnier]

>>> This can be done with ERT, right?  A bunch of .cc files with file-local
>>> variables could be indented "properly" for those variables.  Then the
>>> test logic for each file is 
SM> [...]
>>> Plus, of course, it's not limited to .cc files.  It would work just as
>>> well for .c, .pl, .el, whatever.

SM> Yup, we already have such files in test/indent.

> Yes, I see that now (I didn't see the commit for those, cool!)

> We should have .c and .cc files with the major indentation styles
> specified as file-local variables, plus .ini, Perl, Java, Python, and of
> course Lisp (CL and ELisp).  Should I add those?

Feel free.  I only added the ones I actually worked on.  BTW, the
octave.m testcase is too long for that kind of test (although the length
was instrumental in fixing some performance bugs).


        Stefan

Re: ERT indentation testing

[Ted Zlatanov]

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

On Thu, 19 May 2011 08:56:07 -0300 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>>>> This can be done with ERT, right?  A bunch of .cc files with file-local
>>>> variables could be indented "properly" for those variables.  Then the
>>>> test logic for each file is 
SM> [...]
>>>> Plus, of course, it's not limited to .cc files.  It would work just as
>>>> well for .c, .pl, .el, whatever.

SM> Yup, we already have such files in test/indent.

>> Yes, I see that now (I didn't see the commit for those, cool!)

>> We should have .c and .cc files with the major indentation styles
>> specified as file-local variables, plus .ini, Perl, Java, Python, and of
>> course Lisp (CL and ELisp).  Should I add those?

SM> Feel free.  I only added the ones I actually worked on.  BTW, the
SM> octave.m testcase is too long for that kind of test (although the length
SM> was instrumental in fixing some performance bugs).

See the attached patch to make the indentation testing use ERT and
operate on any files with a "." in the name except test-indent.el.

The test fails on modula2.mod and I'm not sure why.  I am setting
`enable-local-variables'.  Is it because I'm going line by line?  Or
something with the line boundaries?

Should the test stop when any file fails or keep going?

Thanks
Ted


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: test-indent.patch --]
[-- Type: text/x-diff, Size: 2729 bytes --]

=== modified file 'test/indent/Makefile'
--- test/indent/Makefile	2010-08-30 20:34:52 +0000
+++ test/indent/Makefile	2011-06-01 20:48:21 +0000
@@ -13,3 +13,6 @@
 	    --eval '(indent-region (point-min) (point-max) nil)' \
 	    --eval '(write-region (point-min) (point-max) "$<.new")'
 	diff -u -B $< $<.new
+
+test-indent:
+	$(EMACS) --batch -l ert -l test-indent.el -f ert-run-tests-batch-and-exit

=== added file 'test/indent/test-indent.el'
--- test/indent/test-indent.el	1970-01-01 00:00:00 +0000
+++ test/indent/test-indent.el	2011-06-01 21:28:07 +0000
@@ -0,0 +1,57 @@
+;;; test-indent.el --- run indentation tests
+
+;; Copyright (C) 2011  Free Software Foundation, Inc.
+
+;; Author: Ted Zlatanov <tzz@lifelogs.com>
+
+;; 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 <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; The purpose of this module is to verify that all the files in te
+;; current directory are indented correctly.
+
+;;; Code:
+
+(eval-when-compile
+  (require 'ert)
+  (require 'cl))
+
+(ert-deftest test-indent-all ()
+  (let ((enable-local-variables :all)
+        lnum)
+    (loop for f in (delete "test-indent.el"
+                           (directory-files "." nil "^[^.]+\\."))
+          do (with-temp-buffer
+               (message "Testing indentation of %s" f)
+               (insert-file-contents f)
+               (goto-char (point-min))
+               (setq lnum 0)
+               (while (not (eobp))
+                 (incf lnum)
+                 (let* ((a (line-beginning-position))
+                        (b (line-end-position))
+                        (line (buffer-substring a b)))
+                   (message "Testing indentation of %s:%05d: %s" f lnum line)
+                   (unless (string-match "KNOWN INDENT BUG" line)
+                     (indent-region a b nil)
+                     (should (equal line (buffer-substring
+                                          (line-beginning-position)
+                                          (line-end-position)))))
+                   (forward-line)))))))
+
+(provide 'test-indent)
+;;; test-indent.el ends here

Re: ERT indentation testing

[Alan Mackenzie]

Hi, Ted.

On 19th May 2011, you wrote:

>>> This can be done with ERT, right?  A bunch of .cc files with file-local
>>> variables could be indented "properly" for those variables.  Then the
>>> test logic for each file is 
>SM> [...]
>>> Plus, of course, it's not limited to .cc files.  It would work just as
>>> well for .c, .pl, .el, whatever.

>SM> Yup, we already have such files in test/indent.

>Yes, I see that now (I didn't see the commit for those, cool!)

>We should have .c and .cc files with the major indentation styles
>specified as file-local variables, plus .ini, Perl, Java, Python, and of
>course Lisp (CL and ELisp).  Should I add those?

>>> I can write the ERT wrapper if this sounds useful.

>SM> If you could integrate test/indent with ERT's automated tests, that
>SM> would be nice.

>OK, I'll put it on my TODO list.

You are aware that there's an extensive test suite for CC Mode's
indentation (and font locking).  It's in the tests directory of CC Mode's
CVS repository, which can be downloaded from
http://cc-mode.sourceforge.net/.  Look for the file 000tests.el.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: ERT indentation testing

[Ted Zlatanov]

On Thu, 2 Jun 2011 12:37:33 +0000 Alan Mackenzie <acm@muc.de> wrote: 

AM> You are aware that there's an extensive test suite for CC Mode's
AM> indentation (and font locking).  It's in the tests directory of CC Mode's
AM> CVS repository, which can be downloaded from
AM> http://cc-mode.sourceforge.net/.  Look for the file 000tests.el.

I can't access that repository at the moment.  Is there a mirror I can
use over HTTP, e.g. Git or Bazaar?

I didn't know that these tests existed.  Why not simply integrate them
in the Emacs tests?  Are they too specific?

A general indentation and font-locking tester is very useful, so if we
can generalize this code, it would be wonderful.

Ted

Re: ERT indentation testing

[Ted Zlatanov]

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

On Wed, 01 Jun 2011 16:30:56 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> See the attached patch to make the indentation testing use ERT and
TZ> operate on any files with a "." in the name except test-indent.el.

Second patch:

TZ> The test fails on modula2.mod and I'm not sure why.  I am setting
TZ> `enable-local-variables'.  Is it because I'm going line by line?  Or
TZ> something with the line boundaries?

Uses `find-file' to process the local variables correctly.

TZ> Should the test stop when any file fails or keep going?

One test is created per file now, and any one file won't block the other tests.

Ted


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: test-indent.patch --]
[-- Type: text/x-diff, Size: 3376 bytes --]

=== modified file 'test/indent/Makefile'
--- test/indent/Makefile	2010-08-30 20:34:52 +0000
+++ test/indent/Makefile	2011-06-02 18:07:15 +0000
@@ -13,3 +13,6 @@
 	    --eval '(indent-region (point-min) (point-max) nil)' \
 	    --eval '(write-region (point-min) (point-max) "$<.new")'
 	diff -u -B $< $<.new
+
+test-indent:
+	$(EMACS) --batch -l ert -l test-indent.el -f ert-run-tests-batch-and-exit

=== added file 'test/indent/test-indent.el'
--- test/indent/test-indent.el	1970-01-01 00:00:00 +0000
+++ test/indent/test-indent.el	2011-06-02 18:39:23 +0000
@@ -0,0 +1,67 @@
+;;; test-indent.el --- run indentation tests
+
+;; Copyright (C) 2011  Free Software Foundation, Inc.
+
+;; Author: Ted Zlatanov <tzz@lifelogs.com>
+
+;; 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 <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; The purpose of this module is to verify that all the files in te
+;; current directory are indented correctly.
+
+;;; Code:
+
+(eval-when-compile
+  (require 'ert)
+  (require 'cl)
+
+  (defsubst test-indent-trim (string)
+    "Lose leading and trailing whitespace.  Copied from BBDB."
+    (if (string-match "\\`[ \t\n]+" string)
+        (setq string (substring string (match-end 0))))
+    (if (string-match "[ \t\n]+\\'" string)
+        (setq string (substring string 0 (match-beginning 0))))
+    string)
+
+  (let ((files (delete "test-indent.el"
+                       (directory-files "." nil "\\`[^.]+\\..+[a-zA-Z]\\'")))
+        f)
+    (loop for i from 1 to (length files)
+          for f = (nth (1- i) files)
+          do (eval `(ert-deftest ,(intern (format "test-indent-%d-%s" i f)) ()
+                      (let ((lnum 0))
+                        (message "Testing indentation of %s" ,f)
+                        (find-file ,f)
+                        (goto-char (point-min))
+                        (while (not (eobp))
+                          (incf lnum)
+                          (let* ((a (line-beginning-position))
+                                 (b (line-end-position))
+                                 (line (buffer-substring a b)))
+                            (message "Testing indentation of %s:%05d: %s"
+                                     ,f lnum line)
+                            (unless (or (string-match "KNOWN INDENT BUG" line)
+                                        (equal (test-indent-trim line) ""))
+                              (indent-region a b nil)
+                              (should (equal line (buffer-substring
+                                                   (line-beginning-position)
+                                                   (line-end-position)))))
+                            (forward-line)))))))))
+
+(provide 'test-indent)
+;;; test-indent.el ends here

Re: ERT indentation testing

[Stefan Monnier]

> Uses `find-file' to process the local variables correctly.

Never call `find-file' from Elisp.  Always call
find-file-noselect instead.


        Stefan

Re: ERT indentation testing

[Ted Zlatanov]

On Thu, 02 Jun 2011 21:04:59 -0300 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>> Uses `find-file' to process the local variables correctly.

SM> Never call `find-file' from Elisp.  Always call
SM> find-file-noselect instead.

I was trying to copy your original test, which I think uses `find-file':

#+begin_src makefile
%.test: %
	-$(RM) $<.new
	$(EMACS) --batch $< \
	    --eval '(indent-region (point-min) (point-max) nil)' \
	    --eval '(write-region (point-min) (point-max) "$<.new")'
	diff -u -B $< $<.new

#+end_src

Using `find-file-noselect' will just make me select the buffer in ELisp
instead of expecting it to become current.  Is there any real reason to
avoid `find-file'?  Because of the prompts?  I ask because I'm curious;
I've already changed my patch to use (set-buffer (find-file-noselect...))
but can't commit because the Bazaar repo is down.

Also, if `find-file' should be avoided in ELisp generally, the docstring
should say so, like it does for many other functions.

Actually it might be nice if there was a standard way to tag such
extra-interactive functions that are not supposed to be called from an
ELisp context, only from `M-x' or through a key.  A symbol property
maybe?  Then the compiler could check it and print suitable errors...
Tell me it already exists :)

Thanks
Ted

Re: ERT indentation testing

[Ted Zlatanov]

On Thu, 02 Jun 2011 09:23:41 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> On Thu, 2 Jun 2011 12:37:33 +0000 Alan Mackenzie <acm@muc.de> wrote: 
AM> You are aware that there's an extensive test suite for CC Mode's
AM> indentation (and font locking).  It's in the tests directory of CC Mode's
AM> CVS repository, which can be downloaded from
AM> http://cc-mode.sourceforge.net/.  Look for the file 000tests.el.

TZ> I didn't know that these tests existed.  Why not simply integrate them
TZ> in the Emacs tests?  Are they too specific?

TZ> A general indentation and font-locking tester is very useful, so if we
TZ> can generalize this code, it would be wonderful.

I looked at the code and it could be useful.  But it has no notion of
ERT testing and consists of very specific code to force and test
font-lock and indentation.  So it would be a lot of work to generalize
it (or parts of it) and integrate it with the GNU Emacs indentation
tests.  It may be worthwhile nevertheless.

Ted

Re: ERT indentation testing

[martin rudalics]

 > Using `find-file-noselect' will just make me select the buffer in ELisp
 > instead of expecting it to become current.  Is there any real reason to
 > avoid `find-file'?  Because of the prompts?  I ask because I'm curious;
 > I've already changed my patch to use (set-buffer (find-file-noselect...))
 > but can't commit because the Bazaar repo is down.

`find-file' uses `switch-to-buffer' and `switch-to-buffer' should be
avoided in Elisp code.

 > Also, if `find-file' should be avoided in ELisp generally, the docstring
 > should say so, like it does for many other functions.

Indeed.

martin

Re: ERT indentation testing

[Ted Zlatanov]

On Thu, 02 Jun 2011 13:43:27 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> On Wed, 01 Jun 2011 16:30:56 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 
TZ> See the attached patch to make the indentation testing use ERT and
TZ> operate on any files with a "." in the name except test-indent.el.

TZ> Second patch:

TZ> The test fails on modula2.mod and I'm not sure why.  I am setting
TZ> `enable-local-variables'.  Is it because I'm going line by line?  Or
TZ> something with the line boundaries?

TZ> Uses `find-file' to process the local variables correctly.

Is the patch above usable (using `find-file-noselect' as discussed)?
Should I keep working on this, commit it as is, or forget about it?

Thanks
Ted

Re: ERT indentation testing

[Stefan Monnier]

> Is the patch above usable (using `find-file-noselect' as discussed)?

I trust your judgment on it.  The functionality is one I'd like to see,
but I don't have time to check the actual code.

> Should I keep working on this, commit it as is, or forget about it?

If/when you think it's ready, please install it.


        Stefan

Re: ERT indentation testing

[Ted Zlatanov]

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

On Wed, 08 Jun 2011 12:24:13 -0300 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>> Is the patch above usable (using `find-file-noselect' as discussed)?
SM> I trust your judgment on it.  The functionality is one I'd like to see,
SM> but I don't have time to check the actual code.

OK, great.

>> Should I keep working on this, commit it as is, or forget about it?

SM> If/when you think it's ready, please install it.

I'm concerned that it's failing a few tests.  I think the files are
incorrect for the .rc and .sh content, don't know about the Prolog.

   FAILED  test-indent-2-prolog\.prolog
   FAILED  test-indent-3-shell\.rc
   FAILED  test-indent-4-shell\.sh

Third patch attached, just "make test-indent".  If you think those tests
are legitimate failures, I'll commit, but otherwise I should fix them.

Thanks
Ted


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: test-indent.patch --]
[-- Type: text/x-diff, Size: 3398 bytes --]

=== modified file 'test/indent/Makefile'
--- test/indent/Makefile	2010-08-30 20:34:52 +0000
+++ test/indent/Makefile	2011-06-02 18:07:15 +0000
@@ -13,3 +13,6 @@
 	    --eval '(indent-region (point-min) (point-max) nil)' \
 	    --eval '(write-region (point-min) (point-max) "$<.new")'
 	diff -u -B $< $<.new
+
+test-indent:
+	$(EMACS) --batch -l ert -l test-indent.el -f ert-run-tests-batch-and-exit

=== added file 'test/indent/test-indent.el'
--- test/indent/test-indent.el	1970-01-01 00:00:00 +0000
+++ test/indent/test-indent.el	2011-06-02 18:39:23 +0000
@@ -0,0 +1,67 @@
+;;; test-indent.el --- run indentation tests
+
+;; Copyright (C) 2011  Free Software Foundation, Inc.
+
+;; Author: Ted Zlatanov <tzz@lifelogs.com>
+
+;; 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 <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; The purpose of this module is to verify that all the files in te
+;; current directory are indented correctly.
+
+;;; Code:
+
+(eval-when-compile
+  (require 'ert)
+  (require 'cl)
+
+  (defsubst test-indent-trim (string)
+    "Lose leading and trailing whitespace.  Copied from BBDB."
+    (if (string-match "\\`[ \t\n]+" string)
+        (setq string (substring string (match-end 0))))
+    (if (string-match "[ \t\n]+\\'" string)
+        (setq string (substring string 0 (match-beginning 0))))
+    string)
+
+  (let ((files (delete "test-indent.el"
+                       (directory-files "." nil "\\`[^.]+\\..+[a-zA-Z]\\'")))
+        f)
+    (loop for i from 1 to (length files)
+          for f = (nth (1- i) files)
+          do (eval `(ert-deftest ,(intern (format "test-indent-%d-%s" i f)) ()
+                      (let ((lnum 0))
+                        (message "Testing indentation of %s" ,f)
+                        (set-buffer (find-file-noselect ,f))
+                        (goto-char (point-min))
+                        (while (not (eobp))
+                          (incf lnum)
+                          (let* ((a (line-beginning-position))
+                                 (b (line-end-position))
+                                 (line (buffer-substring a b)))
+                            (message "Testing indentation of %s:%05d: %s"
+                                     ,f lnum line)
+                            (unless (or (string-match "KNOWN INDENT BUG" line)
+                                        (equal (test-indent-trim line) ""))
+                              (indent-region a b nil)
+                              (should (equal line (buffer-substring
+                                                   (line-beginning-position)
+                                                   (line-end-position)))))
+                            (forward-line)))))))))
+
+(provide 'test-indent)
+;;; test-indent.el ends here

Re: ERT indentation testing

[Stefan Monnier]

> I'm concerned that it's failing a few tests.  I think the files are
> incorrect for the .rc and .sh content, don't know about the Prolog.

>    FAILED  test-indent-2-prolog\.prolog
>    FAILED  test-indent-3-shell\.rc
>    FAILED  test-indent-4-shell\.sh

Check the current result of the test in test/indent.
If the same problems show up there, it's a sign those have nothing to do
with your changes,


        Stefan