GNU bug report logs - #59028
[PATCH] Rewrite the `kill-buffer-delete-auto-save' tests

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59028 in the body.
You can then email your comments to 59028 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Rewrite the `kill-buffer-delete-auto-save' tests
Date: Fri, 04 Nov 2022 15:53:05 -0700

[Message part 1 (text/plain, inline)]

Tags: patch

Rationale is in the commit description, but it boils down to these tests
being annoying for me recently.  I like to M-x eval-buffer and run tests
interactively, but these tests seemed to work nicely only in batch mode.
In interactive mode they would both fail and issue annoying prompts
"save this modified buffer?" prompts.  In fixing them, I figured out
what they were trying to test and attempted to make that more robust and
clear.

[0003-Rewrite-the-kill-buffer-delete-auto-save-tests.patch (text/patch, attachment)]

Message #8 received at 59028 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: 59028 <at> debbugs.gnu.org
Subject: Re: bug#59028: [PATCH] Rewrite the `kill-buffer-delete-auto-save'
 tests
Date: Thu, 10 Nov 2022 11:56:40 +0200

> From: Matt Armstrong <matt <at> rfc20.org>
> Date: Fri, 04 Nov 2022 15:53:05 -0700
> 
> Rationale is in the commit description, but it boils down to these tests
> being annoying for me recently.  I like to M-x eval-buffer and run tests
> interactively, but these tests seemed to work nicely only in batch mode.
> In interactive mode they would both fail and issue annoying prompts
> "save this modified buffer?" prompts.  In fixing them, I figured out
> what they were trying to test and attempted to make that more robust and
> clear.

Thanks, I installed this.

However, the tests are still quite talkative, and emit a lot of stuff
for which I see no purpose.  Can we shut up those "BEGIN
overlay-modification-hooks test-case ((insert-at . 1))" messages and
also the "Auto-saving...", "Auto-saving...done"?  Or do they serve
some useful purpose?

Message #15 received at 59028 <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 59028 <at> debbugs.gnu.org
Subject: Re: bug#59028: [PATCH] Rewrite the `kill-buffer-delete-auto-save'
 tests
Date: Tue, 15 Nov 2022 10:52:43 -0800

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Matt Armstrong <matt <at> rfc20.org>
>> Date: Fri, 04 Nov 2022 15:53:05 -0700
>>
>> Rationale is in the commit description, but it boils down to these tests
>> being annoying for me recently.  I like to M-x eval-buffer and run tests
>> interactively, but these tests seemed to work nicely only in batch mode.
>> In interactive mode they would both fail and issue annoying prompts
>> "save this modified buffer?" prompts.  In fixing them, I figured out
>> what they were trying to test and attempted to make that more robust and
>> clear.
>
> Thanks, I installed this.
>
> However, the tests are still quite talkative, and emit a lot of stuff
> for which I see no purpose.  Can we shut up those "BEGIN
> overlay-modification-hooks test-case ((insert-at . 1))" messages and
> also the "Auto-saving...", "Auto-saving...done"?  Or do they serve
> some useful purpose?

See the attached patch, which either removes noisy messages or replaces
them with `ert-info'.  I didn't know about `ert-info' when I added the
noisy messages.

As for the "Auto-saving..." messages, they are printed by calls to
`message1' in fileio.c.  There may be a way to hide them, but see the
next paragraph.

There is a behavior difference between ert's interactive runner and
batch runner.  In interactive mode all `message' output is redirected to
a buffer normally hidden, made visible only by request after the test
runs, and I expect people will look at these only when debugging failed
tests.  In batch mode `message' output is printed to stdout (or maybe
stderr) by default, mixed with ert's other output.  It may be possible
to change ert's batch runner to hide each test's `message' output,
perhaps printing it only if the test fails.  Do we have a person who is
familiar with ert that we can run this idea by?

For example, this kind of noise would also be eliminated by such a
change:

   passed  35/48  simple-tests--undo (0.000591 sec)
Undo
Redo
Mark set
Undo in region
Redo
Redo
Redo
Undo
Undo
Undo
Undo
Undo
Undo
   passed  36/48  simple-tests--undo-equiv-table (0.000453 sec)
Mark set
Undo in region
Mark activated
Undo in region
Redo in region
Redo in region

[0001-Reduce-buffer-tests-noisiness-when-run-in-batch-mode.patch (text/x-diff, inline)]

From 717328dac712f3a53ca950bb1e202b4e0936ceae Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt <at> rfc20.org>
Date: Tue, 15 Nov 2022 10:33:00 -0800
Subject: [PATCH] Reduce buffer-tests noisiness when run in batch mode.

* test/src/buffer-tests.el (overlay-modification-hooks): Remove noisy
`message' calls and use `ert-info' to log context of test
failures.  (bug#59028)
(overlay-tests-start-recording-modification-hooks): ditto.
---
 test/src/buffer-tests.el | 77 ++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 42 deletions(-)

diff --git a/test/src/buffer-tests.el b/test/src/buffer-tests.el
index 3fc52eaf8b..2b6c974159 100644
--- a/test/src/buffer-tests.el
+++ b/test/src/buffer-tests.el
@@ -42,7 +42,6 @@ overlay-tests-start-recording-modification-hooks
      overlay
      hooks-property
      (list (lambda (ov &rest args)
-             (message "  %S called on %S with args %S" hooks-property ov args)
              (should inhibit-modification-hooks)
              (should (eq ov overlay))
              (push (list hooks-property args)
@@ -175,47 +174,41 @@ overlay-modification-hooks
                                     (t 1 2 0))
                                    (insert-behind-hooks
                                     (t 1 2 0)))))))
-      (message "BEGIN overlay-modification-hooks test-case %S" test-case)
-
-      ;; All three hooks ignore the overlay's `front-advance' and
-      ;; `rear-advance' option, so test both ways while expecting the same
-      ;; result.
-      (dolist (advance '(nil t))
-        (message "  advance is %S" advance)
-        (let-alist test-case
-          (with-temp-buffer
-            ;; Set up the temporary buffer and overlay as specified by
-            ;; the test case.
-            (insert (or .buffer-text "1234"))
-            (let ((overlay (make-overlay
-                            (or .overlay-beg 2)
-                            (or .overlay-end 4)
-                            nil
-                            advance advance)))
-              (message "  (buffer-string) is %S" (buffer-string))
-              (message "  overlay is %S" overlay)
-              (overlay-tests-start-recording-modification-hooks overlay)
-
-              ;; Modify the buffer, possibly inducing calls to the
-              ;; overlay's modification hooks.
-              (should (or .insert-at .replace))
-              (when .insert-at
-                (goto-char .insert-at)
-                (insert "x")
-                (message "  inserted \"x\" at %S, buffer-string now %S"
-                         .insert-at (buffer-string)))
-              (when .replace
-                (goto-char (point-min))
-                (search-forward .replace)
-                (replace-match "x")
-                (message "  replaced %S with \"x\"" .replace))
-
-              ;; Verify that the expected and actual modification hook
-              ;; calls match.
-              (should (equal
-                       .expected-calls
-                       (overlay-tests-get-recorded-modification-hooks
-                        overlay)))))))))
+      (ert-info ((format "test-case: %S" test-case))
+        ;; All three hooks ignore the overlay's `front-advance' and
+        ;; `rear-advance' option, so test both ways while expecting the same
+        ;; result.
+        (dolist (advance '(nil t))
+          (ert-info ((format "advance is %S" advance))
+            (let-alist test-case
+              (with-temp-buffer
+                ;; Set up the temporary buffer and overlay as specified by
+                ;; the test case.
+                (insert (or .buffer-text "1234"))
+                (let ((overlay (make-overlay
+                                (or .overlay-beg 2)
+                                (or .overlay-end 4)
+                                nil
+                                advance advance)))
+                  (overlay-tests-start-recording-modification-hooks overlay)
+
+                  ;; Modify the buffer, possibly inducing calls to the
+                  ;; overlay's modification hooks.
+                  (should (or .insert-at .replace))
+                  (when .insert-at
+                    (goto-char .insert-at)
+                    (insert "x"))
+                  (when .replace
+                    (goto-char (point-min))
+                    (search-forward .replace)
+                    (replace-match "x"))
+
+                  ;; Verify that the expected and actual modification hook
+                  ;; calls match.
+                  (should (equal
+                           .expected-calls
+                           (overlay-tests-get-recorded-modification-hooks
+                            overlay)))))))))))
 
 (ert-deftest overlay-modification-hooks-message-other-buf ()
   "Test for bug#21824.
-- 
2.35.1

Message #18 received at 59028 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Matt Armstrong <matt <at> rfc20.org>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 59028 <at> debbugs.gnu.org
Subject: Re: bug#59028: [PATCH] Rewrite the `kill-buffer-delete-auto-save'
 tests
Date: Tue, 15 Nov 2022 12:00:37 -0800

Matt Armstrong <matt <at> rfc20.org> writes:

> It may be possible to change ert's batch runner to hide each test's
> `message' output, perhaps printing it only if the test fails.

That's a good idea.  How about opening a feature request to track it?

Message #21 received at 59028 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: 59028 <at> debbugs.gnu.org
Subject: Re: bug#59028: [PATCH] Rewrite the `kill-buffer-delete-auto-save'
 tests
Date: Wed, 16 Nov 2022 16:41:10 +0200

> From: Matt Armstrong <matt <at> rfc20.org>
> Cc: 59028 <at> debbugs.gnu.org
> Date: Tue, 15 Nov 2022 10:52:43 -0800
> 
> > However, the tests are still quite talkative, and emit a lot of stuff
> > for which I see no purpose.  Can we shut up those "BEGIN
> > overlay-modification-hooks test-case ((insert-at . 1))" messages and
> > also the "Auto-saving...", "Auto-saving...done"?  Or do they serve
> > some useful purpose?
> 
> See the attached patch, which either removes noisy messages or replaces
> them with `ert-info'.  I didn't know about `ert-info' when I added the
> noisy messages.

Thanks, I installed this.

> As for the "Auto-saving..." messages, they are printed by calls to
> `message1' in fileio.c.  There may be a way to hide them, but see the
> next paragraph.

There is a way, and I took it.  Now there's no noise.

> There is a behavior difference between ert's interactive runner and
> batch runner.  In interactive mode all `message' output is redirected to
> a buffer normally hidden, made visible only by request after the test
> runs, and I expect people will look at these only when debugging failed
> tests.  In batch mode `message' output is printed to stdout (or maybe
> stderr) by default, mixed with ert's other output.  It may be possible
> to change ert's batch runner to hide each test's `message' output,
> perhaps printing it only if the test fails.  Do we have a person who is
> familiar with ert that we can run this idea by?
> 
> For example, this kind of noise would also be eliminated by such a
> change:
> 
>    passed  35/48  simple-tests--undo (0.000591 sec)
> Undo
> Redo
> Mark set
> Undo in region
> Redo
> Redo
> Redo
> Undo
> Undo
> Undo
> Undo
> Undo
> Undo
>    passed  36/48  simple-tests--undo-equiv-table (0.000453 sec)
> Mark set
> Undo in region
> Mark activated
> Undo in region
> Redo in region
> Redo in region

Yes, it would be good to be able to shut that up as well.

Thanks.

Can we now close this bug?

Message #26 received at 59028-done <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: Stefan Kangas <stefankangas <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 59028-done <at> debbugs.gnu.org
Subject: Re: bug#59028: [PATCH] Rewrite the `kill-buffer-delete-auto-save'
 tests
Date: Wed, 16 Nov 2022 10:54:27 -0800

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Matt Armstrong <matt <at> rfc20.org> writes:
>
>> It may be possible to change ert's batch runner to hide each test's
>> `message' output, perhaps printing it only if the test fails.
>
> That's a good idea.  How about opening a feature request to track it?

Done as bug#59317.

And with that, and Eli's commits, I'll close this bug.  Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.