* bug#59028: [PATCH] Rewrite the `kill-buffer-delete-auto-save' tests
@ 2022-11-04 22:53 Matt Armstrong
2022-11-10 9:56 ` Eli Zaretskii
0 siblings, 1 reply; 6+ messages in thread
From: Matt Armstrong @ 2022-11-04 22:53 UTC (permalink / raw)
To: 59028
[-- Attachment #1: Type: text/plain, Size: 450 bytes --]
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-Rewrite-the-kill-buffer-delete-auto-save-tests.patch --]
[-- Type: text/patch, Size: 7407 bytes --]
From c7a35edb0c62f5cc392f6748dbf409d5bbfa567c Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt@rfc20.org>
Date: Fri, 4 Nov 2022 12:43:30 -0700
Subject: [PATCH 3/6] Rewrite the `kill-buffer-delete-auto-save' tests
These tests had been annoying to me when run in interactive mode.
They failed to bind `kill-buffer-delete-auto-save' as needed, so they
depended on the user's settings, and they failed to mock out the
"Buffer modified, save?" prompt.
* test/src/buffer-tests.el (test-kill-buffer-auto-save): New helper
function that mocks the two different kinds of prompts that occur when
killing modified buffers that visit a file. Tests fail if expected
prompts are not issued.
(test-kill-buffer-auto-save-default): Use it, and explicitly bind
`kill-buffer-delete-auto-save' to nil.
(test-kill-buffer-auto-save-delete): Delete it.
(test-kill-buffer-auto-save-delete-yes): New test for the "yes" half
of the old `test-kill-buffer-auto-save-delete'.
(test-kill-buffer-auto-save-delete-yes): Ditto for the "no" half.
---
test/src/buffer-tests.el | 143 +++++++++++++++++++++++----------------
1 file changed, 85 insertions(+), 58 deletions(-)
diff --git a/test/src/buffer-tests.el b/test/src/buffer-tests.el
index a39d7d51de1..78da02fe0a6 100644
--- a/test/src/buffer-tests.el
+++ b/test/src/buffer-tests.el
@@ -8323,65 +8323,92 @@ test-remove-overlays
(remove-overlays)
(should (= (length (overlays-in (point-min) (point-max))) 0))))
-(ert-deftest test-kill-buffer-auto-save-default ()
- (ert-with-temp-file file
- (let (auto-save)
- ;; Always answer yes.
- (cl-letf (((symbol-function #'yes-or-no-p) (lambda (_) t)))
- (unwind-protect
- (progn
- (find-file file)
- (auto-save-mode t)
- (insert "foo\n")
- (should buffer-auto-save-file-name)
- (setq auto-save buffer-auto-save-file-name)
- (do-auto-save)
- (should (file-exists-p auto-save))
- (kill-buffer (current-buffer))
- (should (file-exists-p auto-save)))
- (when auto-save
- (ignore-errors (delete-file auto-save))))))))
-
-(ert-deftest test-kill-buffer-auto-save-delete ()
+(defun test-kill-buffer-auto-save (auto-save-answer body-func)
+ "Test helper for `kill-buffer-delete-auto-save' tests.
+
+Call BODY-FUNC with the current buffer set to a buffer visiting a
+temporary file. Around the call, mock the \"Buffer modified;
+kill anyway?\" and \"Delete auto-save file?\" prompts, answering
+\"yes\" for the former and AUTO-SAVE-ANSWER for the latter. The
+expectation should be the characters `?y' or `?n', or `nil' if no
+prompt is expected. The test fails if the \"Delete auto-save
+file?\" prompt does not either prompt is not issued as expected.
+Finally, kill the buffer and its temporary file."
(ert-with-temp-file file
- (let (auto-save)
- (should (file-exists-p file))
- (setq kill-buffer-delete-auto-save-files t)
- ;; Always answer yes.
- (cl-letf (((symbol-function #'yes-or-no-p) (lambda (_) t)))
- (unwind-protect
- (progn
- (find-file file)
- (auto-save-mode t)
- (insert "foo\n")
- (should buffer-auto-save-file-name)
- (setq auto-save buffer-auto-save-file-name)
- (do-auto-save)
- (should (file-exists-p auto-save))
- ;; This should delete the auto-save file.
- (kill-buffer (current-buffer))
- (should-not (file-exists-p auto-save)))
- (ignore-errors (delete-file file))
- (when auto-save
- (ignore-errors (delete-file auto-save)))))
- ;; Answer no to deletion.
- (cl-letf (((symbol-function #'yes-or-no-p)
- (lambda (prompt)
- (not (string-search "Delete auto-save file" prompt)))))
- (unwind-protect
- (progn
- (find-file file)
- (auto-save-mode t)
- (insert "foo\n")
- (should buffer-auto-save-file-name)
- (setq auto-save buffer-auto-save-file-name)
- (do-auto-save)
- (should (file-exists-p auto-save))
- ;; This should not delete the auto-save file.
- (kill-buffer (current-buffer))
- (should (file-exists-p auto-save)))
- (when auto-save
- (ignore-errors (delete-file auto-save))))))))
+ (should (file-exists-p file))
+ (save-excursion
+ (find-file file)
+ (should (equal file (buffer-file-name)))
+ (let ((buffer (current-buffer))
+ (auto-save-prompt-happened nil))
+ (cl-letf (((symbol-function #'read-multiple-choice)
+ (lambda (prompt choices &rest _)
+ (should (string-search "modified; kill anyway?" prompt))
+ (let ((answer (assq ?y choices)))
+ (should answer)
+ answer)))
+ ((symbol-function #'yes-or-no-p)
+ (lambda (prompt)
+ (should (string-search "Delete auto-save file?" prompt))
+ (setq auto-save-prompt-happened t)
+ (pcase-exhaustive auto-save-answer
+ (?y t)
+ (?n nil)))))
+ (funcall body-func)
+ (should (equal (null auto-save-prompt-happened)
+ (null auto-save-answer))))
+ (when (buffer-live-p buffer)
+ (with-current-buffer buffer
+ (set-buffer-modified-p nil)
+ (kill-buffer)))))))
+
+(ert-deftest test-kill-buffer-auto-save-default ()
+ (let ((kill-buffer-delete-auto-save-files nil))
+ (test-kill-buffer-auto-save
+ nil
+ (lambda ()
+ (let (auto-save)
+ (auto-save-mode t)
+ (insert "foo\n")
+ (should buffer-auto-save-file-name)
+ (setq auto-save buffer-auto-save-file-name)
+ (do-auto-save)
+ (should (file-exists-p auto-save))
+ (kill-buffer (current-buffer))
+ (should (file-exists-p auto-save)))))))
+
+(ert-deftest test-kill-buffer-auto-save-delete-yes ()
+ (let ((kill-buffer-delete-auto-save-files t))
+ (test-kill-buffer-auto-save
+ ?y
+ (lambda ()
+ (let (auto-save)
+ (auto-save-mode t)
+ (insert "foo\n")
+ (should buffer-auto-save-file-name)
+ (setq auto-save buffer-auto-save-file-name)
+ (do-auto-save)
+ (should (file-exists-p auto-save))
+ ;; This should delete the auto-save file.
+ (kill-buffer (current-buffer))
+ (should-not (file-exists-p auto-save)))))))
+
+(ert-deftest test-kill-buffer-auto-save-delete-no ()
+ (let ((kill-buffer-delete-auto-save-files t))
+ (test-kill-buffer-auto-save
+ ?n
+ (lambda ()
+ (let (auto-save)
+ (auto-save-mode t)
+ (insert "foo\n")
+ (should buffer-auto-save-file-name)
+ (setq auto-save buffer-auto-save-file-name)
+ (do-auto-save)
+ (should (file-exists-p auto-save))
+ ;; This should not delete the auto-save file.
+ (kill-buffer (current-buffer))
+ (should (file-exists-p auto-save))
+ (delete-file auto-save))))))
(ert-deftest test-buffer-modifications ()
(ert-with-temp-file file
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#59028: [PATCH] Rewrite the `kill-buffer-delete-auto-save' tests
2022-11-04 22:53 bug#59028: [PATCH] Rewrite the `kill-buffer-delete-auto-save' tests Matt Armstrong
@ 2022-11-10 9:56 ` Eli Zaretskii
2022-11-15 18:52 ` Matt Armstrong
0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2022-11-10 9:56 UTC (permalink / raw)
To: Matt Armstrong; +Cc: 59028
> From: Matt Armstrong <matt@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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#59028: [PATCH] Rewrite the `kill-buffer-delete-auto-save' tests
2022-11-10 9:56 ` Eli Zaretskii
@ 2022-11-15 18:52 ` Matt Armstrong
2022-11-15 20:00 ` Stefan Kangas
2022-11-16 14:41 ` Eli Zaretskii
0 siblings, 2 replies; 6+ messages in thread
From: Matt Armstrong @ 2022-11-15 18:52 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 59028
[-- Attachment #1: Type: text/plain, Size: 2189 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Matt Armstrong <matt@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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Reduce-buffer-tests-noisiness-when-run-in-batch-mode.patch --]
[-- Type: text/x-diff, Size: 4857 bytes --]
From 717328dac712f3a53ca950bb1e202b4e0936ceae Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt@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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#59028: [PATCH] Rewrite the `kill-buffer-delete-auto-save' tests
2022-11-15 18:52 ` Matt Armstrong
@ 2022-11-15 20:00 ` Stefan Kangas
2022-11-16 18:54 ` Matt Armstrong
2022-11-16 14:41 ` Eli Zaretskii
1 sibling, 1 reply; 6+ messages in thread
From: Stefan Kangas @ 2022-11-15 20:00 UTC (permalink / raw)
To: Matt Armstrong, Eli Zaretskii; +Cc: 59028
Matt Armstrong <matt@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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#59028: [PATCH] Rewrite the `kill-buffer-delete-auto-save' tests
2022-11-15 18:52 ` Matt Armstrong
2022-11-15 20:00 ` Stefan Kangas
@ 2022-11-16 14:41 ` Eli Zaretskii
1 sibling, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2022-11-16 14:41 UTC (permalink / raw)
To: Matt Armstrong; +Cc: 59028
> From: Matt Armstrong <matt@rfc20.org>
> Cc: 59028@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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#59028: [PATCH] Rewrite the `kill-buffer-delete-auto-save' tests
2022-11-15 20:00 ` Stefan Kangas
@ 2022-11-16 18:54 ` Matt Armstrong
0 siblings, 0 replies; 6+ messages in thread
From: Matt Armstrong @ 2022-11-16 18:54 UTC (permalink / raw)
To: Stefan Kangas, Eli Zaretskii; +Cc: 59028-done
Stefan Kangas <stefankangas@gmail.com> writes:
> Matt Armstrong <matt@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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-11-16 18:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-04 22:53 bug#59028: [PATCH] Rewrite the `kill-buffer-delete-auto-save' tests Matt Armstrong
2022-11-10 9:56 ` Eli Zaretskii
2022-11-15 18:52 ` Matt Armstrong
2022-11-15 20:00 ` Stefan Kangas
2022-11-16 18:54 ` Matt Armstrong
2022-11-16 14:41 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).