From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Gemini Lasswell Newsgroups: gmane.emacs.bugs Subject: bug#25158: [PATCH] A better way for test code to access messages Date: Sat, 10 Dec 2016 09:29:05 -0800 Message-ID: NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1481391020 5706 195.159.176.226 (10 Dec 2016 17:30:20 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 10 Dec 2016 17:30:20 +0000 (UTC) Cc: Michael Albinus To: 25158@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sat Dec 10 18:30:16 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cFlTR-0000g0-A8 for geb-bug-gnu-emacs@m.gmane.org; Sat, 10 Dec 2016 18:30:13 +0100 Original-Received: from localhost ([::1]:52571 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cFlTV-00040r-8v for geb-bug-gnu-emacs@m.gmane.org; Sat, 10 Dec 2016 12:30:17 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:50528) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cFlTK-0003xT-Iv for bug-gnu-emacs@gnu.org; Sat, 10 Dec 2016 12:30:08 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cFlTH-0004Ke-Cl for bug-gnu-emacs@gnu.org; Sat, 10 Dec 2016 12:30:06 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:50003) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cFlTH-0004KY-95; Sat, 10 Dec 2016 12:30:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1cFlTG-0004E9-LH; Sat, 10 Dec 2016 12:30:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Gemini Lasswell Original-Sender: "Debbugs-submit" Resent-CC: eliz@gnu.org, michael.albinus@gmx.de, bug-gnu-emacs@gnu.org Resent-Date: Sat, 10 Dec 2016 17:30:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 25158 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch X-Debbugs-Original-To: bug-gnu-emacs@gnu.org X-Debbugs-Original-Xcc: Eli Zaretskii , Michael Albinus Original-Received: via spool by submit@debbugs.gnu.org id=B.148139097916187 (code B ref -1); Sat, 10 Dec 2016 17:30:02 +0000 Original-Received: (at submit) by debbugs.gnu.org; 10 Dec 2016 17:29:39 +0000 Original-Received: from localhost ([127.0.0.1]:37168 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cFlSs-0004D0-GC for submit@debbugs.gnu.org; Sat, 10 Dec 2016 12:29:39 -0500 Original-Received: from eggs.gnu.org ([208.118.235.92]:59407) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cFlSq-0004Cl-1d for submit@debbugs.gnu.org; Sat, 10 Dec 2016 12:29:36 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cFlSi-00042l-Hq for submit@debbugs.gnu.org; Sat, 10 Dec 2016 12:29:30 -0500 Original-Received: from lists.gnu.org ([2001:4830:134:3::11]:50229) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cFlSi-00042h-EO for submit@debbugs.gnu.org; Sat, 10 Dec 2016 12:29:28 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:50372) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cFlSf-0003WN-9T for bug-gnu-emacs@gnu.org; Sat, 10 Dec 2016 12:29:28 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cFlSc-0003zB-33 for bug-gnu-emacs@gnu.org; Sat, 10 Dec 2016 12:29:25 -0500 Original-Received: from aibo.runbox.com ([91.220.196.211]:52180) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cFlSb-0003xY-Hn for bug-gnu-emacs@gnu.org; Sat, 10 Dec 2016 12:29:22 -0500 Original-Received: from [10.9.9.211] (helo=mailfront11.runbox.com) by bars.runbox.com with esmtp (Exim 4.71) (envelope-from ) id 1cFlSY-0001bb-Sb for bug-gnu-emacs@gnu.org; Sat, 10 Dec 2016 18:29:18 +0100 Original-Received: from c-24-22-244-161.hsd1.wa.comcast.net ([24.22.244.161] helo=rainbow.local) by mailfront11.runbox.com with esmtpsa (uid:179284 ) (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) id 1cFlSN-0004vR-Pa for bug-gnu-emacs@gnu.org; Sat, 10 Dec 2016 18:29:08 +0100 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x [fuzzy] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:126815 Archived-At: --=-=-= Content-Type: text/plain Here is a quote from Bug#24939's discussion, regarding the technique used by autorevert-tests.el, filenotify-tests.el and the proposed kmacro-tests.el to collect messages issued during part of a test by temporarily narrowing *Messages*: Eli Zaretskii writes: > I don't like this implementation. First, playing restriction games > with *Messages* is inherently unsafe, because that buffer is treated > specially by the code which puts messages there. Second, this assumes > *Messages* will have each message verbatim, which is false, because > repeated messages aren't inserted. And finally, some code can disable > message logging or use some mechanism for displaying echo-area > messages that bypasses *Messages*, in which case this macro will not > catch the message. > > So I'd suggest instead to override or advice 'message', so you could > get your hands on the messages more reliably. It is possible we > should have a more thorough infrastructure for collecting echo-area > messages, which probably means parts of it should be implemented in C, > but that's a separate project. > The attached patch adds a new macro called ert-with-message-capture to ert-x.el which temporarily adds advice to 'message' to collect messages. I've also modified autorevert-tests.el and filenotify-tests.el to use the new macro. If or when the more thorough infrastructure is implemented, that could replace the use of advice in this macro but the tests which use it should not have to change. Michael, in modifying autorevert-tests.el, at the start of auto-revert-test02-auto-revert-deleted-file, *Messages* was narrowed, and then narrowed again before the call to auto-revert--wait-for-revert, so it looked safe to delete the first narrowing instead of replacing it with ert-with-message-capture. Let me know if I've missed something there. --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=0001-Add-ert-with-message-capture.patch >From da4bfb3adbb97841a3bf8bd80dfee7e6a474be32 Mon Sep 17 00:00:00 2001 From: gazally Date: Wed, 30 Nov 2016 12:12:53 -0800 Subject: [PATCH] Add ert-with-message-capture * lisp/emacs-lisp/ert-x.el (ert-with-message-capture): New function. * test/lisp/autorevert-tests.el (auto-revert--wait-for-revert) (auto-revert-test00-auto-revert-mode) (auto-revert-test01-auto-revert-several-files) (auto-revert-test02-auto-revert-deleted-file) (auto-revert-test03-auto-revert-tail-mode) (auto-revert-test04-auto-revert-mode-dired): Use it. * test/lisp/filenotify-tests.el (file-notify-test03-autorevert): Use it. --- lisp/emacs-lisp/ert-x.el | 24 ++++++ test/lisp/autorevert-tests.el | 166 +++++++++++++++++++----------------------- test/lisp/filenotify-tests.el | 56 +++++++------- 3 files changed, 125 insertions(+), 121 deletions(-) diff --git a/lisp/emacs-lisp/ert-x.el b/lisp/emacs-lisp/ert-x.el index 2a2418f..42899b3 100644 --- a/lisp/emacs-lisp/ert-x.el +++ b/lisp/emacs-lisp/ert-x.el @@ -285,6 +285,30 @@ ert-buffer-string-reindented (kill-buffer clone))))))) +(defmacro ert-with-message-capture (var &rest body) + "Execute BODY while collecting anything written with `message' in VAR. + +Capture all messages produced by `message' when it is called from +Lisp, and concatenate them separated by newlines into one string. + +This is useful for separating the issuance of messages by the +code under test from the behavior of the *Messages* buffer." + (declare (debug (symbolp body)) + (indent 1)) + (let ((g-advice (cl-gensym))) + `(let* ((,var "") + (,g-advice (lambda (func &rest args) + (if (or (null args) (equal (car args) "")) + (apply func args) + (let ((msg (apply #'format-message args))) + (setq ,var (concat ,var msg "\n")) + (funcall func "%s" msg)))))) + (advice-add 'message :around ,g-advice) + (unwind-protect + (progn ,@body) + (advice-remove 'message ,g-advice))))) + + (provide 'ert-x) ;;; ert-x.el ends here diff --git a/test/lisp/autorevert-tests.el b/test/lisp/autorevert-tests.el index 2f951c0..4dbeec5 100644 --- a/test/lisp/autorevert-tests.el +++ b/test/lisp/autorevert-tests.el @@ -24,24 +24,29 @@ ;;; Code: (require 'ert) +(require 'ert-x) (require 'autorevert) (setq auto-revert-notify-exclude-dir-regexp "nothing-to-be-excluded" auto-revert-stop-on-user-input nil) (defconst auto-revert--timeout 10 - "Time to wait until a message appears in the *Messages* buffer.") + "Time to wait for a message.") + +(defvar auto-revert--messages nil + "Used to collect messages issued during a section of a test.") (defun auto-revert--wait-for-revert (buffer) - "Wait until the *Messages* buffer reports reversion of BUFFER." + "Wait until a message reports reversion of BUFFER. +This expects `auto-revert--messages' to be bound by +`ert-with-message-capture' before calling." (with-timeout (auto-revert--timeout nil) - (with-current-buffer "*Messages*" - (while - (null (string-match - (format-message "Reverting buffer `%s'." (buffer-name buffer)) - (buffer-string))) - (if (with-current-buffer buffer auto-revert-use-notify) - (read-event nil nil 0.1) - (sleep-for 0.1)))))) + (while + (null (string-match + (format-message "Reverting buffer `%s'." (buffer-name buffer)) + auto-revert--messages)) + (if (with-current-buffer buffer auto-revert-use-notify) + (read-event nil nil 0.1) + (sleep-for 0.1))))) (ert-deftest auto-revert-test00-auto-revert-mode () "Check autorevert for a file." @@ -51,41 +56,38 @@ auto-revert--wait-for-revert buf) (unwind-protect (progn - (with-current-buffer (get-buffer-create "*Messages*") - (narrow-to-region (point-max) (point-max))) - (write-region "any text" nil tmpfile nil 'no-message) + (write-region "any text" nil tmpfile nil 'no-message) (setq buf (find-file-noselect tmpfile)) - (with-current-buffer buf - (should (string-equal (buffer-string) "any text")) - ;; `buffer-stale--default-function' checks for - ;; `verify-visited-file-modtime'. We must ensure that it - ;; returns nil. - (sleep-for 1) - (auto-revert-mode 1) - (should auto-revert-mode) + (with-current-buffer buf + (ert-with-message-capture auto-revert--messages + (should (string-equal (buffer-string) "any text")) + ;; `buffer-stale--default-function' checks for + ;; `verify-visited-file-modtime'. We must ensure that it + ;; returns nil. + (sleep-for 1) + (auto-revert-mode 1) + (should auto-revert-mode) - ;; Modify file. We wait for a second, in order to have - ;; another timestamp. - (sleep-for 1) - (write-region "another text" nil tmpfile nil 'no-message) + ;; Modify file. We wait for a second, in order to have + ;; another timestamp. + (sleep-for 1) + (write-region "another text" nil tmpfile nil 'no-message) - ;; Check, that the buffer has been reverted. - (auto-revert--wait-for-revert buf) + ;; Check, that the buffer has been reverted. + (auto-revert--wait-for-revert buf)) (should (string-match "another text" (buffer-string))) ;; When the buffer is modified, it shall not be reverted. - (with-current-buffer (get-buffer-create "*Messages*") - (narrow-to-region (point-max) (point-max))) - (set-buffer-modified-p t) - (sleep-for 1) - (write-region "any text" nil tmpfile nil 'no-message) + (ert-with-message-capture auto-revert--messages + (set-buffer-modified-p t) + (sleep-for 1) + (write-region "any text" nil tmpfile nil 'no-message) - ;; Check, that the buffer hasn't been reverted. - (auto-revert--wait-for-revert buf) + ;; Check, that the buffer hasn't been reverted. + (auto-revert--wait-for-revert buf)) (should-not (string-match "any text" (buffer-string))))) ;; Exit. - (with-current-buffer "*Messages*" (widen)) (ignore-errors (with-current-buffer buf (set-buffer-modified-p nil)) (kill-buffer buf)) @@ -106,13 +108,11 @@ auto-revert--wait-for-revert (make-temp-file (expand-file-name "auto-revert-test" tmpdir1))) buf1 buf2) (unwind-protect - (progn - (with-current-buffer (get-buffer-create "*Messages*") - (narrow-to-region (point-max) (point-max))) - (write-region "any text" nil tmpfile1 nil 'no-message) - (setq buf1 (find-file-noselect tmpfile1)) - (write-region "any text" nil tmpfile2 nil 'no-message) - (setq buf2 (find-file-noselect tmpfile2)) + (ert-with-message-capture auto-revert--messages + (write-region "any text" nil tmpfile1 nil 'no-message) + (setq buf1 (find-file-noselect tmpfile1)) + (write-region "any text" nil tmpfile2 nil 'no-message) + (setq buf2 (find-file-noselect tmpfile2)) (dolist (buf (list buf1 buf2)) (with-current-buffer buf @@ -148,7 +148,6 @@ auto-revert--wait-for-revert (should (string-match "another text" (buffer-string)))))) ;; Exit. - (with-current-buffer "*Messages*" (widen)) (ignore-errors (dolist (buf (list buf1 buf2)) (with-current-buffer buf (set-buffer-modified-p nil)) @@ -165,8 +164,6 @@ auto-revert--wait-for-revert buf) (unwind-protect (progn - (with-current-buffer (get-buffer-create "*Messages*") - (narrow-to-region (point-max) (point-max))) (write-region "any text" nil tmpfile nil 'no-message) (setq buf (find-file-noselect tmpfile)) (with-current-buffer buf @@ -184,42 +181,36 @@ auto-revert--wait-for-revert 'before-revert-hook (lambda () (delete-file buffer-file-name)) nil t) - (with-current-buffer (get-buffer-create "*Messages*") - (narrow-to-region (point-max) (point-max))) - (sleep-for 1) - (write-region "another text" nil tmpfile nil 'no-message) - ;; Check, that the buffer hasn't been reverted. File - ;; notification should be disabled, falling back to - ;; polling. - (auto-revert--wait-for-revert buf) + (ert-with-message-capture auto-revert--messages + (sleep-for 1) + (write-region "another text" nil tmpfile nil 'no-message) + (auto-revert--wait-for-revert buf)) + ;; Check, that the buffer hasn't been reverted. File + ;; notification should be disabled, falling back to + ;; polling. (should (string-match "any text" (buffer-string))) (should-not auto-revert-use-notify) ;; Once the file has been recreated, the buffer shall be ;; reverted. (kill-local-variable 'before-revert-hook) - (with-current-buffer (get-buffer-create "*Messages*") - (narrow-to-region (point-max) (point-max))) - (sleep-for 1) - (write-region "another text" nil tmpfile nil 'no-message) - - ;; Check, that the buffer has been reverted. - (auto-revert--wait-for-revert buf) + (ert-with-message-capture auto-revert--messages + (sleep-for 1) + (write-region "another text" nil tmpfile nil 'no-message) + (auto-revert--wait-for-revert buf)) + ;; Check, that the buffer has been reverted. (should (string-match "another text" (buffer-string))) ;; An empty file shall still be reverted. - (with-current-buffer (get-buffer-create "*Messages*") - (narrow-to-region (point-max) (point-max))) - (sleep-for 1) - (write-region "" nil tmpfile nil 'no-message) - - ;; Check, that the buffer has been reverted. - (auto-revert--wait-for-revert buf) + (ert-with-message-capture auto-revert--messages + (sleep-for 1) + (write-region "" nil tmpfile nil 'no-message) + (auto-revert--wait-for-revert buf)) + ;; Check, that the buffer has been reverted. (should (string-equal "" (buffer-string))))) ;; Exit. - (with-current-buffer "*Messages*" (widen)) (ignore-errors (with-current-buffer buf (set-buffer-modified-p nil)) (kill-buffer buf)) @@ -232,9 +223,7 @@ auto-revert--wait-for-revert (let ((tmpfile (make-temp-file "auto-revert-test")) buf) (unwind-protect - (progn - (with-current-buffer (get-buffer-create "*Messages*") - (narrow-to-region (point-max) (point-max))) + (ert-with-message-capture auto-revert--messages (write-region "any text" nil tmpfile nil 'no-message) (setq buf (find-file-noselect tmpfile)) (with-current-buffer buf @@ -259,7 +248,6 @@ auto-revert--wait-for-revert (string-match "modified text\nanother text" (buffer-string))))) ;; Exit. - (with-current-buffer "*Messages*" (widen)) (ignore-errors (kill-buffer buf)) (ignore-errors (delete-file tmpfile))))) @@ -283,33 +271,29 @@ auto-revert--wait-for-revert (should (string-match name (substring-no-properties (buffer-string)))) - ;; Delete file. We wait for a second, in order to have - ;; another timestamp. - (with-current-buffer (get-buffer-create "*Messages*") - (narrow-to-region (point-max) (point-max))) - (sleep-for 1) - (delete-file tmpfile) - - ;; Check, that the buffer has been reverted. - (auto-revert--wait-for-revert buf) + (ert-with-message-capture auto-revert--messages + ;; Delete file. We wait for a second, in order to have + ;; another timestamp. + (sleep-for 1) + (delete-file tmpfile) + (auto-revert--wait-for-revert buf)) + ;; Check, that the buffer has been reverted. (should-not (string-match name (substring-no-properties (buffer-string)))) - ;; Make dired buffer modified. Check, that the buffer has - ;; been still reverted. - (with-current-buffer (get-buffer-create "*Messages*") - (narrow-to-region (point-max) (point-max))) - (set-buffer-modified-p t) - (sleep-for 1) - (write-region "any text" nil tmpfile nil 'no-message) + (ert-with-message-capture auto-revert--messages + ;; Make dired buffer modified. Check, that the buffer has + ;; been still reverted. + (set-buffer-modified-p t) + (sleep-for 1) + (write-region "any text" nil tmpfile nil 'no-message) - ;; Check, that the buffer has been reverted. - (auto-revert--wait-for-revert buf) + (auto-revert--wait-for-revert buf)) + ;; Check, that the buffer has been reverted. (should (string-match name (substring-no-properties (buffer-string)))))) ;; Exit. - (with-current-buffer "*Messages*" (widen)) (ignore-errors (with-current-buffer buf (set-buffer-modified-p nil)) (kill-buffer buf)) diff --git a/test/lisp/filenotify-tests.el b/test/lisp/filenotify-tests.el index 0e6e58e..07a88ed 100644 --- a/test/lisp/filenotify-tests.el +++ b/test/lisp/filenotify-tests.el @@ -36,6 +36,7 @@ ;;; Code: (require 'ert) +(require 'ert-x) (require 'filenotify) (require 'tramp) @@ -640,21 +641,19 @@ file-notify--test-with-events (should auto-revert-notify-watch-descriptor) ;; Modify file. We wait for a second, in order to have - ;; another timestamp. - (with-current-buffer (get-buffer-create "*Messages*") - (narrow-to-region (point-max) (point-max))) - (sleep-for 1) - (write-region - "another text" nil file-notify--test-tmpfile nil 'no-message) - - ;; Check, that the buffer has been reverted. - (with-current-buffer (get-buffer-create "*Messages*") - (file-notify--wait-for-events - timeout - (string-match + ;; another timestamp. + (ert-with-message-capture captured-messages + (sleep-for 1) + (write-region + "another text" nil file-notify--test-tmpfile nil 'no-message) + + ;; Check, that the buffer has been reverted. + (file-notify--wait-for-events + timeout + (string-match (format-message "Reverting buffer `%s'." (buffer-name buf)) - (buffer-string)))) - (should (string-match "another text" (buffer-string))) + captured-messages)) + (should (string-match "another text" (buffer-string)))) ;; Stop file notification. Autorevert shall still work via polling. (file-notify-rm-watch auto-revert-notify-watch-descriptor) @@ -665,27 +664,24 @@ file-notify--test-with-events ;; Modify file. We wait for two seconds, in order to ;; have another timestamp. One second seems to be too - ;; short. - (with-current-buffer (get-buffer-create "*Messages*") - (narrow-to-region (point-max) (point-max))) - (sleep-for 2) - (write-region - "foo bla" nil file-notify--test-tmpfile nil 'no-message) - - ;; Check, that the buffer has been reverted. - (with-current-buffer (get-buffer-create "*Messages*") - (file-notify--wait-for-events - timeout - (string-match - (format-message "Reverting buffer `%s'." (buffer-name buf)) - (buffer-string)))) - (should (string-match "foo bla" (buffer-string)))) + ;; short. + (ert-with-message-capture captured-messages + (sleep-for 2) + (write-region + "foo bla" nil file-notify--test-tmpfile nil 'no-message) + + ;; Check, that the buffer has been reverted. + (file-notify--wait-for-events + timeout + (string-match + (format-message "Reverting buffer `%s'." (buffer-name buf)) + captured-messages)) + (should (string-match "foo bla" (buffer-string))))) ;; The environment shall be cleaned up. (file-notify--test-cleanup-p)) ;; Cleanup. - (with-current-buffer "*Messages*" (widen)) (ignore-errors (kill-buffer buf)) (file-notify--test-cleanup)))) -- 2.10.1 --=-=-=--