bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[Helmut Eller]

When I run this file

(require 'ert)
(ert-deftest infloop0 () (while t))
(ert-deftest infloop1 () (while t))
(ert-deftest infloop2 () (while t))
(ert-deftest infloop3 () (while t))
(ert-deftest infloop4 () (while t))

with emacs -Q -nw -l infloops.el -eval "(ert t)"

I need to press C-g five times before ERT finally stops.

It would be much less annoying if ERT would quit after the first C-g.
This is especially important for test suites with more than 100 tests.


In GNU Emacs 24.3.50.1 (i686-pc-linux-gnu, GTK+ Version 2.24.10)
 of 2014-02-23 on ix
Windowing system distributor `The X.Org Foundation', version 11.0.11204000
System Description:	Debian GNU/Linux 7.1 (wheezy)

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[Helmut Eller]

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

On Sun, Feb 23 2014, Helmut Eller wrote:

> I need to press C-g five times before ERT finally stops.
>
> It would be much less annoying if ERT would quit after the first C-g.
> This is especially important for test suites with more than 100 tests.

Here is a patch to fix this:

	Make it easier to abort a series of tests with C-g.

	* emacs-lisp/ert.el (ert-run-tests): Add "interactively" arg.  If
	interactively is true and a test was aborted then ask if the
	remaining tests should be aborted too.
	(ert-run-tests-batch, ert-run-tests-interactively): Pass in
	interactively arg.



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

diff --git lisp/emacs-lisp/ert.el lisp/emacs-lisp/ert.el
index 34041aa..aa01c3f 100644
--- lisp/emacs-lisp/ert.el
+++ lisp/emacs-lisp/ert.el
@@ -1238,7 +1238,7 @@ SELECTOR is the selector that was used to select TESTS."
         (funcall listener 'test-ended stats test result))
       (setf (ert--stats-current-test stats) nil))))
 
-(defun ert-run-tests (selector listener)
+(defun ert-run-tests (selector listener interactively)
   "Run the tests specified by SELECTOR, sending progress updates to LISTENER."
   (let* ((tests (ert-select-tests selector t))
          (stats (ert--make-stats tests selector)))
@@ -1249,10 +1249,14 @@ SELECTOR is the selector that was used to select TESTS."
           (let ((ert--current-run-stats stats))
             (force-mode-line-update)
             (unwind-protect
-                (progn
-                  (cl-loop for test in tests do
-                           (ert-run-or-rerun-test stats test listener))
-                  (setq abortedp nil))
+		(cl-loop for test in tests do
+			 (ert-run-or-rerun-test stats test listener)
+			 (when (and interactively
+				    (ert-test-quit-p
+				     (ert-test-most-recent-result test))
+				    (y-or-n-p "Abort testing? "))
+			   (cl-return))
+			 finally (setq abortedp nil))
               (setf (ert--stats-aborted-p stats) abortedp)
               (setf (ert--stats-end-time stats) (current-time))
               (funcall listener 'run-ended stats abortedp)))
@@ -1443,7 +1447,8 @@ Returns the stats object."
                                                  (ert-test-result-expected-p
                                                   test result))
                      (1+ (ert--stats-test-pos stats test))
-                     (ert-test-name test)))))))))
+                     (ert-test-name test)))))))
+   nil))
 
 ;;;###autoload
 (defun ert-run-tests-batch-and-exit (&optional selector)
@@ -1952,9 +1957,8 @@ and how to display message."
                                                       test result)))
                      (ert--results-update-stats-display-maybe ewoc stats)
                      (ewoc-invalidate ewoc node))))))))
-    (ert-run-tests
-     selector
-     listener)))
+    (ert-run-tests selector listener t)))
+
 ;;;###autoload
 (defalias 'ert 'ert-run-tests-interactively)
 

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[Helmut Eller]

On Sun, Feb 23 2014, Helmut Eller wrote:

[...]
> It would be much less annoying if ERT would quit after the first C-g.
> This is especially important for test suites with more than 100 tests.

Has somebody time to look at this issue, please?  This bug is quite a
showstopper for me.

Helmut

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[Andreas Röhler]

On 07.09.2014 12:24, Helmut Eller wrote:
> On Sun, Feb 23 2014, Helmut Eller wrote:
>
> [...]
>> It would be much less annoying if ERT would quit after the first C-g.
>> This is especially important for test suites with more than 100 tests.
>
> Has somebody time to look at this issue, please?  This bug is quite a
> showstopper for me.
>
> Helmut
>

Hi,

just out of couriosity: why not run it from shell, which would not have that nuisance?

Andreas

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[Helmut Eller]

On Sun, Sep 07 2014, Andreas Röhler wrote:

> just out of couriosity: why not run it from shell, which would not
> have that nuisance?

I need to test display related code, like checking if the proper region
is visible; that wouldn't work in batch mode.  Also the *ert* buffer is
easier to browse than log files.

Helmut

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[Andreas Röhler]

On 07.09.2014 22:37, Helmut Eller wrote:
> On Sun, Sep 07 2014, Andreas Röhler wrote:
>
>> just out of couriosity: why not run it from shell, which would not
>> have that nuisance?
>
> I need to test display related code, like checking if the proper region
> is visible; that wouldn't work in batch mode.

Why not? But understand there might be some work left to write the related test, checks if being visible.

   Also the *ert* buffer is
> easier to browse than log files.
>

Get nice output here should something fail. Run this manually then...

Cheers,

Andreas

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[Helmut Eller]

On Mon, Sep 08 2014, Andreas Röhler wrote:

>> I need to test display related code, like checking if the proper region
>> is visible; that wouldn't work in batch mode.
>
> Why not? But understand there might be some work left to write the
> related test, checks if being visible.

Try this:
 emacs -batch -eval '(message "%s" (pos-visible-in-window-p (window-start)))'

It prints nil.  Shouldn't window-start always be visible?

>   Also the *ert* buffer is
>> easier to browse than log files.
>>
>
> Get nice output here should something fail. Run this manually then...

Sure, use what works best for a given problem.  My log output is long
and full of random calls to "message".

Helmut

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[Andreas Röhler]

On 08.09.2014 16:02, Helmut Eller wrote:
> On Mon, Sep 08 2014, Andreas Röhler wrote:
>
>>> I need to test display related code, like checking if the proper region
>>> is visible; that wouldn't work in batch mode.
>>
>> Why not? But understand there might be some work left to write the
>> related test, checks if being visible.
>
> Try this:
>   emacs -batch -eval '(message "%s" (pos-visible-in-window-p (window-start)))'
>
> It prints nil.  Shouldn't window-start always be visible?

I see, thanks.

BTW it also refused to message (point).

Andreas

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[Eli Zaretskii]

> From: Helmut Eller <eller.helmut@gmail.com>
> Date: Mon, 08 Sep 2014 16:02:53 +0200
> 
> Try this:
>  emacs -batch -eval '(message "%s" (pos-visible-in-window-p (window-start)))'
> 
> It prints nil.  Shouldn't window-start always be visible?

No, not when the window is not "live".

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[Helmut Eller]

On Tue, Sep 09 2014, Eli Zaretskii wrote:

>> From: Helmut Eller <eller.helmut@gmail.com>
>> Date: Mon, 08 Sep 2014 16:02:53 +0200
>> 
>> Try this:
>>  emacs -batch -eval '(message "%s" (pos-visible-in-window-p (window-start)))'
>> 
>> It prints nil.  Shouldn't window-start always be visible?
>
> No, not when the window is not "live".

The window in the example is the selected window and it is live:
 emacs -batch -eval '(message "%s" (window-live-p (selected-window)))'
=> t

Helmut

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[Eli Zaretskii]

> From: Helmut Eller <eller.helmut@gmail.com>
> Cc: 16853@debbugs.gnu.org
> Date: Tue, 09 Sep 2014 15:45:27 +0200
> 
> On Tue, Sep 09 2014, Eli Zaretskii wrote:
> 
> >> From: Helmut Eller <eller.helmut@gmail.com>
> >> Date: Mon, 08 Sep 2014 16:02:53 +0200
> >> 
> >> Try this:
> >>  emacs -batch -eval '(message "%s" (pos-visible-in-window-p (window-start)))'
> >> 
> >> It prints nil.  Shouldn't window-start always be visible?
> >
> > No, not when the window is not "live".
> 
> The window in the example is the selected window and it is live:
>  emacs -batch -eval '(message "%s" (window-live-p (selected-window)))'
> => t

Not in batch mode, it isn't.

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[Helmut Eller]

On Tue, Sep 09 2014, Eli Zaretskii wrote:

>> From: Helmut Eller <eller.helmut@gmail.com>
>> Cc: 16853@debbugs.gnu.org
>> Date: Tue, 09 Sep 2014 15:45:27 +0200
>> 
>> On Tue, Sep 09 2014, Eli Zaretskii wrote:
>> 
>> >> From: Helmut Eller <eller.helmut@gmail.com>
>> >> Date: Mon, 08 Sep 2014 16:02:53 +0200
>> >> 
>> >> Try this:
>> >>  emacs -batch -eval '(message "%s" (pos-visible-in-window-p
>> >> (window-start)))'
>> >> 
>> >> It prints nil.  Shouldn't window-start always be visible?
>> >
>> > No, not when the window is not "live".
>> 
>> The window in the example is the selected window and it is live:
>>  emacs -batch -eval '(message "%s" (window-live-p (selected-window)))'
>> => t
>
> Not in batch mode, it isn't.

Are you wasting time on purpose or what is this?  The point of the
example was to show that pos-visible-in-window-p doesn't work normally
in batch mode.  Claiming that the window isn't live when window-live-p
obviously returns t is IMO silly.

Helmut

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[Eli Zaretskii]

> From: Helmut Eller <eller.helmut@gmail.com>
> Cc: 16853@debbugs.gnu.org
> Date: Tue, 09 Sep 2014 16:14:05 +0200
> 
> Are you wasting time on purpose or what is this?

I've just stopped.

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[Lars Ingebrigtsen]

Helmut Eller <eller.helmut@gmail.com> writes:

> Here is a patch to fix this:
>
> 	Make it easier to abort a series of tests with C-g.
>
> 	* emacs-lisp/ert.el (ert-run-tests): Add "interactively" arg.  If
> 	interactively is true and a test was aborted then ask if the
> 	remaining tests should be aborted too.
> 	(ert-run-tests-batch, ert-run-tests-interactively): Pass in
> 	interactively arg.

I think this makes sense, but the discussion seemed to go off on a
tangent, and the patch wasn't applied.

And I'm not familiar with the ert internals, but could somebody look at
the patch and see whether it looks reasonable?

> diff --git lisp/emacs-lisp/ert.el lisp/emacs-lisp/ert.el
> index 34041aa..aa01c3f 100644
> --- lisp/emacs-lisp/ert.el
> +++ lisp/emacs-lisp/ert.el
> @@ -1238,7 +1238,7 @@ SELECTOR is the selector that was used to select TESTS."
>          (funcall listener 'test-ended stats test result))
>        (setf (ert--stats-current-test stats) nil))))
>  
> -(defun ert-run-tests (selector listener)
> +(defun ert-run-tests (selector listener interactively)
>    "Run the tests specified by SELECTOR, sending progress updates to LISTENER."
>    (let* ((tests (ert-select-tests selector t))
>           (stats (ert--make-stats tests selector)))
> @@ -1249,10 +1249,14 @@ SELECTOR is the selector that was used to select TESTS."
>            (let ((ert--current-run-stats stats))
>              (force-mode-line-update)
>              (unwind-protect
> -                (progn
> -                  (cl-loop for test in tests do
> -                           (ert-run-or-rerun-test stats test listener))
> -                  (setq abortedp nil))
> +		(cl-loop for test in tests do
> +			 (ert-run-or-rerun-test stats test listener)
> +			 (when (and interactively
> +				    (ert-test-quit-p
> +				     (ert-test-most-recent-result test))
> +				    (y-or-n-p "Abort testing? "))
> +			   (cl-return))
> +			 finally (setq abortedp nil))
>                (setf (ert--stats-aborted-p stats) abortedp)
>                (setf (ert--stats-end-time stats) (current-time))
>                (funcall listener 'run-ended stats abortedp)))
> @@ -1443,7 +1447,8 @@ Returns the stats object."
>                                                   (ert-test-result-expected-p
>                                                    test result))
>                       (1+ (ert--stats-test-pos stats test))
> -                     (ert-test-name test)))))))))
> +                     (ert-test-name test)))))))
> +   nil))
>  
>  ;;;###autoload
>  (defun ert-run-tests-batch-and-exit (&optional selector)
> @@ -1952,9 +1957,8 @@ and how to display message."
>                                                        test result)))
>                       (ert--results-update-stats-display-maybe ewoc stats)
>                       (ewoc-invalidate ewoc node))))))))
> -    (ert-run-tests
> -     selector
> -     listener)))
> +    (ert-run-tests selector listener t)))
> +
>  ;;;###autoload
>  (defalias 'ert 'ert-run-tests-interactively)
>  
>

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

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[npostavs]

severity 16853 wishlist
quit

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I think this makes sense, but the discussion seemed to go off on a
> tangent, and the patch wasn't applied.
>
> And I'm not familiar with the ert internals, but could somebody look at
> the patch and see whether it looks reasonable?

Looks good to me.  I'll push to master in week or so, if there are no
objections.

bug#16853: 24.3.50; ert-run-tests-interactively difficult to abort

[npostavs]

tags 16853 fixed
close 16853 26.1
quit

npostavs@users.sourceforge.net writes:

>
> Looks good to me.  I'll push to master in week or so, if there are no
> objections.

Done [1: ef44346782].

1: 2017-03-24 20:13:17 -0400 ef443467826dc33c6e58081e7af7c68905f6db26
  Make it easier to abort a series of tests with C-g