Re: [Emacs-diffs] master 75336a2: Merge branch 'feature/standard-test-location'

Re: [Emacs-diffs] master 75336a2: Merge branch 'feature/standard-test-location'

[Dmitry Gutov]

Hi Phillip,

On 11/26/2015 11:14 PM, Phillip Lord wrote:
> branch: master
> commit 75336a29460752700e4e424a9c7aa932cc237689
> Merge: 82a8ad2 6fdd750
> Author: Phillip Lord <phillip.lord@russet.org.uk>
> Commit: Phillip Lord <phillip.lord@russet.org.uk>
>
>      Merge branch 'feature/standard-test-location'
>   test/{ => manual}/BidiCharacterTest.txt            |    0
>   test/{ => manual}/biditest.el                      |    0
>   test/{ => manual}/cedet/cedet-utests.el            |    0
>   ...
>   test/{ => manual}/etags/CTAGS.good                 |    0
>   ...
>   test/{ => manual}/indent/Makefile                  |    0
>   ...
>   test/{ => manual}/redisplay-testsuite.el           |    0
>   test/{ => manual}/rmailmm.el                       |    0

Are all of these really manual?

I can't speak for cedet of etags tests, but the indentation tests can be 
run in an automatic fashion, at least from their directory:

cd test/indent
make ruby.rb.test

And 'make all' runs all indentation tests, even though it unfortunately 
stops at the first failing one.

Re: [Emacs-diffs] master 75336a2: Merge branch 'feature/standard-test-location'

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 28 Nov 2015 20:56:12 +0200
> 
> >   test/{ => manual}/BidiCharacterTest.txt            |    0
> >   test/{ => manual}/biditest.el                      |    0
> >   test/{ => manual}/cedet/cedet-utests.el            |    0
> >   ...
> >   test/{ => manual}/etags/CTAGS.good                 |    0
> >   ...
> >   test/{ => manual}/indent/Makefile                  |    0
> >   ...
> >   test/{ => manual}/redisplay-testsuite.el           |    0
> >   test/{ => manual}/rmailmm.el                       |    0
> 
> Are all of these really manual?
> 
> I can't speak for cedet of etags tests

The etags tests are all run by "make" in their directory.

Re: [Emacs-diffs] master 75336a2: Merge branch 'feature/standard-test-location'

[Phillip Lord]

Dmitry Gutov <dgutov@yandex.ru> writes:
>>      Merge branch 'feature/standard-test-location'
>>   test/{ => manual}/BidiCharacterTest.txt            |    0
>>   test/{ => manual}/biditest.el                      |    0
>>   test/{ => manual}/cedet/cedet-utests.el            |    0
>>   ...
>>   test/{ => manual}/etags/CTAGS.good                 |    0
>>   ...
>>   test/{ => manual}/indent/Makefile                  |    0
>>   ...
>>   test/{ => manual}/redisplay-testsuite.el           |    0
>>   test/{ => manual}/rmailmm.el                       |    0
>
> Are all of these really manual?
>
> I can't speak for cedet of etags tests, but the indentation tests can be run
> in an automatic fashion, at least from their directory:
>
> cd test/indent
> make ruby.rb.test
>
> And 'make all' runs all indentation tests, even though it unfortunately stops
> at the first failing one.

Possibly not. I just inverted the logic -- I moved all of the files from
"automated" into top-level, and everything from top-level into "manual".

They are "manual" in that, they are in the manual directory and are not
automatically run from the make file.

In the long term it might be nice to either link these into "make check"
from the test dir, or make them ert driven and move them to the main
directory. I've been working on a new package with some higher-level
predicates which should do most of what the indent tests do, which
should help.

Phil

Re: [Emacs-diffs] master 75336a2: Merge branch 'feature/standard-test-location'

[Dmitry Gutov]

On 11/28/2015 09:20 PM, Phillip Lord wrote:

> Possibly not. I just inverted the logic -- I moved all of the files from
> "automated" into top-level, and everything from top-level into "manual".

All right, thanks, I can see the idea now. I haven't noticed that 
there's no 'automatic' directory now.

> In the long term it might be nice to either link these into "make check"
> from the test dir,

And then, the 'manual' directory will disappear? Sounds good.

> or make them ert driven and move them to the main
> directory.

In the previous discussions, Stefan has been a been proponent to keeping 
indentation tests as sample files, instead of ERT scripts.

Because, as the argument went, it's much easier to work on the 
indentation code when the file contents are right before your eyes, and 
you can interact with them live.

Initially, I've put a number of indentation tests into 
automated/ruby-mode-tests.el, but now I more or less agree with the 
above position.

> I've been working on a new package with some higher-level
> predicates which should do most of what the indent tests do, which
> should help.

There is a `ruby-should-indent-buffer' helper in the aforementioned 
file. I haven't really needed much else.

Re: [Emacs-diffs] master 75336a2: Merge branch 'feature/standard-test-location'

[Phillip Lord]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 11/28/2015 09:20 PM, Phillip Lord wrote:
>
>> Possibly not. I just inverted the logic -- I moved all of the files from
>> "automated" into top-level, and everything from top-level into "manual".
>
> All right, thanks, I can see the idea now. I haven't noticed that there's no
> 'automatic' directory now.

In my experience, pulling from master leaves the "automatic" directory
there, although it's gone from git. I'd expected it to delete when the
equivalent was removed from git, but apparently not.

>> In the long term it might be nice to either link these into "make check"
>> from the test dir,
>
> And then, the 'manual' directory will disappear? Sounds good.

Well, there is a place for manual test material. ERT is good for doing
unit tests, but some integration tests are likely to remain. But if a
test is automatable, then I see no reason it should not run.

>> or make them ert driven and move them to the main
>> directory.
>
> In the previous discussions, Stefan has been a been proponent to keeping
> indentation tests as sample files, instead of ERT scripts.
>
> Because, as the argument went, it's much easier to work on the indentation
> code when the file contents are right before your eyes, and you can interact
> with them live.

I'd agree with this entirely, but this doesn't contradict the idea the
idea of using ERT. I mean, ERT can read from a file right? It was for
this purpose that I had the idea of "-resource" directories in the
file location specification.

> Initially, I've put a number of indentation tests into
> automated/ruby-mode-tests.el, but now I more or less agree with the above
> position.

If we need an indent test location, then I think this would be good to
support, but it would be nice to have a standard naming scheme. I'd like
to add support to the make file so that tests (or the .log file) depend
on the file that they are testing. At the moment, if you edit (for
example) lisp/calc/calc.el, and then run make check, the calc-tests.el
file does not get run. This makes no sense to me. The same should be
true with indentation tests.


>> I've been working on a new package with some higher-level
>> predicates which should do most of what the indent tests do, which
>> should help.
>
> There is a `ruby-should-indent-buffer' helper in the aforementioned
> file. I haven't really needed much else.

My package lets you do something like...

(ert-deftest indent
   (should
      (sisyphus-indentation=
         'emacs-lisp-mode
         "emacs-lisp-indented.el"
         "emacs-lisp-unindented.el")))

If it fails, then it runs diff on the results.

I hadn't thought of the idea of unindenting first -- I shall add that.

It's got a "with-temp-buffers" macro also (for when you need several at
once), and "with-preserved-buffer-list".

Anyway, that package is early days yet. We shall see if it become useful
as it develops.

Phil

Re: [Emacs-diffs] master 75336a2: Merge branch 'feature/standard-test-location'

[Dmitry Gutov]

On 11/28/2015 11:20 PM, Phillip Lord wrote:

> In my experience, pulling from master leaves the "automatic" directory
> there, although it's gone from git. I'd expected it to delete when the
> equivalent was removed from git, but apparently not.

Apparently it's being left there as long as there are some untracked (or 
ignored) files inside.

> Well, there is a place for manual test material. ERT is good for doing
> unit tests, but some integration tests are likely to remain. But if a
> test is automatable, then I see no reason it should not run.

Right. But all "manual" tests we have now should be automatable, I think.

> I'd agree with this entirely, but this doesn't contradict the idea the
> idea of using ERT. I mean, ERT can read from a file right? It was for
> this purpose that I had the idea of "-resource" directories in the
> file location specification.

That sounds fine.

> If we need an indent test location, then I think this would be good to
> support, but it would be nice to have a standard naming scheme. I'd like
> to add support to the make file so that tests (or the .log file) depend
> on the file that they are testing. At the moment, if you edit (for
> example) lisp/calc/calc.el, and then run make check, the calc-tests.el
> file does not get run. This makes no sense to me. The same should be
> true with indentation tests.

'make check' runs all tests, doesn't it? Including calc-tests.el.

It would be nice to be able to run the tests corresponding to the 
current file only, but Makefile might not be the best place to implement 
that.

> My package lets you do something like...
>
> (ert-deftest indent
>     (should
>        (sisyphus-indentation=
>           'emacs-lisp-mode
>           "emacs-lisp-indented.el"
>           "emacs-lisp-unindented.el")))

The .el extension points to emacs-lisp-mode already, doesn't it? There's 
also the issue of customization variables that affect indentation: thus 
far we've put them into the files, but we might as well specify them 
programmatically here.

> I hadn't thought of the idea of unindenting first -- I shall add that.

Actually, having -unindented files seems largely unnecessary to me. I 
ported that idea straight from the test suite inside the Ruby core, but 
the benefits are relatively small, and that would double the number of 
files. But hey, we can try it both ways.

> It's got a "with-temp-buffers" macro also (for when you need several at
> once), and "with-preserved-buffer-list".
>
> Anyway, that package is early days yet. We shall see if it become useful
> as it develops.

Have you considered just adding a few functions to ert-x? As a first 
step, I mean.

Re: automated indent tests

[Stephen Leake]

Dmitry Gutov <dgutov@yandex.ru> writes:

>> My package lets you do something like...
>>
>> (ert-deftest indent
>>     (should
>>        (sisyphus-indentation=
>>           'emacs-lisp-mode
>>           "emacs-lisp-indented.el"
>>           "emacs-lisp-unindented.el")))
>
> The .el extension points to emacs-lisp-mode already, doesn't it?

For tests where auto-mode-alists fails, we could require a file local
variable to set the mode. That would make it easier to debug the test
interactively as well.

> There's also the issue of customization variables that affect
> indentation: thus far we've put them into the files, but we might as
> well specify them programmatically here.

I prefer them in the files as file local variables; that makes it easier
to debug the test interactively.

Unless you want to use one unindented file, indent it with several
different indent options, and compare each result to a different
known-good file. That would be a good use case.

>> I hadn't thought of the idea of unindenting first -- I shall add that.
>
> Actually, having -unindented files seems largely unnecessary to me. I
> ported that idea straight from the test suite inside the Ruby core,
> but the benefits are relatively small, and that would double the
> number of files. But hey, we can try it both ways.

In some cases, the final indentation can depend on the starting point.
Certainly that will be true for languages where the syntax relies on the
indentation (Python etc).

So maybe allowing for both forms of test would be best.


For my ada-mode indentation tests, I store only the known-good file in
the resource directory, autmatically de-indent every line by two spaces,
then reindent and compare to the original. That has been sufficient so
far.

-- 
-- Stephe

Re: automated indent tests

[Dmitry Gutov]

On 11/29/2015 07:50 PM, Stephen Leake wrote:

> For tests where auto-mode-alists fails, we could require a file local
> variable to set the mode. That would make it easier to debug the test
> interactively as well.

> I prefer them in the files as file local variables; that makes it easier
> to debug the test interactively.
>
> Unless you want to use one unindented file, indent it with several
> different indent options, and compare each result to a different

> In some cases, the final indentation can depend on the starting point.
> Certainly that will be true for languages where the syntax relies on the
> indentation (Python etc).
>
> So maybe allowing for both forms of test would be best.

All good ideas.

> For my ada-mode indentation tests, I store only the known-good file in
> the resource directory, autmatically de-indent every line by two spaces,
> then reindent and compare to the original. That has been sufficient so
> far.

On top-level, there's nowhere to de-indent. So maybe add two spaces of 
indentation instead?

And also, most indentation functions don't change the indentation inside 
multiline string literals. So that would have to be taken care of 
somehow, maybe with a "this is a string" annotation comments.

Re: [Emacs-diffs] master 75336a2: Merge branch 'feature/standard-test-location'

[Phillip Lord]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 11/28/2015 11:20 PM, Phillip Lord wrote:
>
>> In my experience, pulling from master leaves the "automatic" directory
>> there, although it's gone from git. I'd expected it to delete when the
>> equivalent was removed from git, but apparently not.
>
> Apparently it's being left there as long as there are some untracked (or
> ignored) files inside.

Ah okay. So if you make distclean before pulling it would be okay.

>> Well, there is a place for manual test material. ERT is good for doing
>> unit tests, but some integration tests are likely to remain. But if a
>> test is automatable, then I see no reason it should not run.
>
> Right. But all "manual" tests we have now should be automatable, I think.

I'll have a look at this maybe, although I'd prefer the original authors
of the tests ported them.


>> If we need an indent test location, then I think this would be good to
>> support, but it would be nice to have a standard naming scheme. I'd like
>> to add support to the make file so that tests (or the .log file) depend
>> on the file that they are testing. At the moment, if you edit (for
>> example) lisp/calc/calc.el, and then run make check, the calc-tests.el
>> file does not get run. This makes no sense to me. The same should be
>> true with indentation tests.
>
> 'make check' runs all tests, doesn't it? Including calc-tests.el.

No. make check runs all tests that are not up to date. So, it runs
calc-tests.el only if calc-tests.el has been changed.

> It would be nice to be able to run the tests corresponding to the
> current file only, but Makefile might not be the best place to
> implement that.

The make file before my changes allowed

make calc-tests
make calc-tests.log

The former dumps output to screen, the latter to file.

Now you can do any of:

make calc-tests
make lisp/calc/calc-tests
make lisp/calc/calc-tests.log

The make file on feature/tests-depend-on-source also makes tests depend
on the source file (based on file name). It uses Emacs to generate a
include make file like so:

lisp/calc/calc-tests.log: ../lisp/calc/calc.elc
lisp/calendar/icalendar-tests.log: ../lisp/calendar/icalendar.elc
lisp/emacs-lisp/cl-generic-tests.log: ../lisp/emacs-lisp/cl-generic.elc

Seems to work. If you are building Emacs a lot, then you just switch
from "make" to "make check" and it runs the relevant tests.

At the moment, I think, tests do not get routinely run. The coverage
hydra build has been failing on tests for 2 or 3 weeks, for instance.


> The .el extension points to emacs-lisp-mode already, doesn't it?

Then you are testing two things: auto-mode-alist *and* indentation.

>> I hadn't thought of the idea of unindenting first -- I shall add that.
>
> Actually, having -unindented files seems largely unnecessary to me. I ported
> that idea straight from the test suite inside the Ruby core, but the benefits
> are relatively small, and that would double the number of files. But hey, we
> can try it both ways.

No, I agree with you. unindenting first seems generally better to me.


>> It's got a "with-temp-buffers" macro also (for when you need several at
>> once), and "with-preserved-buffer-list".
>>
>> Anyway, that package is early days yet. We shall see if it become useful
>> as it develops.
>
> Have you considered just adding a few functions to ert-x? As a first step, I
> mean.

Not yet, no. I wanted to get something working and useful first. The
problem with adding it to ert-x is that it's core, and I think I'd
rather have it in ELPA in the short term.

Phil

Re: [Emacs-diffs] master 75336a2: Merge branch 'feature/standard-test-location'

[Stephen Leake]

phillip.lord@russet.org.uk (Phillip Lord) writes:

>> The .el extension points to emacs-lisp-mode already, doesn't it?
>
> Then you are testing two things: auto-mode-alist *and* indentation.

Yes. But that's a Good Thing.

And it will significantly reduce perceived "noise" in the test sources,
which is also a Good Thing.

The need to set the mode via a local variable documents that you are not
expecting auto-mode-alist to handle it.

>> Have you considered just adding a few functions to ert-x? As a first step, I
>> mean.
>
> Not yet, no. I wanted to get something working and useful first. The
> problem with adding it to ert-x is that it's core, and I think I'd
> rather have it in ELPA in the short term.

I think "it" here is your new test macros, that make it easier to write
indentation tests?

They are useful for testing core code, so they should be in core. At
least until we get "core ELPA" packages working; then this would be a
candidate for moving to Gnu ELPA.

Putting them in a separate package for now probably makes sense; once
they are fully polished we can decide if they should be in ert or ert-x.

--
-- Stephe

Re: [Emacs-diffs] master 75336a2: Merge branch 'feature/standard-test-location'

[Phillip Lord]

Stephen Leake <stephen_leake@stephe-leake.org> writes:

> phillip.lord@russet.org.uk (Phillip Lord) writes:
>
>>> The .el extension points to emacs-lisp-mode already, doesn't it?
>>
>> Then you are testing two things: auto-mode-alist *and* indentation.
>
> Yes. But that's a Good Thing.
>
> And it will significantly reduce perceived "noise" in the test sources,
> which is also a Good Thing.
>
> The need to set the mode via a local variable documents that you are not
> expecting auto-mode-alist to handle it.

I think that you are partly right about this, although, of course, you
are assuming that indentation tests WILL be in files. As is stands
sisyphus does not make this assumption -- it's possible to drop code
inline into tests also. There are advantages to this for short pieces of
code.

Still, you are correct -- auto-mode-alist should be usable if wanted.


>>> Have you considered just adding a few functions to ert-x? As a first step, I
>>> mean.
>>
>> Not yet, no. I wanted to get something working and useful first. The
>> problem with adding it to ert-x is that it's core, and I think I'd
>> rather have it in ELPA in the short term.
>
> I think "it" here is your new test macros, that make it easier to write
> indentation tests?
>
> They are useful for testing core code, so they should be in core. At
> least until we get "core ELPA" packages working; then this would be a
> candidate for moving to Gnu ELPA.
>
> Putting them in a separate package for now probably makes sense; once
> they are fully polished we can decide if they should be in ert or ert-x.


I'd agree with all this also. I think that my packages will be good for
testing core. I'm not making any decisions till I find out how long it
takes me write. Maybe "core ELPA" will be working by then. Who knows!

Phil

Re: [Emacs-diffs] master 75336a2: Merge branch 'feature/standard-test-location'

[Ken Brown]

On 11/28/2015 1:56 PM, Dmitry Gutov wrote:
> I can't speak for cedet of etags tests, but the indentation tests can be
> run in an automatic fashion, at least from their directory:
>
> cd test/indent
> make ruby.rb.test
>
> And 'make all' runs all indentation tests, even though it unfortunately
> stops at the first failing one.

What about 'make -k'?

Ken

Re: [Emacs-diffs] master 75336a2: Merge branch 'feature/standard-test-location'

[Dmitry Gutov]

On 11/28/2015 10:15 PM, Ken Brown wrote:

> What about 'make -k'?

It shows lots of spurious test failures, at least after you run 'make 
-k' twice or more.

They look like this:

Makefile:13: recipe for target 'pascal.pas.new.test' failed

The test run leaves *.*.new files behind to help diagnose failures, and 
apparently the 'all' target picks them up before performing 'clean'.

Extracting that part to a separate task doesn't help. If someone more 
experienced with Make were to look at the problem, that would be great.