Re: master 12ca463 1/4: ; * test/lisp/textmodes/css-mode-tests.el: Add TODO.

Re: master 12ca463 1/4: ; * test/lisp/textmodes/css-mode-tests.el: Add TODO.

[Dmitry Gutov]

On 21.09.2020 17:46, Stefan Kangas wrote:
> +;; TODO: Convert these into unit proper tests instead of using an
> +;;       external file.

I wonder: did we ever decide to switch from manual/indent tests to ERT 
tests wholesale?

I'm asking as a developer from started with the latter and then switched 
mostly to the former, due to the ease of debugging.

The manual/indent tests also have an automated runner, which uses the 
'diff' program.

Re: master 12ca463 1/4: ; * test/lisp/textmodes/css-mode-tests.el: Add TODO.

[Stefan Kangas]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 21.09.2020 17:46, Stefan Kangas wrote:
>> +;; TODO: Convert these into unit proper tests instead of using an
>> +;;       external file.
>
> I wonder: did we ever decide to switch from manual/indent tests to ERT
> tests wholesale?

I was working under the assumption that these tests existed in their
current form only for historical reasons.  So here I am, wreaking
general havoc.

> I'm asking as a developer from started with the latter and then switched
> mostly to the former, due to the ease of debugging.
>
> The manual/indent tests also have an automated runner, which uses the
> 'diff' program.

I think that the manual tests fail on at least two accounts:

  1. There is no automated verification of success.

  2. They don't run with "make check".

That said, please feel free to revert any commits you disagree with or
that gets in the way of your work.  I think it goes without saying that
I don't want to make debugging harder for you or anyone else.

But if there are limitations in debugging with ert, perhaps we should
work on improving its capabilities?

Best regards,
Stefan Kangas

Re: master 12ca463 1/4: ; * test/lisp/textmodes/css-mode-tests.el: Add TODO.

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> But if there are limitations in debugging with ert, perhaps we should
> work on improving its capabilities?

Yeah, more automatic checks are better, but they can sometimes be
cumbersome to work with manually...  Fixing the latter somehow would be
nice.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 12ca463 1/4: ; * test/lisp/textmodes/css-mode-tests.el: Add TODO.

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Yeah, more automatic checks are better, but they can sometimes be
> cumbersome to work with manually...  Fixing the latter somehow would be
> nice.

I usually prefer running tests non-interactively.  My main issue is
that I have to manually scroll to find the failing test in the
compilation buffer.  Perhaps we should adapt the ert format to be more
easily parsable by compilation-mode?  ISTR a discussion like that not
long ago.

I would also like the option to see the full backtrace for a test on
"make check" et al.

Re: master 12ca463 1/4: ; * test/lisp/textmodes/css-mode-tests.el: Add TODO.

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> Yeah, more automatic checks are better, but they can sometimes be
>> cumbersome to work with manually...  Fixing the latter somehow would be
>> nice.
>
> I usually prefer running tests non-interactively.  My main issue is
> that I have to manually scroll to find the failing test in the
> compilation buffer.

I was thinking of using the ert tests manually (i.e., in a buffer),
which is what I do when developing the tests.

This is sometimes awkward with some of the tests that emulate a lot of
buffer stuff, and I assumed that that was what Dmitry was referring to?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 12ca463 1/4: ; * test/lisp/textmodes/css-mode-tests.el: Add TODO.

[Dmitry Gutov]

On 22.09.2020 18:06, Lars Ingebrigtsen wrote:
> This is sometimes awkward with some of the tests that emulate a lot of
> buffer stuff, and I assumed that that was what Dmitry was referring to?

Indeed.

ERT allows you to debug a test, and even pop up a buffer where the 
failure happened. But in a lot of cases it is nevertheless easier to 
debug indentation in a "simple" buffer.

Re: master 12ca463 1/4: ; * test/lisp/textmodes/css-mode-tests.el: Add TODO.

[Stefan Monnier]

FWIW, when I saw Stefan (the other)'s change, I thought "oh well, it was
bound to happen", because just like Dmitry, I find those manual tests
much easier to deal with when the time comes to fix the code.

But I didn't say anything because indeed, it's better if those tests are
run as part of "make check".

Of course, if someone could find it in their heart/schedule to write ERT
tests which use the "manual" files, I'd get have my cake and eat it to.


        Stefan

Re: master 12ca463 1/4: ; * test/lisp/textmodes/css-mode-tests.el: Add TODO.

[Stefan Kangas]

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

> Of course, if someone could find it in their heart/schedule to write ERT
> tests which use the "manual" files, I'd get have my cake and eat it to.

Do you mean like in commit b63c3535b0, or do you have something else in
mind?

Re: master 12ca463 1/4: ; * test/lisp/textmodes/css-mode-tests.el: Add TODO.

[Stefan Monnier]

>> Of course, if someone could find it in their heart/schedule to write ERT
>> tests which use the "manual" files, I'd get have my cake and eat it to.
> Do you mean like in commit b63c3535b0,

Yes (tho minus the "TODO" ;-)


        Stefan

Re: master 12ca463 1/4: ; * test/lisp/textmodes/css-mode-tests.el: Add TODO.

[Stefan Kangas]

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

>>> Of course, if someone could find it in their heart/schedule to write ERT
>>> tests which use the "manual" files, I'd get have my cake and eat it to.
>> Do you mean like in commit b63c3535b0,
>
> Yes (tho minus the "TODO" ;-)

Good, then let's hope that everyone can agree to that.

Meanwhile, anyone who feels disappointed that their favorite file got
removed and converted into unit tests, please just revert the offending
commit and we can re-do that work along the lines proposed above.

(And yes, I'll remove the relevant TODO:s... :-)

Re: master 12ca463 1/4: ; * test/lisp/textmodes/css-mode-tests.el: Add TODO.

[Dmitry Gutov]

On 21.09.2020 21:23, Stefan Kangas wrote:

> I was working under the assumption that these tests existed in their
> current form only for historical reasons.  So here I am, wreaking
> general havoc.

As was I, at a certain time in the past.

>> I'm asking as a developer from started with the latter and then switched
>> mostly to the former, due to the ease of debugging.
>>
>> The manual/indent tests also have an automated runner, which uses the
>> 'diff' program.
> 
> I think that the manual tests fail on at least two accounts:
> 
>    1. There is no automated verification of success.

They can even work semi-automatically, try:

$ cd test/manual/indent

$ make ruby.rb.test
../../../src/emacs --batch ruby.rb \
     --eval '(indent-region (point-min) (point-max) nil)' \
     --eval '(write-region (point-min) (point-max) "ruby.rb.new")'
Indenting region...
Indenting region...done
diff -u -B ruby.rb ruby.rb.new

Unfortunately, 'make all' doesn't work as expected there, only running 
that test for octave.m (which currently fails, to boot).

>    2. They don't run with "make check".

I think it can be made to run the above tests.

Wish someone with more Makefile-related experience than myself, as well 
as free time, could look into it.

> That said, please feel free to revert any commits you disagree with or
> that gets in the way of your work.  I think it goes without saying that
> I don't want to make debugging harder for you or anyone else.

I guess unless I do the above, I don't have a moral standing to revert them?

Having an ERT test per each file in there might seem a tad redundant 
given the above option, but maybe there are good reasons why nobody has 
fixed the situations yet. As long as said manual test files are present, 
in one form or another, the debugging workflows can be retained.

> But if there are limitations in debugging with ert, perhaps we should
> work on improving its capabilities?

It's capable enough in that regard (e.g. one can press 'd' and drop into 
the debugger, in the target buffer, exactly at the point of failure), 
yet still it requires more ceremony than the "manual" approach.

Re: master 12ca463 1/4: ; * test/lisp/textmodes/css-mode-tests.el: Add TODO.

[Stefan Kangas]

Dmitry Gutov <dgutov@yandex.ru> writes:

>> That said, please feel free to revert any commits you disagree with or
>> that gets in the way of your work.  I think it goes without saying that
>> I don't want to make debugging harder for you or anyone else.
>
> I guess unless I do the above, I don't have a moral standing to revert them?

Whatever you do, I think you have about the same moral standing as I had
to make the changes in the first place.  :-)

But maybe we can find a way to make everyone happy (see below).

> Having an ERT test per each file in there might seem a tad redundant
> given the above option, but maybe there are good reasons why nobody has
> fixed the situations yet. As long as said manual test files are present,
> in one form or another, the debugging workflows can be retained.

OK, that is encouraging.  Would arranging things more or less as in
commit b63c3535b0 be satisfactory then, or am I misunderstanding what
you mean?

>> But if there are limitations in debugging with ert, perhaps we should
>> work on improving its capabilities?
>
> It's capable enough in that regard (e.g. one can press 'd' and drop into
> the debugger, in the target buffer, exactly at the point of failure),
> yet still it requires more ceremony than the "manual" approach.

I agree that ERT's interactive UI could be more streamlined (that's why
I prefer running tests in batch).  Maybe we could think about ways to
remove some of the ceremony you are speaking of?