unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#24939: [PATCH] Add tests for lisp/kmacro.el
@ 2016-11-13 21:23 Gemini Lasswell
  2016-11-14 15:49 ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Gemini Lasswell @ 2016-11-13 21:23 UTC (permalink / raw)
  To: 24939

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

Hello,
There weren't any tests for kmacro.el, so I have written some.

Two things that may yet need to be done:

1. These tests make extensive use of two macros, called
kmacro-tests-should-call and kmacro-tests-should-not-call. They are
context-creating macros which add advice to named functions for the
duration of a test. I think that these two macros would be a useful
addition to ERT, and I'll submit that idea as a separate patch.

2. I found several minor bugs in the process of writing these, leading
to tests marked as :expected-result :failed. One is a way to create an
empty keyboard macro using the mouse and the rest are ways to get
kmacro-step-edit-macro to behave oddly. I haven't sent them to
bug-gnu-emacs yet. When I do so would it be better to send them
individually or put all the step-edit ones in one report?

My copyright assignment paperwork was finished as of Nov 2.


[-- Attachment #2: 0001-Add-tests-for-lisp-kmacro.el.patch --]
[-- Type: text/plain, Size: 58822 bytes --]

From 6dc518ac6f356a55593184ed6e1bafa5cd764623 Mon Sep 17 00:00:00 2001
From: gazally <gazally@users.noreply.github.com>
Date: Sun, 13 Nov 2016 08:02:38 -0800
Subject: [PATCH] Add tests for lisp/kmacro.el

* test/lisp/kmacro-tests.el: New file.
---
 test/lisp/kmacro-tests.el | 1251 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 1251 insertions(+)
 create mode 100644 test/lisp/kmacro-tests.el

diff --git a/test/lisp/kmacro-tests.el b/test/lisp/kmacro-tests.el
new file mode 100644
index 0000000..fae9cbc
--- /dev/null
+++ b/test/lisp/kmacro-tests.el
@@ -0,0 +1,1251 @@
+;;; kmacro-tests.el --- Tests for kmacro.el       -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2016 Free Software Foundation, Inc.
+
+;; Author: Gemini Lasswell
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'kmacro)
+(require 'edmacro)
+(require 'ert)
+(require 'ert-x)
+
+;;; Test fixtures:
+
+(defvar kmacro-tests-keymap
+  (let ((map (make-sparse-keymap)))
+
+    (dotimes (i 26)
+      (define-key map (string (+ ?a i)) 'self-insert-command))
+    (dotimes (i 10)
+      (define-key map (string (+ ?0 i)) 'self-insert-command))
+
+    ;; define a few key sequences of different lengths
+    (dolist (item '(("\C-a" . beginning-of-line)
+                    ("\C-e" . end-of-line)
+                    ("\C-r" . isearch-backward)
+                    ("\C-u" . universal-argument)
+                    ("\M-x" . execute-extended-command)
+                    ("\C-cd" . downcase-word)
+                    ("\C-cxu" . upcase-word)
+                    ("\C-cxq" . quoted-insert)))
+      (define-key map (car item) (cdr item)))
+
+    map)
+  "Keymap to use for testing keyboard macros.
+Used to obtain consistent results even if tests are run in an
+environment with rebound keys.")
+
+(defmacro kmacro-tests-with-kmacro-clean-slate (directions &rest body)
+  "Create a clean environment for a kmacro test to run in.
+Save the kmacro global variables and set them to reasonable
+default values.  Temporarily bind the functions defined by
+macros.c and used by kmacro.el to mocked versions as requested by
+DIRECTIONS (see the docstring of `kmacro-tests-make-macros-c-rebindings'
+for details).  Then execute BODY, and restore the variables and
+functions."
+  (declare (debug ((&rest (gate gv-place &optional form)) body)))
+  (let ((var-bindings
+         '(;; Macro customizations
+           (kmacro-execute-before-append t)
+           (kmacro-ring-max 8)
+           (kmacro-repeat-no-prefix t)
+           (kmacro-call-repeat-key nil)
+           (kmacro-call-repeat-with-arg nil)
+
+           ;; Macro recording state
+           (kbd-macro-termination-hook nil)
+           (defining-kbd-macro nil)
+           (executing-kbd-macro nil)
+           (executing-kbd-macro-index 0)
+           (last-kbd-macro nil)
+
+           ;; Macro history
+           (kmacro-ring nil)
+
+           ;; Macro counter
+           (kmacro-counter 0)
+           (kmacro-default-counter-format "%d")
+           (kmacro-counter-format "%d")
+           (kmacro-counter-format-start "%d")
+           (kmacro-counter-value-start 0)
+           (kmacro-last-counter 0)
+           (kmacro-initial-counter-value nil)))
+
+        ;; Rebindings for macros.c functions
+        (func-bindings (kmacro-tests-make-macros-c-rebindings directions)))
+
+    `(cl-letf* ,(append var-bindings func-bindings) ,@body)))
+
+(defvar kmacro-tests-stubs
+  '((start-kbd-macro .     (lambda (_append _noexec)
+                             (setq defining-kbd-macro t)))
+    (end-kbd-macro .       (lambda (&optional _repeat _loopfunc)
+                             (setq defining-kbd-macro nil)))
+    (call-last-kbd-macro . (lambda (_count loopfunc)
+                             (and loopfunc (funcall loopfunc))))
+    (execute-kbd-macro .   (lambda (_mac _count loopfunc)
+                             (and loopfunc (funcall loopfunc)))))
+  "Stubs for functions from macros.c that can be used to test kmacro.el.")
+
+(defun kmacro-tests-make-macros-c-rebindings (directions)
+  "Create rebindings suitable for `cl-letf' for the functions in macros.c.
+DIRECTIONS is a list that may contain :use, :stub and the symbols
+for the four functions in macros.c which are used by
+kmacro.el.  If any of the four functions is not in the list it
+will be rebound to a form which will cause a test failure if it
+is called.  A function preceded by :stub will be bound to a mock
+with a minimal level of functionality, and one preceded by :use
+will be left unbound."
+  (let ((protected (mapcar #'car kmacro-tests-stubs))
+        stub-bindings safety-bindings func directive)
+    (while directions
+      (if (keywordp (car directions))
+          (setq directive (car directions))
+        (setq func (car directions))
+        (cond
+         ;; directions should only contain macros.c symbols or :use or :stub
+         ((or (not (keywordp directive))
+              (not (symbolp func))
+              (not (assoc func kmacro-tests-stubs)))
+          (message "rebindings syntax %s" directions))
+
+         ;; write a binding for a stubbed function
+         ((eq directive :stub)
+          (let ((binding
+                 `((symbol-function #',func)
+                   (lambda (&rest args)
+                     (apply ,(alist-get func kmacro-tests-stubs) args)))))
+            (setq stub-bindings (cons binding stub-bindings))))
+
+         ;; Remove function from list that will be protected from
+         ;; unwanted calls
+         ((eq directive :use) (setq protected (delq func protected)))
+
+         (t (should-not directive))))
+      (setq directions (cdr directions)))
+
+    ;; Write bindings that will cause a test failure if any functions
+    ;; not named by :stub or :use are called.
+    (setq safety-bindings
+          (mapcar (lambda (func)
+                    `((symbol-function #',func)
+                      (lambda (&rest args) (should-not ',func))))
+                  protected))
+    (append safety-bindings stub-bindings)))
+
+;;; Wrapper for ert-deftest to set up everthing a kmacro test needs
+
+(defmacro kmacro-tests-deftest (name _args docstring &rest keys-and-body)
+  "Set up clean context for a kmacro unit test.
+NAME is the name of the test, _ARGS should be nil, and DOCSTRING
+is required.  To avoid having to duplicate ert's keyword parsing
+here, its keywords and values (if any) must be inside a list
+after the docstring, preceding the body, here combined with the
+body in KEYS-AND-BODY.  Directives for rebinding functions from
+macros.c may be placed in another list after ert's keywords and
+before the body.  For example:
+
+    (kmacro-tests-deftest a-kmacro-test ()
+      \"docstring\"
+       (:tags '(:expensive-test) :expected-result :fail)
+       (:stub end-kbd-macro :use execute-kbd-macro)
+       (body-of-test))"
+
+  (declare (debug (name sexp stringp
+                        (&rest keywordp sexp)
+                        (&rest [keywordp sexp])
+                        body))
+           (doc-string 3)
+           (indent 2))
+
+  (let* ((keys (when (and (listp (car keys-and-body))
+                          (keywordp (caar keys-and-body))
+                          (not (eq (caar keys-and-body) :use))
+                          (not (eq (caar keys-and-body) :stub)))
+                 (car keys-and-body)))
+         (directions-and-body (if keys (cdr keys-and-body)
+                                keys-and-body))
+         (directions (when (and (listp (car directions-and-body))
+                                (keywordp (caar directions-and-body)))
+                       (car directions-and-body)))
+         (body (if directions (cdr directions-and-body)
+                 directions-and-body)))
+    `(ert-deftest ,name ()
+       ,docstring ,@keys
+       (kmacro-tests-with-kmacro-clean-slate ,directions
+                                             ,@body))))
+
+(defmacro kmacro-tests-with-current-prefix-arg (form)
+  "Set `current-prefix-arg' temporarily while executing FORM.
+Use the second element of form as the argument.  It will be
+evaluated twice."
+  (declare (debug (form)))
+  `(cl-letf ((current-prefix-arg ,(cadr form)))
+     ,form))
+
+;; Some more powerful expectations
+
+(defun kmacro-tests-arg-recorder ()
+  "Return a closure which will save its arguments for later examination.
+Helper for `kmacro-tests-should-call'.  Create a lambda which
+saves the arguments given to it inside a closure.  If you instead
+pass it :call-args, it will return the list of saved arguments."
+  (let (arglist)
+    (lambda (&rest args)
+      (if (eq (car args) :call-args)
+          arglist
+        (setq arglist (cons args arglist))))))
+
+(defmacro kmacro-tests-should-call (defs &rest body)
+  "Verify that the function(s) in DEFS are called by BODY.
+DEFS should be a list containing elements of the form:
+ (FUNC ARGCHECK WHERE FUNCTION)
+
+where FUNC is a symbol, ARGCHECK is either :once, :times followed
+by a value, or :check-args-with followed by a function value.
+WHERE and FUNCTION are optional and have the same meaning as in
+`advice-add'.
+
+Temporarily add advice to each FUNC in DEFS, including advice
+which records the arguments passed to FUNC (by reference not
+copy, relevant for destructive functions), execute BODY, and then
+depending on the ARGCHECK forms, verify that FUNC was either
+called once, a specified number of times, or that the function
+following :check-args-with returns a non-nil value when passed a
+list of all the arguments passed to FUNC (which will be in
+reverse order)."
+  (declare (debug ((&rest (gate fboundp
+                                [&or ":once"
+                                     [":times" form]
+                                     [":check-args-with" function-form]]
+                                &optional keywordp function-form))
+                   body))
+           (indent 1))
+  (if (null defs)
+      `(progn ,@body)
+    (let* ((g-argrec (cl-gensym))
+           (g-advice (cl-gensym))
+           (def (car defs))
+           (func (car def))
+           (check-type (nth 1 def))
+           (advice-given (> (length def) 3))
+           (advice-keyword (car (last def 2)))
+           (advice-function (car (last def))))
+      ;; Add two pieces of advice to the function: the one provided in
+      ;; the definitions list, and another to record the arguments.
+      `(let ((,g-argrec (kmacro-tests-arg-recorder))
+             (,g-advice ,advice-function)) ; only evaluate lambdas once
+         (advice-add ',func :before ,g-argrec '((depth . -100)))
+         (unwind-protect
+             (progn
+               ,(when advice-given
+                  `(advice-add ',func ,advice-keyword ,g-advice
+                               '((depth . -99))))
+               (unwind-protect
+                   (kmacro-tests-should-call ,(cdr defs) ,@body)
+                 ,(when advice-given
+                    `(advice-remove ',func ,g-advice))))
+           (advice-remove ',func ,g-argrec))
+         ;; Generate the after-execution argument list check.
+         ,(cond
+           ((eq check-type :once)
+            `(should (eql 1 (length (funcall ,g-argrec :call-args)))))
+           ((eq check-type :times)
+            `(should (eql ,(nth 2 def) (length (funcall ,g-argrec
+                                                        :call-args)))))
+           ((eq check-type :check-args-with)
+            `(should (funcall ,(nth 2 def) (funcall ,g-argrec
+                                                    :call-args)))))))))
+
+(defmacro kmacro-tests-should-not-call (func-or-funcs &rest body)
+  "Verify that a function or functions are not called during execution.
+FUNC-OR-FUNCS can either be a single function or a list of them.
+Temporarily override them them during the execution of BODY with advice
+containing `should-not' and a non-nil value.  Use the function symbol
+as the non-nil value to make tracking down what went wrong a bit
+faster."
+  (declare (debug (&or [(&rest fboundp) body]
+                       [fboundp body]))
+           (indent 1))
+  (let* ((funcs (if (consp func-or-funcs)
+                    func-or-funcs
+                  (list func-or-funcs)))
+         (defs (mapcar (lambda (f)
+                         `(,f :times 0 :override
+                              (lambda (&rest args) (should-not ',f))))
+                       funcs)))
+    `(kmacro-tests-should-call ,defs
+       ,@body)))
+
+(defmacro kmacro-tests-should-insert (value &rest body)
+  "Verify that VALUE is inserted by the execution of BODY.
+Execute BODY, then check that the string VALUE was inserted
+into the current buffer at point.  As a side effect captures the
+symbols p_ and bsize_."
+  (declare (debug (stringp body))
+           (indent 1))
+  `(let ((p_ (point))
+         (bsize_ (buffer-size)))
+     ,@body
+     (should (equal (buffer-substring p_ (point)) ,value))
+     (should (equal (- (buffer-size) bsize_) (string-width ,value)))))
+
+(defmacro kmacro-tests-should-match-message (value &rest body)
+  "Verify that a message matching VALUE is issued while executing BODY.
+Execute BODY, then check for a regexp match between
+VALUE and any text written to *Messages* during the execution."
+  (declare (debug (stringp body))
+           (indent 1))
+  `(with-current-buffer (get-buffer-create "*Messages*")
+     (save-restriction
+       (narrow-to-region (point-max) (point-max))
+       ,@body
+       (should (string-match-p ,value (buffer-string))))))
+
+;; Tests begin here
+
+(kmacro-tests-deftest kmacro-tests-test-insert-counter-01-nil ()
+  "Insert counter adds one with nil arg."
+  (with-temp-buffer
+    (kmacro-tests-should-insert "0"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter nil)))
+    (kmacro-tests-should-insert "1"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter nil)))))
+
+(kmacro-tests-deftest kmacro-tests-test-insert-counter-02-int ()
+  "Insert counter increments by value of list argument."
+  (with-temp-buffer
+    (kmacro-tests-should-insert "0"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter 2)))
+    (kmacro-tests-should-insert "2"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter 3)))
+    (kmacro-tests-should-insert "5"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter nil)))))
+
+(kmacro-tests-deftest kmacro-tests-test-insert-counter-03-list ()
+  "Insert counter doesn't increment when given universal argument."
+  (with-temp-buffer
+    (kmacro-tests-should-insert "0"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter '(16))))
+    (kmacro-tests-should-insert "0"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter '(4))))))
+
+(kmacro-tests-deftest kmacro-tests-test-insert-counter-04-neg ()
+  "Insert counter decrements with '- prefix argument"
+  (ert-with-test-buffer (:name "")
+    (kmacro-tests-should-insert "0"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter '-))
+      )
+    (kmacro-tests-should-insert "-1"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter nil)))))
+
+(kmacro-tests-deftest kmacro-tests-test-start-format-counter ()
+  "Insert counter uses start value and format."
+  (ert-with-test-buffer (:name "start-format-counter")
+    (kmacro-tests-with-current-prefix-arg (kmacro-set-counter 10))
+    (kmacro-tests-should-insert "10"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter nil)))
+    (kmacro-tests-should-insert "11"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter nil)))
+    (kmacro-set-format "c=%s")
+    (kmacro-tests-with-current-prefix-arg (kmacro-set-counter 50))
+    (kmacro-tests-should-insert "c=50"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter nil)))))
+
+(kmacro-tests-deftest kmacro-tests-test-start-macro-when-defining-macro ()
+  "Starting a macro while defining a macro does not start a second macro."
+  ;; Verify kmacro-start-macro calls start-kbd-macro
+  (:stub start-kbd-macro)
+  (kmacro-tests-should-call
+      ((start-kbd-macro :once :before (lambda (append noexec)
+                                        (should-not append)
+                                        (should-not noexec))))
+    (kmacro-tests-with-current-prefix-arg (kmacro-start-macro nil)))
+  ;; And leaves macro in being-recorded state
+  (should defining-kbd-macro)
+  ;; And that it doesn't call it again
+  (kmacro-tests-should-not-call start-kbd-macro
+    (kmacro-tests-with-current-prefix-arg (kmacro-start-macro nil))))
+
+
+(kmacro-tests-deftest kmacro-tests-set-macro-counter-while-defining ()
+  "Use of the prefix arg with kmacro-start sets kmacro-counter."
+  (:stub start-kbd-macro)
+  ;; Give kmacro-start-macro an argument
+  (kmacro-tests-should-call
+      ((start-kbd-macro :once :before (lambda (append noexec)
+                                        (should-not append)
+                                        (should-not noexec))))
+    (kmacro-tests-with-current-prefix-arg (kmacro-start-macro 5)))
+  (should defining-kbd-macro)
+  ;; And verify that the counter is set to that value
+  (ert-with-test-buffer (:name "counter-while-defining")
+    (kmacro-tests-should-insert "5"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter nil)))
+    ;; change it while defining macro
+    (kmacro-tests-with-current-prefix-arg (kmacro-set-counter 1))
+    (kmacro-tests-should-insert "1"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter nil)))
+    ;; using universal arg to to set counter should reset to starting value
+    (kmacro-tests-with-current-prefix-arg (kmacro-set-counter '(4)))
+    (kmacro-tests-should-insert "5"
+      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter nil)))))
+
+
+(kmacro-tests-deftest kmacro-tests-start-insert-counter-appends-to-macro ()
+  "Use of the universal arg appends to the previous macro."
+  (:stub start-kbd-macro end-kbd-macro)
+  ;; start recording a macro
+  (kmacro-tests-should-call ((start-kbd-macro :once))
+    (kmacro-tests-with-current-prefix-arg
+     (kmacro-start-macro-or-insert-counter nil)))
+  ;; make sure we are still recording
+  (should defining-kbd-macro)
+  ;; called again should insert the counter
+  (ert-with-test-buffer (:name "start-insert-counter-appends")
+    (kmacro-tests-should-not-call start-kbd-macro
+      (kmacro-tests-should-insert "0"
+        (kmacro-tests-with-current-prefix-arg
+         (kmacro-start-macro-or-insert-counter nil)))))
+  ;; still recording
+  (should defining-kbd-macro)
+  ;; end recording, check that repeat count is passed
+  (kmacro-tests-should-call ((end-kbd-macro
+                              :once :after
+                              (lambda (repeat loopfunc)
+                                (should (eql repeat 3))
+                                (should (functionp loopfunc))
+                                (setq last-kbd-macro
+                                      (string-to-vector "hello\C-a")))))
+    (kmacro-tests-with-current-prefix-arg (kmacro-end-or-call-macro 3)))
+  ;; now not recording
+  (should-not defining-kbd-macro)
+  ;; now append to the previous macro
+  (kmacro-tests-should-call
+      ((start-kbd-macro
+        :once :before (lambda (append noexec)
+                        (should append)
+                        (if kmacro-execute-before-append
+                            (should noexec)
+                          (should-not noexec)))))
+    (kmacro-tests-with-current-prefix-arg
+     (kmacro-start-macro-or-insert-counter '(16))))
+  (should (equal defining-kbd-macro 'append)))
+
+(kmacro-tests-deftest kmacro-tests-end-call-macro-prefix-args ()
+  "kmacro-end-call-macro changes behavior based on prefix arg."
+  (:stub call-last-kbd-macro)
+  ;; "record" two macros
+  (dotimes (i 2)
+    (kmacro-tests-mock-define-macro (make-vector (1+ i) i)))
+
+  ;; with no prefix arg, it should call the second macro
+  (kmacro-tests-should-call ((call-last-kbd-macro
+                              :once :before
+                              (lambda (count loopfunc)
+                                (should (equal last-kbd-macro [1 1])))))
+    (kmacro-tests-with-current-prefix-arg (kmacro-end-or-call-macro nil)))
+
+  ;; universal arg should call the first one
+  (kmacro-tests-should-call ((execute-kbd-macro
+                              :once :override
+                              (lambda (macro count loopfunc)
+                                (should (equal [0] macro))
+                                (should (eql 1 count))
+                                (should (functionp loopfunc)))))
+    (kmacro-tests-with-current-prefix-arg (kmacro-end-or-call-macro '(4)))))
+
+
+(kmacro-tests-deftest kmacro-tests-end-and-call-macro-tests ()
+  "Commands to end and call macro work under various conditions."
+  ;; this is currently failing because kmacro-end-call-mouse leaves an empty
+  ;; macro at the head of the ring.
+  (:expected-result :failed)
+  (:stub start-kbd-macro end-kbd-macro call-last-kbd-macro)
+  ;; test 2 very similar ways to do the same thing
+  (dolist (command '(kmacro-end-and-call-macro
+                     kmacro-end-call-mouse))
+
+    (kmacro-tests-should-call
+        ((start-kbd-macro :once)
+         (end-kbd-macro :once :after
+                        (lambda (repeat loopfunc)
+                          (setq last-kbd-macro "")))
+         (call-last-kbd-macro
+          :once :before
+          (lambda (count loopfunc)
+            (should (eql (if (eq command 'kmacro-end-and-call-macro) 2)
+                         count))))
+         ;; kmacro-end-call-mouse uses this
+         (mouse-set-point :check-args-with (lambda (_args) t)
+                          :override #'ignore))
+      (kmacro-tests-with-current-prefix-arg (kmacro-start-macro nil))
+      ;; try the command while defining a macro
+      (cl-letf ((current-prefix-arg 2))
+        (ert-simulate-command `(,command 2))))
+
+    ;; check that it stopped defining and that no macro was recorded
+    (should-not defining-kbd-macro)
+    (should-not last-kbd-macro)
+
+    ;; now try it while not defining a macro
+    (kmacro-tests-should-call ((call-last-kbd-macro
+                                :once :before
+                                (lambda (count loopfunc)
+                                  (should (eql nil count)))))
+      (cl-letf ((current-prefix-arg nil))
+        (ert-simulate-command `(,command nil))))))
+
+(kmacro-tests-deftest kmacro-tests-call-macro-hint-and-repeat ()
+  "`kmacro-call-macro' gives hint in Messages and sets up repeat keymap.
+\(Bug#11817)"
+  (:stub start-kbd-macro end-kbd-macro call-last-kbd-macro)
+  (kmacro-tests-mock-define-macro [5])
+  (cl-letf ((kmacro-call-repeat-key t)
+            (kmacro-call-repeat-with-arg t)
+            (last-input-event ?e))
+    (let (exitfunc)
+
+      (message "")  ; clear the echo area
+      (kmacro-tests-should-match-message "Type e to repeat macro"
+        (kmacro-tests-should-call
+            ((this-single-command-keys :once
+                                       :override (lambda () [?\C-x ?e]))
+             (call-last-kbd-macro :times 2
+                                  :before (lambda (count loopfunc)
+                                            (should (eql 3 count))))
+             (set-transient-map :times 2
+                                :around (lambda (func &rest args)
+                                          (push (apply func args)
+                                                exitfunc)
+                                          (car exitfunc))))
+
+          (kmacro-call-macro 3)
+          ;; check that it set up for repeat, and run the repeat
+          (funcall (lookup-key overriding-terminal-local-map "e"))))
+      ;; get rid of the transient maps made by `kmacro-call-macro'
+      (while exitfunc
+        (funcall (pop exitfunc))))))
+
+(kmacro-tests-deftest
+    kmacro-tests-run-macro-command-recorded-in-macro ()
+  "No infinite loop if `kmacro-end-and-call-macro' is recorded in the macro.
+\(Bug#15126)"
+  (ert-skip "Skipping due to Bug#24921 (an ERT bug)")
+  (:expected-result :failed)
+  (:stub start-kbd-macro end-kbd-macro :use call-last-kbd-macro)
+  (kmacro-tests-mock-define-macro (vconcat "foo" [return]
+                                           (where-is-internal
+                                            'execute-extended-command nil t)
+                                           "kmacro-end-and-call-macro"))
+  (ert-with-test-buffer (:name "Bug#15126")
+    (switch-to-buffer (current-buffer))
+    (use-local-map kmacro-tests-keymap)
+    (ert-simulate-command '(kmacro-end-and-call-macro nil))))
+
+
+(kmacro-tests-deftest kmacro-tests-test-ring-2nd-commands ()
+  "2nd macro in ring is displayed and executed normally and on repeat."
+  (:stub start-kbd-macro end-kbd-macro execute-kbd-macro)
+  (kmacro-tests-should-call ((start-kbd-macro :once)
+                             (end-kbd-macro
+                              :once :after (lambda (repeat loopfunc)
+                                             (setq last-kbd-macro [1]))))
+    ;; Record one macro, with count
+    (kmacro-tests-with-current-prefix-arg (kmacro-start-macro 1))
+    (kmacro-tests-with-current-prefix-arg (kmacro-end-macro nil)))
+
+  ;; Check that execute and display do nothing with no 2nd macro
+  (kmacro-tests-should-not-call execute-kbd-macro
+    (kmacro-tests-with-current-prefix-arg (kmacro-call-ring-2nd nil)))
+  (kmacro-tests-should-match-message "Only one keyboard macro defined"
+    (kmacro-view-ring-2nd))
+
+  ;; Record another one, with format
+  (kmacro-set-format "=%d=")
+  (kmacro-tests-mock-define-macro [2 2])
+
+  ;; Execute the first one, mocked up to insert counter.
+  ;; Should get default format.
+  (kmacro-tests-should-call
+      ((execute-kbd-macro
+        :once :after
+        (lambda (mac count loopfunc)
+          (should (equal [1] mac))
+          (should (null count))
+          (kmacro-tests-with-current-prefix-arg
+           (kmacro-insert-counter nil))
+          (kmacro-tests-with-current-prefix-arg
+           (kmacro-insert-counter '(4))))))
+    (with-temp-buffer
+      (kmacro-tests-should-insert "11"
+        (kmacro-tests-with-current-prefix-arg (kmacro-call-ring-2nd nil)))))
+
+  ;; Now display and check that the [1] becomes C-a
+  (kmacro-tests-should-match-message "C-a" (kmacro-view-ring-2nd)))
+
+
+(kmacro-tests-deftest kmacro-tests-fill-ring-and-rotate ()
+  "Macro ring can do the hokey pokey, and shake it all about."
+  (:stub call-last-kbd-macro)
+  (cl-letf ((kmacro-ring-max 4))
+    ;; Record enough macros that the first one drops off the history
+    (dotimes (n (1+ kmacro-ring-max))
+      (kmacro-tests-mock-define-macro (make-vector (1+ n) (1+ n))))
+
+    ;; Cycle the ring and check that #2 comes up
+    (kmacro-tests-should-match-message "2*C-b" (kmacro-cycle-ring-next nil))
+
+    ;; Execute the current macro and check arguments
+    (kmacro-tests-should-call ((call-last-kbd-macro
+                                :once :before
+                                (lambda (count loopfunc)
+                                  (should (zerop count)))))
+      (kmacro-call-macro 0 t))
+
+    ;; Cycle the ring the other way; #5 expected
+    (kmacro-tests-should-match-message "5*C-e" (kmacro-cycle-ring-previous nil))
+    ;; Swapping the top two should give #4
+    (kmacro-tests-should-match-message "4*C-d" (kmacro-swap-ring))
+    ;; Delete the top and expect #5
+    (kmacro-tests-should-match-message "5*C-e" (kmacro-delete-ring-head))))
+
+
+(kmacro-tests-deftest kmacro-tests-test-ring-commands-when-no-macros ()
+  "Ring commands give appropriate message when no macros exist."
+  (dolist (cmd '((kmacro-cycle-ring-next nil)
+                 (kmacro-cycle-ring-previous nil)
+                 (kmacro-swap-ring)
+                 (kmacro-delete-ring-head)
+                 (kmacro-view-ring-2nd)
+                 (kmacro-call-ring-2nd nil)
+                 (kmacro-view-macro)))
+    (kmacro-tests-should-match-message "No keyboard macro defined"
+      (apply #'funcall cmd))))
+
+(kmacro-tests-deftest kmacro-tests-repeat-on-last-key ()
+  "Kmacro commands can be run in sequence without prefix keys."
+  (:stub start-kbd-macro end-kbd-macro
+         :use call-last-kbd-macro execute-kbd-macro)
+  ;; I'm attempting to make the test work even if keys have been
+  ;; rebound, but if this is failing try emacs -Q first.
+  (let* ((prefix (where-is-internal 'kmacro-keymap nil t))
+         ;; Make a sequence of events to run
+         ;; comments are expected output of mock macros
+         ;; on the first and second run of the sequence (see below)
+         (events (mapcar #'kmacro-tests-get-kmacro-key
+                         '(kmacro-end-or-call-macro-repeat ;c / b
+                           kmacro-end-or-call-macro-repeat ;c / b
+                           kmacro-call-ring-2nd-repeat     ;b / a
+                           kmacro-cycle-ring-next
+                           kmacro-end-or-call-macro-repeat ;a / a
+                           kmacro-cycle-ring-previous
+                           kmacro-end-or-call-macro-repeat ;c / b
+                           kmacro-delete-ring-head
+                           kmacro-end-or-call-macro-repeat ;b / a
+                           )))
+         ;; What we want kmacro to see as keyboard command sequence
+         (first-event (seq-concatenate
+                       'vector
+                       prefix
+                       (vector (kmacro-tests-get-kmacro-key
+                                'kmacro-end-or-call-macro-repeat)))))
+    (cl-letf
+        ;; standardize repeat options
+        ((kmacro-repeat-no-prefix t)
+         (kmacro-call-repeat-key t)
+         (kmacro-call-repeat-with-arg nil))
+
+      ;; "Record" two macros
+      (dotimes (n 2)
+        (kmacro-tests-mock-define-macro (make-vector 1 (+ ?a n))))
+      ;; Start recording #3
+      (kmacro-tests-should-call
+          ((start-kbd-macro :once))
+        (kmacro-tests-with-current-prefix-arg (kmacro-start-macro nil)))
+      ;; Set up pending keyboard events and a fresh buffer
+      ;; kmacro-set-counter is not one of the repeating kmacro
+      ;; commands so it should end the sequence.
+      (let* ((end-key (kmacro-tests-get-kmacro-key 'kmacro-set-counter))
+             (seq1 (append events (list end-key))))
+        (kmacro-tests-should-call
+            ((end-kbd-macro :once :after
+                            (lambda (repeat loopfunc)
+                              (setq last-kbd-macro [?c])))
+             (call-last-kbd-macro :times 5)
+             (execute-kbd-macro :once)
+             (read-event :times (length seq1)
+                         :around (kmacro-tests-make-mock-read-func seq1))
+             (this-single-command-keys :once :override (lambda ()
+                                                         first-event)))
+          (ert-with-test-buffer (:name "first sequence")
+            (switch-to-buffer (current-buffer))
+            (use-local-map kmacro-tests-keymap)
+            (kmacro-tests-should-insert "ccbacb"
+              ;; end #3 and launch loop to read events
+              (kmacro-end-or-call-macro-repeat nil))
+            (switch-to-prev-buffer nil t))))
+
+      ;; kmacro-edit-macro-repeat should also stop the sequence,
+      ;; so run it again with that at the end.
+      (let* ((end-key (kmacro-tests-get-kmacro-key 'kmacro-edit-macro-repeat))
+             (seq2 (append events (list end-key))))
+        (kmacro-tests-should-call
+            ((call-last-kbd-macro :times 6)
+             (execute-kbd-macro :once)
+             (edit-kbd-macro :once :override #'ignore)
+             (read-event :times (length seq2)
+                         :around (kmacro-tests-make-mock-read-func seq2))
+             (this-single-command-keys :once :override (lambda ()
+                                                         first-event)))
+          (ert-with-test-buffer (:name "second sequence")
+            (switch-to-buffer (current-buffer))
+            (use-local-map kmacro-tests-keymap)
+            (kmacro-tests-should-insert "bbbbbaaba"
+              (kmacro-end-or-call-macro-repeat 3))
+            (switch-to-prev-buffer nil t)))))))
+
+(kmacro-tests-deftest kmacro-tests-repeat-view-and-run ()
+  "Kmacro view cycles through ring and executes macro just viewed."
+  (:use execute-kbd-macro)
+  ;; I'm attempting to make the test work even if keys have been
+  ;; rebound, but if this is failing try emacs -Q first.
+  (let* ((prefix (where-is-internal 'kmacro-keymap nil t))
+         (events (mapcar #'kmacro-tests-get-kmacro-key
+                         (append (make-list 5 'kmacro-view-macro-repeat)
+                                 '(kmacro-end-or-call-macro-repeat
+                                   kmacro-set-counter))))
+         ;; What we want kmacro to see as keyboard command sequence
+         (first-event (seq-concatenate
+                       'vector
+                       prefix
+                       (vector (kmacro-tests-get-kmacro-key
+                                'kmacro-view-macro-repeat))))
+         ;; Results of repeated view-repeats
+         (macros '("d" "c" "b" "a" "d" "c")))
+    (cl-letf ((kmacro-repeat-no-prefix t)
+              (kmacro-call-repeat-key t)
+              (kmacro-call-repeat-with-arg nil))
+      ;; "Record" some macros
+      (dotimes (n 4)
+        (kmacro-tests-mock-define-macro (make-vector 1 (+ ?a n))))
+
+      ;; Set up pending keyboard events and a fresh buffer
+      (ert-with-test-buffer (:name "view sequence")
+        (switch-to-buffer (current-buffer))
+        (use-local-map kmacro-tests-keymap)
+        ;; 6 views (the direct call plus the 5 in events) should
+        ;; cycle through the ring and get to the second-to-last
+        ;; macro defined.
+        (kmacro-tests-should-insert "c"
+          (kmacro-tests-should-call
+              ((execute-kbd-macro :once)
+               (read-event :times (length events)
+                           :around (kmacro-tests-make-mock-read-func events))
+               (message :times 6
+                        :after
+                        (lambda (&rest _args)
+                          (with-current-buffer
+                              (get-buffer-create "*Messages*")
+                            (should (equal
+                                     (car macros)
+                                     (buffer-substring (- (point-max) 2)
+                                                       (- (point-max) 1))))
+                            (setq macros (cdr macros)))))
+               (this-single-command-keys :once :override (lambda ()
+                                                           first-event)))
+
+            (ert-simulate-command '(kmacro-view-macro-repeat nil))))
+        (switch-to-prev-buffer nil t)))))
+
+(kmacro-tests-deftest kmacro-tests-bind-to-key-when-recording ()
+  "Bind to key fails when recording a macro."
+  (:stub start-kbd-macro)
+  (kmacro-tests-should-call
+      ((start-kbd-macro :once))
+    (kmacro-tests-with-current-prefix-arg (kmacro-start-macro 1)))
+  (kmacro-tests-should-not-call read-key-sequence
+    (kmacro-bind-to-key nil)))
+
+(kmacro-tests-deftest kmacro-tests-name-or-bind-to-key-when-no-macro ()
+  "Bind to key, symbol or register fails when when no macro exists."
+  (should-error (kmacro-bind-to-key nil))
+  (should-error (kmacro-name-last-macro 'kmacro-tests-symbol-for-test))
+  (should-error (kmacro-to-register)))
+
+(kmacro-tests-deftest kmacro-tests-bind-to-key-bad-key-sequence ()
+  "Bind to key fails to bind to ^G."
+  (kmacro-tests-mock-define-macro [1])
+  (kmacro-tests-should-call ((read-key-sequence :once
+                                                :override (lambda (_s)
+                                                            "\C-g")))
+    (kmacro-tests-should-not-call define-key (kmacro-bind-to-key nil))))
+
+(kmacro-tests-deftest kmacro-tests-bind-to-key-with-key-sequence-in-use ()
+  "Bind to key respects yes-or-no-p when given already bound key sequence."
+  (kmacro-tests-mock-define-macro [1])
+  (with-temp-buffer
+    (switch-to-buffer (current-buffer))
+    (let ((map (make-sparse-keymap)))
+      (define-key map "\C-hi" 'info)
+      (use-local-map map))
+
+    (kmacro-tests-should-call
+        ((read-key-sequence
+          :times 2
+          :around (kmacro-tests-make-mock-read-func (make-list 2 "\C-hi"))))
+
+      ;; Try the command with yes-or-no-p set up to say no
+      (kmacro-tests-should-call ((yes-or-no-p
+                                  :once
+                                  :override
+                                  (lambda (prompt)
+                                    (should (string-match-p "info" prompt))
+                                    (should (string-match-p "C-h i" prompt))
+                                    nil)))
+        (kmacro-tests-should-not-call define-key
+          (kmacro-bind-to-key nil)))
+
+      ;; Try it again with yes
+      (kmacro-tests-should-call ((yes-or-no-p :once
+                                              :override (lambda (_prompt) t))
+                                 (define-key  :once :override
+                                   (lambda (map keys l-form)
+                                     (should (eq map global-map))
+                                     (should (equal keys "\C-hi"))
+                                     (should (functionp l-form)))))
+        (kmacro-bind-to-key nil)))
+    (switch-to-prev-buffer nil t)))
+
+
+(kmacro-tests-deftest kmacro-tests-kmacro-bind-to-single-key ()
+  "Bind to key uses C-x C-k A when asked to bind to A."
+  (:stub start-kbd-macro end-kbd-macro execute-kbd-macro)
+  ;; record a macro with counter and format set
+  (kmacro-set-format "<%d>")
+  (kmacro-tests-should-call ((start-kbd-macro :once)
+                             (end-kbd-macro :once :after
+                                            (lambda (repeat loopfunc)
+                                              (setq last-kbd-macro [1]))))
+    (kmacro-tests-with-current-prefix-arg
+     (kmacro-start-macro-or-insert-counter 5))
+    (kmacro-tests-with-current-prefix-arg (kmacro-end-macro nil)))
+
+  ;; bind it to a key and save the lambda form it makes
+  (let ((prefix (where-is-internal 'kmacro-keymap nil t))
+        lambda-form)
+    (kmacro-tests-should-call ((read-key-sequence :once
+                                                  :override (lambda (_s) "A"))
+                               (define-key :once :override
+                                 (lambda (map seq lform)
+                                   (should (eq map global-map))
+                                   (should (equal seq (concat prefix "A")))
+                                   (setq lambda-form lform))))
+      (kmacro-bind-to-key nil))
+
+    ;; record a second macro with different counter and format
+    (kmacro-set-format "%d")
+    (kmacro-tests-mock-define-macro [2])
+
+    ;; Check the lambda form and run it and verify correct counter
+    ;; and format
+    (should (equal [1] (car (kmacro-extract-lambda lambda-form))))
+    (ert-with-test-buffer (:name "bind single key")
+      (kmacro-tests-should-insert "<5>"
+        (kmacro-tests-should-call ((execute-kbd-macro
+                                    :once :after
+                                    (lambda (mac count loopfunc)
+                                      (should (equal [1] mac))
+                                      (should-not count)
+                                      (kmacro-tests-with-current-prefix-arg
+                                       (kmacro-insert-counter nil)))))
+          (funcall lambda-form))))))
+
+(kmacro-tests-deftest kmacro-tests-name-last-macro-unable-to-bind ()
+  "Name last macro won't bind to symbol which is already bound."
+  (kmacro-tests-mock-define-macro [1])
+  ;; set up a test symbol which looks like a function
+  (setplist 'kmacro-tests-symbol-for-test nil)
+  (fset 'kmacro-tests-symbol-for-test #'ignore)
+  (should-error (kmacro-name-last-macro 'kmacro-tests-symbol-for-test))
+  ;; the empty string symbol also can't be bound
+  (should-error (kmacro-name-last-macro (make-symbol ""))))
+
+(kmacro-tests-deftest kmacro-tests-name-last-macro-bind-and-rebind ()
+  "Name last macro can rebind a symbol it binds."
+  ;; make sure our symbol is unbound
+  (when (fboundp 'kmacro-tests-symbol-for-test)
+    (fmakunbound 'kmacro-tests-symbol-for-test))
+  (setplist 'kmacro-tests-symbol-for-test nil)
+  ;; make two macros and bind them to the same symbol
+  (dotimes (i 2)
+    (kmacro-tests-mock-define-macro (make-vector (1+ i) (1+ i)))
+    (kmacro-name-last-macro 'kmacro-tests-symbol-for-test)
+    (should (fboundp 'kmacro-tests-symbol-for-test)))
+
+  ;; now run the function bound to the symbol, should be the second macro
+  (kmacro-tests-should-call ((execute-kbd-macro
+                              :once :override
+                              (lambda (mac _count _loopfunc)
+                                (should (equal mac [2 2])))))
+    (funcall #'kmacro-tests-symbol-for-test)))
+
+(kmacro-tests-deftest kmacro-tests-store-in-register ()
+  "Macro can be stored in and retrieved from a register."
+  (kmacro-tests-mock-define-macro [1])
+  ;; save and restore register 200 so we can use it for the test
+  (let ((saved-reg-contents (get-register 200)))
+    (unwind-protect
+        (progn
+          (kmacro-to-register 200)
+          ;; then make a new different macro
+          (kmacro-tests-mock-define-macro [2 2])
+          (should (equal last-kbd-macro [2 2]))
+          ;; call from the register, should get first macro
+          (kmacro-tests-should-call ((call-last-kbd-macro
+                                      :once :override
+                                      (lambda (count _loopfunc)
+                                        (should (equal count 3))
+                                        (should (equal last-kbd-macro [1])))))
+            (cl-letf ((current-prefix-arg 3))
+              (jump-to-register 200 3)))
+          ;; check-that insert-register gives us human-readable macro
+          (ert-with-test-buffer (:name "register")
+            (cl-letf ((current-prefix-arg nil))
+              (insert-register 200 nil))
+            (should (equal (buffer-string) "C-a"))))
+      (set-register 200 saved-reg-contents))))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-01 ()
+  "Step-edit act, skip, insert, quit."
+  (:use call-last-kbd-macro)
+
+  ;; Stepping through macro with 'act key
+  (kmacro-tests-run-step-edit "he\C-u2lo"
+                              :events (make-list 6 'act)
+                              :result "hello"
+                              :macro-result "he\C-u2lo")
+
+  ;; Grouping similar commands using act-repeat
+  (kmacro-tests-run-step-edit "f\C-aoo\C-abar"
+                              :events (make-list 5 'act-repeat)
+                              :states '("" "f" "f" "oof" "oof")
+                              :result "baroof"
+                              :macro-result "f\C-aoo\C-abar")
+
+  ;; skipping parts of macro while editing
+  (kmacro-tests-run-step-edit "ofoofff"
+                              :events '(skip skip-keep skip-keep skip-keep
+                                             skip-rest)
+                              :result ""
+                              :macro-result "foo")
+
+  ;; quitting while step-editing leaves macro unchanged
+  (kmacro-tests-run-step-edit "bar"
+                              :events '(help insert skip help quit)
+                              :sequences '("f" "o" "o" "\C-j")
+                              :result "foo"
+                              :macro-result "bar")
+
+  ;; insert and insert-1 add to macro
+  (kmacro-tests-run-step-edit "fbazbop"
+                              :events '(insert act insert-1 act-repeat)
+                              :sequences '("o" "o" "\C-a" "\C-j" "\C-e")
+                              :result "foobazbop"
+                              :macro-result "oo\C-af\C-ebazbop"))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-replace-digit-argument ()
+  "Step-edit replace can replace numeric argument in macro."
+  (:expected-result :failed)
+  (:use call-last-kbd-macro)
+  ;; Replace numeric argument in macro
+  (kmacro-tests-run-step-edit "\C-u3b\C-a\C-cxu"
+                              :events '(act replace automatic)
+                              :sequences '("8" "x" "\C-j")
+                              :result "XXXXXXXX"
+                              :macro-result "\C-u8x\C-a\C-cxu"))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-replace ()
+  "Step-edit replace and replace-1"
+  (:use call-last-kbd-macro)
+  (kmacro-tests-run-step-edit "a\C-a\C-cxu"
+                              :events '(act act replace)
+                              :sequences '("b" "c" "\C-j")
+                              :result "bca"
+                              :macro-result "a\C-abc")
+  (kmacro-tests-run-step-edit "a\C-a\C-cxucd"
+                              :events '(act replace-1 automatic)
+                              :sequences '("b")
+                              :result "abcd"
+                              :macro-result "ab\C-cxucd")
+  (kmacro-tests-run-step-edit "by"
+                              :events '(act replace)
+                              :sequences '("a" "r" "\C-j")
+                              :result "bar"
+                              :macro-result "bar"))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-append ()
+  "Step edit append inserts after point, and append-end inserts at end."
+  (:use call-last-kbd-macro)
+
+  (kmacro-tests-run-step-edit "f-b"
+                              :events '(append append-end)
+                              :sequences '("o" "o" "\C-j" "a" "r" "\C-j")
+                              :result "foo-bar"
+                              :macro-result "foo-bar")
+  (kmacro-tests-run-step-edit "x"
+                              :events '(append)
+                              :sequences '("\C-a" "\C-cxu" "\C-e" "y" "\C-j")
+                              :result "Xy"
+                              :macro-result "x\C-a\C-cxu\C-ey"))
+
+(kmacro-tests-deftest kmacro-tests-append-end-at-end-appends ()
+  "Append-end when already at end of macro appends to end of macro."
+  (:expected-result :failed)
+  (:use call-last-kbd-macro)
+
+  (kmacro-tests-run-step-edit "x"
+                              :events '(append-end)
+                              :sequences '("\C-a" "\C-cxu" "\C-e" "y" "\C-j")
+                              :result "Xy"
+                              :macro-result "x\C-a\C-cxu\C-ey"))
+
+
+(kmacro-tests-deftest kmacro-tests-step-edit-skip-entire ()
+  "Skipping whole macro in step-edit leaves macro unchanged."
+  (:expected-result :failed)
+  (:use call-last-kbd-macro)
+
+  (kmacro-tests-run-step-edit "xyzzy"
+                              :events '(skip-rest)
+                              :result ""
+                              :macro-result "xyzzy"))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-with-quoted-insert ()
+  "Stepping through a macro that uses quoted insert leaves macro unchanged."
+  (:expected-result :failed)
+  (:use call-last-kbd-macro)
+  (cl-letf ((read-quoted-char-radix 8))
+    (kmacro-tests-run-step-edit "\C-cxq17051i there"
+                                :events '(act automatic)
+                                :result "ḩi there"
+                                :macro-result "\C-cxq17051i there")))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-step-through-negative-argument ()
+  "Step edit works on macros using negative universal argument."
+  (:expected-result :failed)
+  (:use call-last-kbd-macro)
+  (kmacro-tests-run-step-edit "boo\C-u-\C-cu"
+                              :events '(act-repeat automatic )
+                              :result "BOO"
+                              :macro-result "boo\C-u-\C-cd"))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-can-insert-before-quoted-insert ()
+  "Step edit can insert  before a quoted-insert in a macro."
+  (:expected-result :failed)
+  (:use call-last-kbd-macro)
+  (cl-letf ((read-quoted-char-radix 8))
+    (kmacro-tests-run-step-edit "g\C-cxq17051i"
+                                :events '(act insert-1 automatic)
+                                :sequences '("-")
+                                :result "g-ḩi"
+                                :macro-result "g-\C-cxq17051i")))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-ignores-qr-map-commands ()
+  "Unimplemented commands from query-replace map are ignored."
+  (:use call-last-kbd-macro)
+  (kmacro-tests-run-step-edit "yep"
+                              :events '(edit-replacement
+                                        act-and-show act-and-exit
+                                        delete-and-edit
+                                        recenter backup
+                                        scroll-up scroll-down
+                                        scroll-other-window
+                                        scroll-other-window-down
+                                        exit-prefix
+                                        act act act)
+                              :result "yep"
+                              :macro-result "yep"))
+
+(kmacro-tests-deftest
+    kmacro-tests-step-edit-edits-macro-with-extended-command ()
+  "Step-editing a macro which uses the minibuffer can change the macro."
+  (:use call-last-kbd-macro)
+  (let ((mac (vconcat [?\M-x] "eval-expression" '[return]
+                      "(insert-char (+ ?a \C-e" [?1] "))" '[return]))
+        (mac-after (vconcat [?\M-x] "eval-expression" '[return]
+                            "(insert-char (+ ?a \C-e" [?2] "))" '[return])))
+
+    (kmacro-tests-run-step-edit mac
+                                :events '(act act-repeat
+                                              act act-repeat act
+                                              replace-1 act-repeat act)
+                                :sequences '("2")
+                                :result "c"
+                                :macro-result mac-after)))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-step-through-isearch ()
+  "Step-editing can edit a macro which uses isearch-backward (Bug#22488)."
+  (:expected-result :failed)
+  (:use call-last-kbd-macro)
+  (let ((mac (vconcat "test Input" '[return]
+                      [?\C-r] "inp" '[return] "\C-cxu"))
+        (mac-after (vconcat "test input" '[return]
+                            [?\C-r] "inp" '[return] "\C-cd")))
+
+    (kmacro-tests-run-step-edit mac
+                                :events '(act-repeat act act
+                                                     act-repeat act
+                                                     replace-1)
+                                :sequences '("\C-cd")
+                                :result "test input\n"
+                                :macro-result mac-after)))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-cleans-up-hook ()
+  "Step-editing properly cleans up post-command-hook. (Bug #18708)"
+  (:expected-result :failed)
+  (:use call-last-kbd-macro)
+  (let* ((mac "x")
+         (mac-after "x"))
+    (kmacro-tests-run-step-edit mac
+                                :setup (lambda ()
+                                         (setq-local post-command-hook '(t)))
+                                :teardown (lambda ()
+                                            (ert-simulate-command
+                                             '(beginning-of-line)))
+                                :events '(act)
+                                :result "x"
+                                :macro-result mac-after)))
+
+
+(cl-defun kmacro-tests-run-step-edit
+    (macro &key events sequences setup states teardown result macro-result)
+  "Implement a sort of macro to test editing other macros.
+
+Run kmacro-step-edit-macro with MACRO defined as a keyboard macro
+and mock functions attached to read-event and read-key-sequence
+which will return items from EVENTS and SEQUENCES
+respectively.  SEQUENCES may be nil, but EVENTS should not be.
+EVENTS should be a list of symbols bound in kmacro-step-edit-map
+or query-replace map, and this function will do the keymap lookup
+for you. SEQUENCES should contain return values for
+read-key-sequence.
+
+During execution an ert test buffer will be current.  SETUP, if
+provided, should be a function taking no arguments which will be
+called after the buffer for the test is created and before the
+macro runs.  STATES may be nil or a list of strings, and if it is
+the latter, before each call to read-event, the contents of the
+buffer will be expected to match the strings in the
+list. TEARDOWN, if non-nil, should be a function taking no
+arguments which will be called after the macro is run and before
+the buffer is deleted.
+
+RESULT is the string that should be inserted in (the empty test)
+buffer during the step-editing process, and MACRO-RESULT is the
+expected value of last-kbd-macro after the editing is
+complete."
+
+  (let* ((event-keys (mapcar #'kmacro-tests-get-kmacro-step-edit-key events))
+         (mock-read-event (kmacro-tests-make-mock-read-func event-keys))
+         (current-state states))
+
+    (kmacro-tests-mock-define-macro (string-to-vector macro))
+    (ert-with-test-buffer (:name "run-step-edit")
+      (when setup
+        (funcall setup))
+      (switch-to-buffer (current-buffer))
+      (use-local-map kmacro-tests-keymap)
+      (kmacro-tests-should-call
+          ((call-last-kbd-macro :once)
+           (read-event :check-args-with
+                       (lambda (arglists)
+                         ;; extra calls may come from sit-for
+                         (>= (length arglists) (length event-keys)))
+                       :around
+                       (lambda (&rest args)
+                         (when states
+                           (should (equal (car current-state) (buffer-string)))
+                           (setq current-state (cdr current-state)))
+                         (apply mock-read-event args)))
+           (read-key-sequence
+            :times (length sequences)
+            :around (kmacro-tests-make-mock-read-func sequences)))
+        (kmacro-step-edit-macro))
+      (when result
+        (should (equal result (buffer-string))))
+      (when macro-result
+        (should (equal last-kbd-macro (string-to-vector macro-result))))
+      (when teardown
+        (funcall teardown))
+      (switch-to-prev-buffer nil t))))
+
+;;; Mocks
+
+(defun kmacro-tests-mock-define-macro (mac)
+  "Mock-define a macro.
+Call into kmacro.el to start and end macro recording with the
+macros.c calls mocked to set `last-kbd-macro' to MAC.  Does its own
+mocking of `start-kbd-macro' and `end-kbd-macro' so they don't have to
+be stubbed by the calling test."
+  (kmacro-tests-should-call
+      ((start-kbd-macro :once :override
+                        (alist-get 'start-kbd-macro kmacro-tests-stubs))
+       (end-kbd-macro :once :override
+                      (lambda (&rest args)
+                        (apply
+                         (alist-get 'end-kbd-macro kmacro-tests-stubs) args)
+                        (setq last-kbd-macro mac))))
+    (kmacro-tests-with-current-prefix-arg (kmacro-start-macro nil))
+    (kmacro-tests-with-current-prefix-arg (kmacro-end-macro nil))))
+
+(defun kmacro-tests-get-kmacro-key (sym)
+  "Look up kmacro command SYM in kmacro's keymap.
+Return the integer key value found."
+  (aref (where-is-internal sym kmacro-keymap t) 0))
+
+(defun kmacro-tests-get-kmacro-step-edit-key (sym)
+  "Return the first key bound to SYM in kmacro-step-edit-map."
+  (let ((where (aref (where-is-internal sym kmacro-step-edit-map t) 0)))
+    (cond
+     ((consp where) (car where))
+     (t where))))
+
+
+(defun kmacro-tests-make-mock-read-func (events)
+  "Create around advice for `read-event' or `read-sequence'.
+If a keyboard macro is being executed, call the real function.
+Otherwise return the items in EVENTS one by one until
+they are exhausted, at which point start calling the real
+read function."
+  (lambda (orig-func &rest _args)
+    (if (or executing-kbd-macro (null events))
+        (funcall orig-func)
+      (prog1
+          (car events)
+        (setq events (cdr events))))))
+
+
+
+(provide 'kmacro-tests)
+
+;;; kmacro-tests.el ends here
-- 
2.10.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#24939: [PATCH] Add tests for lisp/kmacro.el
  2016-11-13 21:23 bug#24939: [PATCH] Add tests for lisp/kmacro.el Gemini Lasswell
@ 2016-11-14 15:49 ` Eli Zaretskii
  2016-11-14 18:26   ` Gemini Lasswell
  2016-12-31 17:42   ` Gemini Lasswell
  0 siblings, 2 replies; 11+ messages in thread
From: Eli Zaretskii @ 2016-11-14 15:49 UTC (permalink / raw)
  To: Gemini Lasswell; +Cc: 24939

> From: Gemini Lasswell <gazally@runbox.com>
> Date: Sun, 13 Nov 2016 13:23:51 -0800
> 
> There weren't any tests for kmacro.el, so I have written some.

Thanks!

> 1. These tests make extensive use of two macros, called
> kmacro-tests-should-call and kmacro-tests-should-not-call. They are
> context-creating macros which add advice to named functions for the
> duration of a test. I think that these two macros would be a useful
> addition to ERT, and I'll submit that idea as a separate patch.

Thanks, I respond to this point there.

> 2. I found several minor bugs in the process of writing these, leading
> to tests marked as :expected-result :failed. One is a way to create an
> empty keyboard macro using the mouse and the rest are ways to get
> kmacro-step-edit-macro to behave oddly. I haven't sent them to
> bug-gnu-emacs yet. When I do so would it be better to send them
> individually or put all the step-edit ones in one report?

It's a judgment call.  If the offending function is small enough (it
is in this case), and the required changes aren't expected to be too
complex, then one report is better.

Allow me a few comments to your proposed patch.

> +(defmacro kmacro-tests-should-not-call (func-or-funcs &rest body)
> +  "Verify that a function or functions are not called during execution.
> +FUNC-OR-FUNCS can either be a single function or a list of them.
> +Temporarily override them them during the execution of BODY with advice
                        ^^^^^^^^^
One "them" is redundant.

> +containing `should-not' and a non-nil value.  Use the function symbol
> +as the non-nil value to make tracking down what went wrong a bit
> +faster."

A general comment: I think your doc strings describe the
implementation too intimately.  I appreciate the effort that went into
doing that, but the result has 2 disadvantages: (1) the doc string
needs much more maintenance than it should, because any change to the
implementation will need the doc string to be updated (which runs a
high risk of the doc becoming out of sync), and (2) the user of the
function needs to wade through details she doesn't really need to
know.

A doc string should describe the arguments, the return value, and the
side effects, if any, of the function, without going into the
implementation.

> +(defmacro kmacro-tests-should-insert (value &rest body)
> +  "Verify that VALUE is inserted by the execution of BODY.
> +Execute BODY, then check that the string VALUE was inserted
> +into the current buffer at point.  As a side effect captures the
> +symbols p_ and bsize_."

Without explaining what p_ and bsize_ are, the last sentence of the
doc string are not really useful, IMO.

> +  (declare (debug (stringp body))
> +           (indent 1))
> +  `(let ((p_ (point))
> +         (bsize_ (buffer-size)))
> +     ,@body
> +     (should (equal (buffer-substring p_ (point)) ,value))
> +     (should (equal (- (buffer-size) bsize_) (string-width ,value)))))
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This doesn't look right to me: string-width takes the char-width of
each character into account, so it cannot be reliably compared to
buffer size.  I think you want to call length instead.

> +(defmacro kmacro-tests-should-match-message (value &rest body)
> +  "Verify that a message matching VALUE is issued while executing BODY.
> +Execute BODY, then check for a regexp match between
> +VALUE and any text written to *Messages* during the execution."
> +  (declare (debug (stringp body))
> +           (indent 1))
> +  `(with-current-buffer (get-buffer-create "*Messages*")
> +     (save-restriction
> +       (narrow-to-region (point-max) (point-max))
> +       ,@body
> +       (should (string-match-p ,value (buffer-string))))))

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.

> +(kmacro-tests-deftest kmacro-tests-test-insert-counter-01-nil ()
> +  "Insert counter adds one with nil arg."
      ^^^^^^^^^^^^^^
I think you meant "Inserted counter", here and elsewhere.

> +(kmacro-tests-deftest kmacro-tests-test-start-macro-when-defining-macro ()
> +  "Starting a macro while defining a macro does not start a second macro."
> +  ;; Verify kmacro-start-macro calls start-kbd-macro
> +  (:stub start-kbd-macro)
> +  (kmacro-tests-should-call
> +      ((start-kbd-macro :once :before (lambda (append noexec)
> +                                        (should-not append)
> +                                        (should-not noexec))))
> +    (kmacro-tests-with-current-prefix-arg (kmacro-start-macro nil)))
> +  ;; And leaves macro in being-recorded state
> +  (should defining-kbd-macro)
> +  ;; And that it doesn't call it again
> +  (kmacro-tests-should-not-call start-kbd-macro
> +    (kmacro-tests-with-current-prefix-arg (kmacro-start-macro nil))))

I have a grave issue with this kind of testing: it is too tied to the
implementation, instead of testing the behavior and the effects.
E.g., the fact that start-kbd-macro is only called once isn't a
reliable evidence that "starting a macro while defining a macro does
not start a second macro"; a reliable evidence would be to verify that
the second macro isn't defined after the form finishes.

I think there's a fundamental issue here, regarding the purpose of our
test suite and consequently the methodology we should use for writing
the tests.

More about this in my response to your proposed ert-should-call and
ert-should-not-call additions.  Here I just note that most of your
tests are based on this paradigm, which I think makes these tests much
more complex and fragile than they need to be.

> +(kmacro-tests-deftest kmacro-tests-set-macro-counter-while-defining ()
> +  "Use of the prefix arg with kmacro-start sets kmacro-counter."
> +  (:stub start-kbd-macro)
> +  ;; Give kmacro-start-macro an argument
> +  (kmacro-tests-should-call
> +      ((start-kbd-macro :once :before (lambda (append noexec)
> +                                        (should-not append)
> +                                        (should-not noexec))))
> +    (kmacro-tests-with-current-prefix-arg (kmacro-start-macro 5)))
> +  (should defining-kbd-macro)
> +  ;; And verify that the counter is set to that value
> +  (ert-with-test-buffer (:name "counter-while-defining")
> +    (kmacro-tests-should-insert "5"
> +      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter nil)))
> +    ;; change it while defining macro
> +    (kmacro-tests-with-current-prefix-arg (kmacro-set-counter 1))
> +    (kmacro-tests-should-insert "1"
> +      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter nil)))
> +    ;; using universal arg to to set counter should reset to starting value

Please use a consistent style in comments: begin with a capital letter
and end with a period.  These are our code conventions.

> +      (message "")  ; clear the echo area
> +      (kmacro-tests-should-match-message "Type e to repeat macro"

Why is that call to 'message' necessary here?  I suspect this is one
symptom of the fragility of kmacro-tests-should-match-message that I
mentioned above.

> +(kmacro-tests-deftest kmacro-tests-test-ring-commands-when-no-macros ()
> +  "Ring commands give appropriate message when no macros exist."
> +  (dolist (cmd '((kmacro-cycle-ring-next nil)
> +                 (kmacro-cycle-ring-previous nil)
> +                 (kmacro-swap-ring)
> +                 (kmacro-delete-ring-head)
> +                 (kmacro-view-ring-2nd)
> +                 (kmacro-call-ring-2nd nil)
> +                 (kmacro-view-macro)))
> +    (kmacro-tests-should-match-message "No keyboard macro defined"
> +      (apply #'funcall cmd))))
         ^^^^^^^^^^^^^^^^^^^^^
Why not ert-simulate-command?

> +  ;; I'm attempting to make the test work even if keys have been
> +  ;; rebound, but if this is failing try emacs -Q first.

If this comment is still valid, then many other parts of the test have
the same problem, because they clearly assume the default key
bindings.  If these tests are to be run as part of the test suite in
"emacs -batch" or "emacs -Q", then these problems don't exist; but if
not, your "clean slate" should somehow restore the default key
bindings, or else call the functions by name.

Thanks again for working on this.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#24939: [PATCH] Add tests for lisp/kmacro.el
  2016-11-14 15:49 ` Eli Zaretskii
@ 2016-11-14 18:26   ` Gemini Lasswell
  2016-11-14 18:47     ` Eli Zaretskii
  2016-12-31 17:42   ` Gemini Lasswell
  1 sibling, 1 reply; 11+ messages in thread
From: Gemini Lasswell @ 2016-11-14 18:26 UTC (permalink / raw)
  To: Eli Zaretskii, 24939

Thanks for the thorough feedback! I'll work on a new & improved version
incorporating your comments.
Some responses/questions below.

>> +(defmacro kmacro-tests-should-match-message (value &rest body)
>> +  "Verify that a message matching VALUE is issued while executing BODY.
>> +Execute BODY, then check for a regexp match between
>> +VALUE and any text written to *Messages* during the execution."
>> +  (declare (debug (stringp body))
>> +           (indent 1))
>> +  `(with-current-buffer (get-buffer-create "*Messages*")
>> +     (save-restriction
>> +       (narrow-to-region (point-max) (point-max))
>> +       ,@body
>> +       (should (string-match-p ,value (buffer-string))))))
>
> I don't like this implementation.

This strategy is used in autorevert-tests.el and filenotify-tests.el,
which is where I copied it from. So those should be changed too.

> 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.

That would definitely be helpful.

>> +      (message "")  ; clear the echo area
>> +      (kmacro-tests-should-match-message "Type e to repeat macro"
>
> Why is that call to 'message' necessary here?  I suspect this is one
> symptom of the fragility of kmacro-tests-should-match-message that I
> mentioned above.

Actually not. I was attempting to write a regression for bug#11817,
which was about the "Type e to repeat macro" not showing up in the echo
area. But kmacro-call-macro doesn't put up the message if there's
already a message. I just looked into the history of that bit of code,
and kmacro-call-macro's check of (current-message) is actually another
bug fix, for bug#3412.

>> +(kmacro-tests-deftest kmacro-tests-test-ring-commands-when-no-macros ()
>> +  "Ring commands give appropriate message when no macros exist."
>> +  (dolist (cmd '((kmacro-cycle-ring-next nil)
>> +                 (kmacro-cycle-ring-previous nil)
>> +                 (kmacro-swap-ring)
>> +                 (kmacro-delete-ring-head)
>> +                 (kmacro-view-ring-2nd)
>> +                 (kmacro-call-ring-2nd nil)
>> +                 (kmacro-view-macro)))
>> +    (kmacro-tests-should-match-message "No keyboard macro defined"
>> +      (apply #'funcall cmd))))
>          ^^^^^^^^^^^^^^^^^^^^^
> Why not ert-simulate-command?

I'll make that change. What do you think about changing
ert-simulate-command to set current-prefix-arg? That would be very
helpful.

>> +  ;; I'm attempting to make the test work even if keys have been
>> +  ;; rebound, but if this is failing try emacs -Q first.
>
> If this comment is still valid, then many other parts of the test have
> the same problem, because they clearly assume the default key
> bindings.

That comment is out of date, since kmacro-tests-keymap should fix the
problem. I'll remove it.






^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#24939: [PATCH] Add tests for lisp/kmacro.el
  2016-11-14 18:26   ` Gemini Lasswell
@ 2016-11-14 18:47     ` Eli Zaretskii
  2016-11-29 20:56       ` Gemini Lasswell
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-11-14 18:47 UTC (permalink / raw)
  To: Gemini Lasswell; +Cc: 24939

> From: Gemini Lasswell <gazally@runbox.com>
> Cc: 
> Date: Mon, 14 Nov 2016 10:26:42 -0800
> 
> >> +(defmacro kmacro-tests-should-match-message (value &rest body)
> >> +  "Verify that a message matching VALUE is issued while executing BODY.
> >> +Execute BODY, then check for a regexp match between
> >> +VALUE and any text written to *Messages* during the execution."
> >> +  (declare (debug (stringp body))
> >> +           (indent 1))
> >> +  `(with-current-buffer (get-buffer-create "*Messages*")
> >> +     (save-restriction
> >> +       (narrow-to-region (point-max) (point-max))
> >> +       ,@body
> >> +       (should (string-match-p ,value (buffer-string))))))
> >
> > I don't like this implementation.
> 
> This strategy is used in autorevert-tests.el and filenotify-tests.el,
> which is where I copied it from. So those should be changed too.

Most probably.  But let's first see what better implementation we
could come up with.

> > 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.
> 
> That would definitely be helpful.

If you can provide a reasonable spec for such a feature, I'm sure
Someone™ will make it happen.

> >> +    (kmacro-tests-should-match-message "No keyboard macro defined"
> >> +      (apply #'funcall cmd))))
> >          ^^^^^^^^^^^^^^^^^^^^^
> > Why not ert-simulate-command?
> 
> I'll make that change. What do you think about changing
> ert-simulate-command to set current-prefix-arg? That would be very
> helpful.

Sounds like a useful extension to me.

> >> +  ;; I'm attempting to make the test work even if keys have been
> >> +  ;; rebound, but if this is failing try emacs -Q first.
> >
> > If this comment is still valid, then many other parts of the test have
> > the same problem, because they clearly assume the default key
> > bindings.
> 
> That comment is out of date, since kmacro-tests-keymap should fix the
> problem. I'll remove it.

OK, then there are a couple more such comments in the patch.

Thanks.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#24939: [PATCH] Add tests for lisp/kmacro.el
  2016-11-14 18:47     ` Eli Zaretskii
@ 2016-11-29 20:56       ` Gemini Lasswell
  2016-11-30  9:08         ` Michael Albinus
  0 siblings, 1 reply; 11+ messages in thread
From: Gemini Lasswell @ 2016-11-29 20:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 24939

Eli Zaretskii <eliz@gnu.org> writes:

>> >> +(defmacro kmacro-tests-should-match-message (value &rest body)
>> >
>> > I don't like this implementation.
>>
>> This strategy is used in autorevert-tests.el and filenotify-tests.el,
>> which is where I copied it from. So those should be changed too.
>
> Most probably.  But let's first see what better implementation we
> could come up with.
>

Here is an implementation of message capturing using advice:

(defmacro ert-with-message-capture (var &rest body)
  "Execute BODY while collecting messages in VAR.
Capture all messages produced by `message' when it is called from
Lisp, and concatenate them separated by newlines into one string."
  (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)))))

The reason I have set it up to bind a variable during the code body
execution is to make it usable by autorevert-tests.el and
filenotify-tests.el. For example, here's an excerpt from
filenotify-tests.el:

(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
    (format-message "Reverting buffer `%s'." (buffer-name buf))
    (buffer-string))))

file-notify--wait-for-events polls read-event in a loop with a
fractional timeout until the form that is its second argument becomes
non-nil, or the larger timeout expires. With the macro above, this
excerpt becomes:

(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))
      captured-messages)))

Let me know what you think.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#24939: [PATCH] Add tests for lisp/kmacro.el
  2016-11-29 20:56       ` Gemini Lasswell
@ 2016-11-30  9:08         ` Michael Albinus
  2016-11-30 15:51           ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Albinus @ 2016-11-30  9:08 UTC (permalink / raw)
  To: Gemini Lasswell; +Cc: 24939

Gemini Lasswell <gazally@runbox.com> writes:

Hi,

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> > I don't like this implementation.
>>>
>>> This strategy is used in autorevert-tests.el and filenotify-tests.el,
>>> which is where I copied it from. So those should be changed too.
>>
>> Most probably.  But let's first see what better implementation we
>> could come up with.
>>
> Let me know what you think.

At first glance, looks OK to me. However, I didn't understand why the
existing implementation (watching messages in *Messages*) is bad.

Eli?

Best regards, Michael.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#24939: [PATCH] Add tests for lisp/kmacro.el
  2016-11-30  9:08         ` Michael Albinus
@ 2016-11-30 15:51           ` Eli Zaretskii
  2016-11-30 16:51             ` Michael Albinus
  2016-12-10 17:46             ` Gemini Lasswell
  0 siblings, 2 replies; 11+ messages in thread
From: Eli Zaretskii @ 2016-11-30 15:51 UTC (permalink / raw)
  To: Michael Albinus; +Cc: gazally, 24939

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Eli Zaretskii <eliz@gnu.org>,  24939@debbugs.gnu.org
> Date: Wed, 30 Nov 2016 10:08:46 +0100
> 
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> >>> > I don't like this implementation.
> >>>
> >>> This strategy is used in autorevert-tests.el and filenotify-tests.el,
> >>> which is where I copied it from. So those should be changed too.
> >>
> >> Most probably.  But let's first see what better implementation we
> >> could come up with.
> >>
> > Let me know what you think.
> 
> At first glance, looks OK to me.

I agree.  Thanks for working on this, Gemini.

> However, I didn't understand why the existing implementation
> (watching messages in *Messages*) is bad.

Because the display engine treats that buffer specially, and assumes
all kinds of assumptions when it does.  See message_dolog in xdisp.c
for the gory details.

So I'd like us to futz as little as possible with *Messages*, lest we
violate those assumptions.  In particular, setting buffer restrictions
might get in the way.  (Granted, this is only a problem when the test
suite is run interactively, but AFAIK this is actually done
sometimes.)





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#24939: [PATCH] Add tests for lisp/kmacro.el
  2016-11-30 15:51           ` Eli Zaretskii
@ 2016-11-30 16:51             ` Michael Albinus
  2016-12-10 17:46             ` Gemini Lasswell
  1 sibling, 0 replies; 11+ messages in thread
From: Michael Albinus @ 2016-11-30 16:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gazally, 24939

Eli Zaretskii <eliz@gnu.org> writes:

>> However, I didn't understand why the existing implementation
>> (watching messages in *Messages*) is bad.
>
> Because the display engine treats that buffer specially, and assumes
> all kinds of assumptions when it does.  See message_dolog in xdisp.c
> for the gory details.
>
> So I'd like us to futz as little as possible with *Messages*, lest we
> violate those assumptions.  In particular, setting buffer restrictions
> might get in the way.  (Granted, this is only a problem when the test
> suite is run interactively, but AFAIK this is actually done
> sometimes.)

I didn't know this, thanks for the explanation. So I'm OK with the change.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#24939: [PATCH] Add tests for lisp/kmacro.el
  2016-11-30 15:51           ` Eli Zaretskii
  2016-11-30 16:51             ` Michael Albinus
@ 2016-12-10 17:46             ` Gemini Lasswell
  1 sibling, 0 replies; 11+ messages in thread
From: Gemini Lasswell @ 2016-12-10 17:46 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Michael Albinus, 24939

I've submitted a patch to add the message capturing macro to ert-x.el as
a separate issue, #25158.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#24939: [PATCH] Add tests for lisp/kmacro.el
  2016-11-14 15:49 ` Eli Zaretskii
  2016-11-14 18:26   ` Gemini Lasswell
@ 2016-12-31 17:42   ` Gemini Lasswell
  2017-02-04 11:57     ` Eli Zaretskii
  1 sibling, 1 reply; 11+ messages in thread
From: Gemini Lasswell @ 2016-12-31 17:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 24939

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

Eli Zaretskii <eliz@gnu.org> writes:
> Allow me a few comments to your proposed patch.

Here is an updated version taking your comments into account. I was able
to remove the use of advice on all the functions in macros.c except for
end-kbd-macro, because I couldn't come up with another way to make
kmacro-end-macro recognize a keyboard macro defined by a test.
kmacro-end-macro tests last-kbd-macro right after it calls
end-kbd-macro, and end-kbd-macro is going to leave last-kbd-macro empty
unless it has something in current_kboard->kbd_macro_buffer, which Lisp
can't access. If there is another strategy you'd like me to try there,
let me know.

This patch contains kmacro-tests-with-message-capture which is the same
as the macro proposed as an addition to ert-x.el in bug#25158, so if
that patch is adopted, it could be removed from this patch.


[-- Attachment #2: 0001-Add-tests-for-lisp-kmacro.el.patch --]
[-- Type: text/plain, Size: 43156 bytes --]

From ad117828d954ddb8c37d134055d1a74b1344b078 Mon Sep 17 00:00:00 2001
From: gazally <gazally@users.noreply.github.com>
Date: Sun, 13 Nov 2016 08:02:38 -0800
Subject: [PATCH] Add tests for lisp/kmacro.el

* test/lisp/kmacro-tests.el: New file.
---
 test/lisp/kmacro-tests.el | 907 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 907 insertions(+)
 create mode 100644 test/lisp/kmacro-tests.el

diff --git a/test/lisp/kmacro-tests.el b/test/lisp/kmacro-tests.el
new file mode 100644
index 0000000..fbf9685
--- /dev/null
+++ b/test/lisp/kmacro-tests.el
@@ -0,0 +1,907 @@
+;;; kmacro-tests.el --- Tests for kmacro.el       -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2016 Free Software Foundation, Inc.
+
+;; Author: Gemini Lasswell
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'kmacro)
+(require 'ert)
+(require 'ert-x)
+
+;;; Test fixtures:
+
+(defmacro kmacro-tests-with-kmacro-clean-slate (&rest body)
+  "Create a clean environment for a kmacro test BODY to run in."
+  (declare (debug (body)))
+  `(cl-letf* ((kmacro-execute-before-append t)
+              (kmacro-ring-max 8)
+              (kmacro-repeat-no-prefix t)
+              (kmacro-call-repeat-key nil)
+              (kmacro-call-repeat-with-arg nil)
+
+              (kbd-macro-termination-hook nil)
+              (defining-kbd-macro nil)
+              (executing-kbd-macro nil)
+              (executing-kbd-macro-index 0)
+              (last-kbd-macro nil)
+
+              (kmacro-ring nil)
+
+              (kmacro-counter 0)
+              (kmacro-default-counter-format "%d")
+              (kmacro-counter-format "%d")
+              (kmacro-counter-format-start "%d")
+              (kmacro-counter-value-start 0)
+              (kmacro-last-counter 0)
+              (kmacro-initial-counter-value nil)
+
+              (kmacro-tests-macros nil)
+              (kmacro-tests-events nil)
+              (kmacro-tests-sequences nil))
+     (advice-add 'end-kbd-macro :after #'kmacro-tests-end-macro-advice)
+     (advice-add 'read-event :around #'kmacro-tests-read-event-advice )
+     (advice-add 'read-key-sequence :around #'kmacro-tests-read-key-sequence-advice)
+     (unwind-protect
+         (ert-with-test-buffer (:name "")
+           (switch-to-buffer (current-buffer))
+           ,@body)
+       (advice-remove 'read-key-sequence #'kmacro-tests-read-key-sequence-advice)
+       (advice-remove 'read-event #'kmacro-tests-read-event-advice)
+       (advice-remove 'end-kbd-macro #'kmacro-tests-end-macro-advice))))
+
+(defmacro kmacro-tests-deftest (name _args docstring &rest keys-and-body)
+  "Define a kmacro unit test.
+NAME is the name of the test, _ARGS should be nil, and DOCSTRING
+is required.  To avoid having to duplicate ert's keyword parsing
+here, its keywords and values (if any) must be inside a list
+after the docstring, preceding the body, here combined with the
+body in KEYS-AND-BODY."
+  (declare (debug (&define name sexp stringp
+                           [&optional (&rest &or [keywordp sexp])]
+                           def-body))
+           (doc-string 3)
+           (indent 2))
+
+  (let* ((keys (when (and (listp (car keys-and-body))
+                          (keywordp (caar keys-and-body)))
+                 (car keys-and-body)))
+         (body (if keys (cdr keys-and-body)
+                 keys-and-body)))
+    `(ert-deftest ,name ()
+       ,docstring ,@keys
+       (kmacro-tests-with-kmacro-clean-slate ,@body))))
+
+(defvar kmacro-tests-keymap
+  (let ((map (make-sparse-keymap)))
+    (dotimes (i 26)
+      (define-key map (string (+ ?a i)) 'self-insert-command))
+    (dotimes (i 10)
+      (define-key map (string (+ ?0 i)) 'self-insert-command))
+    ;; Define a few key sequences of different lengths.
+    (dolist (item '(("\C-a"     . beginning-of-line)
+                    ("\C-b"     . backward-char)
+                    ("\C-e"     . end-of-line)
+                    ("\C-f"     . forward-char)
+                    ("\C-r"     . isearch-backward)
+                    ("\C-u"     . universal-argument)
+                    ("\C-w"     . kill-region)
+                    ("\C-SPC"   . set-mark-command)
+                    ("\M-w"     . kill-ring-save)
+                    ("\M-x"     . execute-extended-command)
+                    ("\C-cd"    . downcase-word)
+                    ("\C-cxu"   . upcase-word)
+                    ("\C-cxq"   . quoted-insert)
+                    ("\C-cxi"   . kmacro-insert-counter)
+                    ("\C-x\C-k" . kmacro-keymap)))
+      (define-key map (car item) (cdr item)))
+    map)
+  "Keymap to use for testing keyboard macros.
+This is used to obtain consistent results even if tests are run
+in an environment with rebound keys.")
+
+(defvar kmacro-tests-events nil
+  "Input events used by the kmacro test in progress.")
+
+(defun kmacro-tests-read-event-advice (orig-func &rest args)
+  "Pop and return an event from `kmacro-tests-events'.
+Return the result of calling ORIG-FUNC with ARGS if
+`kmacro-tests-events' is empty, or if a keyboard macro is
+running."
+  (if (or executing-kbd-macro (null kmacro-tests-events))
+      (apply orig-func args)
+    (pop kmacro-tests-events)))
+
+(defvar kmacro-tests-sequences nil
+  "Input sequences used by the kmacro test in progress.")
+
+(defun kmacro-tests-read-key-sequence-advice (orig-func &rest args)
+  "Pop and return a string from `kmacro-tests-sequences'.
+Return the result of calling ORIG-FUNC with ARGS if
+`kmacro-tests-sequences' is empty, or if a keyboard macro is
+running."
+  (if (or executing-kbd-macro (null kmacro-tests-sequences))
+      (apply orig-func args)
+    (pop kmacro-tests-sequences)))
+
+(defvar kmacro-tests-macros nil
+  "Keyboard macros (in vector form) used by the kmacro test in progress.")
+
+(defun kmacro-tests-end-macro-advice (&rest _args)
+  "Pop a macro from `kmacro-tests-macros' and assign it to `last-kbd-macro'.
+If `kmacro-tests-macros' is empty, do nothing."
+  (when kmacro-tests-macros
+    (setq last-kbd-macro (pop kmacro-tests-macros))))
+
+;;; Some more powerful expectations:
+
+(defmacro kmacro-tests-should-insert (value &rest body)
+  "Verify that VALUE is inserted by the execution of BODY.
+Execute BODY, then check that the string VALUE was inserted
+into the current buffer at point."
+  (declare (debug (stringp body))
+           (indent 1))
+  (let ((g-p (cl-gensym))
+        (g-bsize (cl-gensym)))
+    `(let ((,g-p (point))
+           (,g-bsize (buffer-size)))
+       ,@body
+       (should (equal (buffer-substring ,g-p (point)) ,value))
+       (should (equal (- (buffer-size) ,g-bsize) (length ,value))))))
+
+(defmacro kmacro-tests-with-message-capture (var &rest body)
+  "Execute BODY while collecting anything written with `message' in VAR."
+  (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)))))
+
+(defmacro kmacro-tests-should-match-message (value &rest body)
+  "Verify that a message matching VALUE is issued while executing BODY.
+Execute BODY, and then if there is not a regexp match between
+VALUE and any text written to *Messages* during the execution,
+cause the current test to fail."
+  (declare (debug (form body))
+           (indent 1))
+  (let ((g-captured-messages (cl-gensym)))
+    `(kmacro-tests-with-message-capture ,g-captured-messages
+       ,@body
+       (should (string-match-p ,value ,g-captured-messages)))))
+
+;;; Tests:
+
+(kmacro-tests-deftest kmacro-tests-test-insert-counter-01-nil ()
+  "`kmacro-insert-counter' adds one to macro counter with nil arg."
+  (kmacro-tests-should-insert "0"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter nil)))
+  (kmacro-tests-should-insert "1"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter nil))))
+
+(kmacro-tests-deftest kmacro-tests-test-insert-counter-02-int ()
+  "`kmacro-insert-counter' increments by value of list argument."
+  (kmacro-tests-should-insert "0"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter 2)))
+  (kmacro-tests-should-insert "2"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter 3)))
+  (kmacro-tests-should-insert "5"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter nil))))
+
+(kmacro-tests-deftest kmacro-tests-test-insert-counter-03-list ()
+  "`kmacro-insert-counter' doesn't increment when given universal argument."
+  (kmacro-tests-should-insert "0"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter (16))))
+  (kmacro-tests-should-insert "0"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter (4)))))
+
+(kmacro-tests-deftest kmacro-tests-test-insert-counter-04-neg ()
+  "`kmacro-insert-counter' decrements with '- prefix argument"
+  (kmacro-tests-should-insert "0"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter -)))
+  (kmacro-tests-should-insert "-1"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter nil))))
+
+(kmacro-tests-deftest kmacro-tests-test-start-format-counter ()
+  "`kmacro-insert-counter' uses start value and format."
+  (kmacro-tests-simulate-command '(kmacro-set-counter 10))
+  (kmacro-tests-should-insert "10"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter nil)))
+  (kmacro-tests-should-insert "11"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter nil)))
+  (kmacro-set-format "c=%s")
+  (kmacro-tests-simulate-command '(kmacro-set-counter 50))
+  (kmacro-tests-should-insert "c=50"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter nil))))
+
+(kmacro-tests-deftest kmacro-tests-test-start-macro-when-defining-macro ()
+  "Starting a macro while defining a macro does not start a second macro."
+  (kmacro-tests-simulate-command '(kmacro-start-macro nil))
+  ;; We should now be in the macro-recording state.
+  (should defining-kbd-macro)
+  (should-not last-kbd-macro)
+  ;; Calling it again should leave us in the same state.
+  (kmacro-tests-simulate-command '(kmacro-start-macro nil))
+  (should defining-kbd-macro)
+  (should-not last-kbd-macro))
+
+
+(kmacro-tests-deftest kmacro-tests-set-macro-counter-while-defining ()
+  "Use of the prefix arg with kmacro-start sets kmacro-counter."
+  ;; Give kmacro-start-macro an argument.
+  (kmacro-tests-simulate-command '(kmacro-start-macro 5))
+  (should defining-kbd-macro)
+  ;; Verify that the counter is set to that value.
+  (kmacro-tests-should-insert "5"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter nil)))
+  ;; Change it while defining a macro.
+  (kmacro-tests-simulate-command '(kmacro-set-counter 1))
+  (kmacro-tests-should-insert "1"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter nil)))
+  ;; Using universal arg to to set counter should reset to starting value.
+  (kmacro-tests-simulate-command '(kmacro-set-counter (4)) '(4))
+  (kmacro-tests-should-insert "5"
+    (kmacro-tests-simulate-command '(kmacro-insert-counter nil))))
+
+
+(kmacro-tests-deftest kmacro-tests-start-insert-counter-appends-to-macro ()
+  "Use of the universal arg appends to the previous macro."
+  (let ((kmacro-tests-macros (list (string-to-vector "hello"))))
+    ;; Start recording a macro.
+    (kmacro-tests-simulate-command '(kmacro-start-macro-or-insert-counter nil))
+    ;; Make sure we are recording.
+    (should defining-kbd-macro)
+    ;; Call it again and it should insert the counter.
+    (kmacro-tests-should-insert "0"
+      (kmacro-tests-simulate-command '(kmacro-start-macro-or-insert-counter nil)))
+    ;; We should still be in the recording state.
+    (should defining-kbd-macro)
+    ;; End recording with repeat count.
+    (kmacro-tests-simulate-command '(kmacro-end-or-call-macro 3))
+    ;; Recording should be finished.
+    (should-not defining-kbd-macro)
+    ;; Now use prefix arg to append to the previous macro.
+    ;; This should run the previous macro first.
+    (kmacro-tests-should-insert "hello"
+      (kmacro-tests-simulate-command
+       '(kmacro-start-macro-or-insert-counter (4))))
+    ;;  Verify that the recording state has changed.
+    (should (equal defining-kbd-macro 'append))))
+
+(kmacro-tests-deftest kmacro-tests-end-call-macro-prefix-args ()
+  "kmacro-end-call-macro changes behavior based on prefix arg."
+  ;; "Record" two macros.
+  (dotimes (i 2)
+    (kmacro-tests-define-macro (vconcat (format "macro #%d" (1+  i)))))
+  ;; With no prefix arg, it should call the second macro.
+  (kmacro-tests-should-insert "macro #2"
+    (kmacro-tests-simulate-command '(kmacro-end-or-call-macro nil)))
+  ;; With universal arg, it should call the first one.
+  (kmacro-tests-should-insert "macro #1"
+    (kmacro-tests-simulate-command '(kmacro-end-or-call-macro (4)))))
+
+(kmacro-tests-deftest kmacro-tests-end-and-call-macro ()
+  "Keyboard command to end and call macro works under various conditions."
+  ;; First, try it with no macro to record.
+  (setq kmacro-tests-macros '(""))
+  (kmacro-tests-simulate-command '(kmacro-start-macro nil))
+  (condition-case err
+      (kmacro-tests-simulate-command '(kmacro-end-and-call-macro 2) 2)
+    (error (should (string= (cadr err)
+                            "No kbd macro has been defined"))))
+
+  ;; Check that it stopped defining and that no macro was recorded.
+  (should-not defining-kbd-macro)
+  (should-not last-kbd-macro)
+
+  ;; Now try it while not recording, but first record a non-nil macro.
+  (kmacro-tests-define-macro "macro")
+  (kmacro-tests-should-insert "macro"
+    (kmacro-tests-simulate-command '(kmacro-end-and-call-macro nil))))
+
+(kmacro-tests-deftest kmacro-tests-end-and-call-macro-mouse ()
+  "Commands to end and call macro work under various conditions.
+This is a regression test for Bug#24992."
+  (:expected-result :failed)
+  (cl-letf (((symbol-function #'mouse-set-point) #'ignore))
+    ;; First, try it with no macro to record.
+    (setq kmacro-tests-macros '(""))
+    (kmacro-tests-simulate-command '(kmacro-start-macro nil))
+    (condition-case err
+        (kmacro-tests-simulate-command '(kmacro-end-call-mouse 2) 2)
+      (error (should (string= (cadr err)
+                              "No kbd macro has been defined"))))
+
+    ;; Check that it stopped defining and that no macro was recorded.
+    (should-not defining-kbd-macro)
+    (should-not last-kbd-macro)
+
+    ;; Now try it while not recording, but first record a non-nil macro.
+    (kmacro-tests-define-macro "macro")
+    (kmacro-tests-should-insert "macro"
+      (kmacro-tests-simulate-command '(kmacro-end-call-mouse nil)))))
+
+(kmacro-tests-deftest kmacro-tests-call-macro-hint-and-repeat ()
+  "`kmacro-call-macro' gives hint in Messages and sets up repeat keymap.
+This is a regression test for: Bug#3412, Bug#11817."
+  (kmacro-tests-define-macro [?m])
+  (let ((kmacro-call-repeat-key t)
+        (kmacro-call-repeat-with-arg t)
+        (overriding-terminal-local-map overriding-terminal-local-map)
+        (last-input-event ?e))
+    (message "")  ; Clear the echo area. (Bug#3412)
+    (kmacro-tests-should-match-message "Type e to repeat macro"
+      (kmacro-tests-should-insert "mmmmmm"
+        (cl-letf (((symbol-function #'this-single-command-keys) (lambda ()
+                                                                  [?\C-x ?e])))
+          (kmacro-call-macro 3))
+        ;; Check that it set up for repeat, and run the repeat.
+        (funcall (lookup-key overriding-terminal-local-map "e"))))))
+
+(kmacro-tests-deftest
+    kmacro-tests-run-macro-command-recorded-in-macro ()
+  "No infinite loop if `kmacro-end-and-call-macro' is recorded in the macro.
+\(Bug#15126)"
+  (:expected-result :failed)
+  (ert-skip "Skipping due to Bug#24921 (an ERT bug)")
+  (kmacro-tests-define-macro (vconcat "foo" [return] "\M-x"
+                                      "kmacro-end-and-call-macro"))
+  (use-local-map kmacro-tests-keymap)
+  (kmacro-tests-simulate-command '(kmacro-end-and-call-macro nil)))
+
+
+(kmacro-tests-deftest kmacro-tests-test-ring-2nd-commands ()
+  "2nd macro in ring is displayed and executed normally and on repeat."
+  (use-local-map kmacro-tests-keymap)
+  ;; Record one macro, with count.
+  (push (vconcat "\C-cxi" "\C-u\C-cxi") kmacro-tests-macros)
+  (kmacro-tests-simulate-command '(kmacro-start-macro 1))
+  (kmacro-tests-simulate-command '(kmacro-end-macro nil))
+  ;; Check that execute and display do nothing with no 2nd macro.
+  (kmacro-tests-should-insert ""
+    (kmacro-tests-simulate-command '(kmacro-call-ring-2nd nil)))
+  (kmacro-tests-should-match-message "Only one keyboard macro defined"
+    (kmacro-tests-simulate-command '(kmacro-view-ring-2nd)))
+  ;; Record another one, with format.
+  (kmacro-set-format "=%d=")
+  (kmacro-tests-define-macro (vconcat "bar"))
+  ;; Execute the first one, mocked up to insert counter.
+  ;; Should get default format.
+  (kmacro-tests-should-insert "11"
+    (kmacro-tests-simulate-command '(kmacro-call-ring-2nd nil)))
+  ;; Now display the 2nd ring macro and check result.
+  (kmacro-tests-should-match-message "C-c x i C-u C-c x i"
+    (kmacro-view-ring-2nd)))
+
+(kmacro-tests-deftest kmacro-tests-fill-ring-and-rotate ()
+  "Macro ring can shift one way, shift the other way, swap and pop."
+  (cl-letf ((kmacro-ring-max 4))
+    ;; Record enough macros that the first one drops off the history.
+    (dotimes (n (1+ kmacro-ring-max))
+      (kmacro-tests-define-macro (make-vector (1+ n) (+ ?a n))))
+    ;; Cycle the ring and check that #2 comes up.
+    (kmacro-tests-should-match-message "2*b"
+      (kmacro-tests-simulate-command '(kmacro-cycle-ring-next nil)))
+    ;; Execute the current macro and check arguments.
+    (kmacro-tests-should-insert "bbbb"
+      (kmacro-call-macro 2 t))
+    ;; Cycle the ring the other way; #5 expected.
+    (kmacro-tests-should-match-message "5*e" (kmacro-cycle-ring-previous nil))
+    ;; Swapping the top two should give #4.
+    (kmacro-tests-should-match-message "4*d" (kmacro-swap-ring))
+    ;; Delete the top and expect #5.
+    (kmacro-tests-should-match-message "5*e" (kmacro-delete-ring-head))))
+
+
+(kmacro-tests-deftest kmacro-tests-test-ring-commands-when-no-macros ()
+  "Ring commands give appropriate message when no macros exist."
+  (dolist (cmd '((kmacro-cycle-ring-next nil)
+                 (kmacro-cycle-ring-previous nil)
+                 (kmacro-swap-ring)
+                 (kmacro-delete-ring-head)
+                 (kmacro-view-ring-2nd)
+                 (kmacro-call-ring-2nd nil)
+                 (kmacro-view-macro)))
+    (kmacro-tests-should-match-message "No keyboard macro defined"
+      (kmacro-tests-simulate-command cmd))))
+
+(kmacro-tests-deftest kmacro-tests-repeat-on-last-key ()
+  "Kmacro commands can be run in sequence without prefix keys."
+  (let* ((prefix (where-is-internal 'kmacro-keymap nil t))
+         ;; Make a sequence of events to run.
+         ;; Comments are expected output of mock macros
+         ;; on the first and second run of the sequence (see below).
+         (events (mapcar #'kmacro-tests-get-kmacro-key
+                         '(kmacro-end-or-call-macro-repeat ;c / b
+                           kmacro-end-or-call-macro-repeat ;c / b
+                           kmacro-call-ring-2nd-repeat     ;b / a
+                           kmacro-cycle-ring-next
+                           kmacro-end-or-call-macro-repeat ;a / a
+                           kmacro-cycle-ring-previous
+                           kmacro-end-or-call-macro-repeat ;c / b
+                           kmacro-delete-ring-head
+                           kmacro-end-or-call-macro-repeat ;b / a
+                           )))
+         (kmacro-tests-macros (list [?a] [?b] [?c]))
+         ;; What we want kmacro to see as keyboard command sequence
+         (first-event (seq-concatenate
+                       'vector
+                       prefix
+                       (vector (kmacro-tests-get-kmacro-key
+                                'kmacro-end-or-call-macro-repeat)))))
+    (cl-letf
+        ;; standardize repeat options
+        ((kmacro-repeat-no-prefix t)
+         (kmacro-call-repeat-key t)
+         (kmacro-call-repeat-with-arg nil))
+      ;; "Record" two macros
+      (dotimes (_n 2)
+        (kmacro-tests-simulate-command '(kmacro-start-macro nil))
+        (kmacro-tests-simulate-command '(kmacro-end-macro nil)))
+      ;; Start recording #3
+      (kmacro-tests-simulate-command '(kmacro-start-macro nil))
+
+      ;; Set up pending keyboard events and a fresh buffer
+      ;; kmacro-set-counter is not one of the repeating kmacro
+      ;; commands so it should end the sequence.
+      (let* ((end-key (kmacro-tests-get-kmacro-key 'kmacro-set-counter))
+             (kmacro-tests-events (append events (list end-key))))
+        (cl-letf (((symbol-function #'this-single-command-keys)
+                   (lambda () first-event)))
+          (use-local-map kmacro-tests-keymap)
+          (kmacro-tests-should-insert "ccbacb"
+            ;; End #3 and launch loop to read events.
+            (kmacro-end-or-call-macro-repeat nil))))
+
+      ;; `kmacro-edit-macro-repeat' should also stop the sequence,
+      ;; so run it again with that at the end.
+      (let* ((end-key (kmacro-tests-get-kmacro-key 'kmacro-edit-macro-repeat))
+             (kmacro-tests-events (append events (list end-key))))
+        (cl-letf (((symbol-function #'edit-kbd-macro) #'ignore)
+                  ((symbol-function #'this-single-command-keys)
+                   (lambda () first-event)))
+          (use-local-map kmacro-tests-keymap)
+          (kmacro-tests-should-insert "bbbbbaaba"
+            (kmacro-end-or-call-macro-repeat 3)))))))
+
+(kmacro-tests-deftest kmacro-tests-repeat-view-and-run ()
+  "Kmacro view cycles through ring and executes macro just viewed."
+  (let* ((prefix (where-is-internal 'kmacro-keymap nil t))
+         (kmacro-tests-events
+          (mapcar #'kmacro-tests-get-kmacro-key
+                  (append (make-list 5 'kmacro-view-macro-repeat)
+                          '(kmacro-end-or-call-macro-repeat
+                            kmacro-set-counter))))
+         ;; Make kmacro see this as keyboard command sequence.
+         (first-event (seq-concatenate
+                       'vector
+                       prefix
+                       (vector (kmacro-tests-get-kmacro-key
+                                'kmacro-view-macro-repeat))))
+         ;; Construct a regexp to match the messages which should be
+         ;; produced by repeated view-repeats.
+         (macros-regexp (apply #'concat
+                               (mapcar (lambda (c) (format ".+%s\n" c))
+                                       '("d" "c" "b" "a" "d" "c")))))
+    (cl-letf ((kmacro-repeat-no-prefix t)
+              (kmacro-call-repeat-key t)
+              (kmacro-call-repeat-with-arg nil)
+              ((symbol-function #'this-single-command-keys) (lambda ()
+                                                              first-event)))
+      ;; "Record" some macros.
+      (dotimes (n 4)
+        (kmacro-tests-define-macro (make-vector 1 (+ ?a n))))
+
+      (use-local-map kmacro-tests-keymap)
+      ;; 6 views (the direct call plus the 5 in events) should
+      ;; cycle through the ring and get to the second-to-last
+      ;; macro defined.
+      (kmacro-tests-should-insert "c"
+        (kmacro-tests-should-match-message macros-regexp
+          (kmacro-tests-simulate-command '(kmacro-view-macro-repeat nil)))))))
+
+(kmacro-tests-deftest kmacro-tests-bind-to-key-when-recording ()
+  "Bind to key doesn't bind a key during macro recording."
+  (cl-letf ((global-map global-map)
+            (saved-binding (key-binding "\C-a"))
+            (kmacro-tests-sequences (list "\C-a")))
+    (kmacro-tests-simulate-command '(kmacro-start-macro 1))
+    (kmacro-bind-to-key nil)
+    (should (eq saved-binding (key-binding "\C-a")))))
+
+(kmacro-tests-deftest kmacro-tests-name-or-bind-to-key-when-no-macro ()
+  "Bind to key, symbol or register fails when when no macro exists."
+  (should-error (kmacro-bind-to-key nil))
+  (should-error (kmacro-name-last-macro 'kmacro-tests-symbol-for-test))
+  (should-error (kmacro-to-register)))
+
+(kmacro-tests-deftest kmacro-tests-bind-to-key-bad-key-sequence ()
+  "Bind to key fails to bind to ^G."
+  (let ((global-map global-map)
+        (saved-binding (key-binding "\C-g"))
+        (kmacro-tests-sequences (list "\C-g")))
+    (kmacro-tests-define-macro [1])
+    (kmacro-bind-to-key nil)
+    (should (eq saved-binding (key-binding "\C-g")))))
+
+(kmacro-tests-deftest kmacro-tests-bind-to-key-with-key-sequence-in-use ()
+  "Bind to key respects yes-or-no-p when given already bound key sequence."
+  (kmacro-tests-define-macro (vconcat "abaab"))
+  (let ((global-map global-map)
+        (map (make-sparse-keymap))
+        (kmacro-tests-sequences (make-list 2 "\C-hi")))
+    (define-key map "\C-hi" 'info)
+    (use-local-map map)
+    ;; Try the command with yes-or-no-p set up to say no.
+    (cl-letf (((symbol-function #'yes-or-no-p)
+               (lambda (prompt)
+                 (should (string-match-p "info" prompt))
+                 (should (string-match-p "C-h i" prompt))
+                 nil)))
+      (kmacro-bind-to-key nil))
+
+    (should (equal (where-is-internal 'info nil t)
+                   (vconcat "\C-hi")))
+    ;; Try it again with yes.
+    (cl-letf (((symbol-function #' yes-or-no-p)
+               (lambda (_prompt) t)))
+      (kmacro-bind-to-key nil))
+
+    (should-not (equal (where-is-internal 'info global-map t)
+                       (vconcat "\C-hi")))
+    (use-local-map nil)
+    (kmacro-tests-should-insert "abaab"
+      (funcall (key-binding "\C-hi")))))
+
+(kmacro-tests-deftest kmacro-tests-kmacro-bind-to-single-key ()
+  "Bind to key uses C-x C-k A when asked to bind to A."
+  (let ((global-map global-map)
+        (kmacro-tests-macros (list (string-to-vector "\C-cxi"))))
+    (use-local-map kmacro-tests-keymap)
+
+    ;; Record a macro with counter and format set.
+    (kmacro-set-format "<%d>")
+    (kmacro-tests-simulate-command '(kmacro-start-macro-or-insert-counter 5))
+    (kmacro-tests-simulate-command '(kmacro-end-macro nil))
+
+    (let ((kmacro-tests-sequences (list "A")))
+      (kmacro-bind-to-key nil))
+
+    ;; Record a second macro with different counter and format.
+    (kmacro-set-format "%d")
+    (kmacro-tests-define-macro [2])
+
+    ;; Check the bound key and run it and verify correct counter
+    ;; and format.
+    (should (equal (string-to-vector "\C-cxi")
+                   (car (kmacro-extract-lambda
+                         (key-binding "\C-x\C-kA")))))
+    (kmacro-tests-should-insert "<5>"
+      (funcall (key-binding "\C-x\C-kA")))))
+
+(kmacro-tests-deftest kmacro-tests-name-last-macro-unable-to-bind ()
+  "Name last macro won't bind to symbol which is already bound."
+  (kmacro-tests-define-macro [1])
+  ;; Set up a test symbol which looks like a function.
+  (setplist 'kmacro-tests-symbol-for-test nil)
+  (fset 'kmacro-tests-symbol-for-test #'ignore)
+  (should-error (kmacro-name-last-macro 'kmacro-tests-symbol-for-test))
+  ;; The empty string symbol also can't be bound.
+  (should-error (kmacro-name-last-macro (make-symbol ""))))
+
+(kmacro-tests-deftest kmacro-tests-name-last-macro-bind-and-rebind ()
+  "Name last macro can rebind a symbol it binds."
+  ;; Make sure our symbol is unbound.
+  (when (fboundp 'kmacro-tests-symbol-for-test)
+    (fmakunbound 'kmacro-tests-symbol-for-test))
+  (setplist 'kmacro-tests-symbol-for-test nil)
+  ;; Make two macros and bind them to the same symbol.
+  (dotimes (i 2)
+    (kmacro-tests-define-macro (make-vector (1+ i) (+ ?a i)))
+    (kmacro-name-last-macro 'kmacro-tests-symbol-for-test)
+    (should (fboundp 'kmacro-tests-symbol-for-test)))
+
+  ;; Now run the function bound to the symbol. Result should be the
+  ;; second macro.
+  (kmacro-tests-should-insert "bb"
+    (kmacro-tests-simulate-command '(kmacro-tests-symbol-for-test))))
+
+(kmacro-tests-deftest kmacro-tests-store-in-register ()
+  "Macro can be stored in and retrieved from a register."
+  (use-local-map kmacro-tests-keymap)
+  ;; Save and restore register 200 so we can use it for the test.
+  (let ((saved-reg-contents (get-register 200)))
+    (unwind-protect
+        (progn
+          ;; Define a macro, and save it to a register.
+          (kmacro-tests-define-macro (vconcat "a\C-a\C-cxu"))
+          (kmacro-to-register 200)
+          ;; Then make a new different macro.
+          (kmacro-tests-define-macro (vconcat "bb\C-a\C-cxu"))
+          ;; When called from the register, result should be first macro.
+          (kmacro-tests-should-insert "AAA"
+            (kmacro-tests-simulate-command '(jump-to-register 200 3) 3))
+          (kmacro-tests-should-insert "a C-a C-c x u"
+            (kmacro-tests-simulate-command '(insert-register 200 t) '(4))))
+      (set-register 200 saved-reg-contents))))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-act ()
+  "Step-edit steps-through a macro with act and act-repeat."
+  (kmacro-tests-run-step-edit "he\C-u2lo"
+                              :events (make-list 6 'act)
+                              :result "hello"
+                              :macro-result "he\C-u2lo")
+
+  (kmacro-tests-run-step-edit "f\C-aoo\C-abar"
+                              :events (make-list 5 'act-repeat)
+                              :result "baroof"
+                              :macro-result "f\C-aoo\C-abar"))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-skip ()
+  "Step-editing can skip parts of macro."
+  (kmacro-tests-run-step-edit "ofoofff"
+                              :events '(skip skip-keep skip-keep skip-keep
+                                             skip-rest)
+                              :result ""
+                              :macro-result "foo"))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-quit ()
+  "Quit while step-editing leaves macro unchanged."
+  (kmacro-tests-run-step-edit "bar"
+                              :events '(help insert skip help quit)
+                              :sequences '("f" "o" "o" "\C-j")
+                              :result "foo"
+                              :macro-result "bar"))
+
+(kmacro-tests-deftest kmacro-tests-step-insert ()
+  "Step edit can insert in macro."
+  (kmacro-tests-run-step-edit "fbazbop"
+                              :events '(insert act insert-1 act-repeat)
+                              :sequences '("o" "o" "\C-a" "\C-j" "\C-e")
+                              :result "foobazbop"
+                              :macro-result "oo\C-af\C-ebazbop"))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-replace-digit-argument ()
+  "Step-edit replace can replace a numeric argument in a macro.
+This is a regression for item 1 in Bug#24991."
+  (:expected-result :failed)
+  (kmacro-tests-run-step-edit "\C-u3b\C-a\C-cxu"
+                              :events '(act replace automatic)
+                              :sequences '("8" "x" "\C-j")
+                              :result "XXXXXXXX"
+                              :macro-result "\C-u8x\C-a\C-cxu"))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-replace ()
+  "Step-edit replace and replace-1 can replace parts of a macro."
+  (kmacro-tests-run-step-edit "a\C-a\C-cxu"
+                              :events '(act act replace)
+                              :sequences '("b" "c" "\C-j")
+                              :result "bca"
+                              :macro-result "a\C-abc")
+  (kmacro-tests-run-step-edit "a\C-a\C-cxucd"
+                              :events '(act replace-1 automatic)
+                              :sequences '("b")
+                              :result "abcd"
+                              :macro-result "ab\C-cxucd")
+  (kmacro-tests-run-step-edit "by"
+                              :events '(act replace)
+                              :sequences '("a" "r" "\C-j")
+                              :result "bar"
+                              :macro-result "bar"))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-append ()
+  "Step edit append inserts after point, and append-end inserts at end."
+  (kmacro-tests-run-step-edit "f-b"
+                              :events '(append append-end)
+                              :sequences '("o" "o" "\C-j" "a" "r" "\C-j")
+                              :result "foo-bar"
+                              :macro-result "foo-bar")
+  (kmacro-tests-run-step-edit "x"
+                              :events '(append)
+                              :sequences '("\C-a" "\C-cxu" "\C-e" "y" "\C-j")
+                              :result "Xy"
+                              :macro-result "x\C-a\C-cxu\C-ey"))
+
+(kmacro-tests-deftest kmacro-tests-append-end-at-end-appends ()
+  "Append-end when already at end of macro appends to end of macro.
+This is a regression for item 2 in Bug#24991."
+  (:expected-result :failed)
+  (kmacro-tests-run-step-edit "x"
+                              :events '(append-end)
+                              :sequences '("\C-a" "\C-cxu" "\C-e" "y" "\C-j")
+                              :result "Xy"
+                              :macro-result "x\C-a\C-cxu\C-ey"))
+
+
+(kmacro-tests-deftest kmacro-tests-step-edit-skip-entire ()
+  "Skipping a whole macro in step-edit leaves macro unchanged.
+This is a regression for item 3 in Bug#24991."
+  (:expected-result :failed)
+  (kmacro-tests-run-step-edit "xyzzy"
+                              :events '(skip-rest)
+                              :result ""
+                              :macro-result "xyzzy"))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-step-through-negative-argument ()
+  "Step edit works on macros using negative universal argument.
+This is a regression for item 4 in Bug#24991."
+  (:expected-result :failed)
+  (kmacro-tests-run-step-edit "boo\C-u-\C-cu"
+                              :events '(act-repeat automatic)
+                              :result "BOO"
+                              :macro-result "boo\C-u-\C-cd"))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-with-quoted-insert ()
+  "Stepping through a macro that uses quoted insert leaves macro unchanged.
+This is a regression for item 5 in Bug#24991."
+  (:expected-result :failed)
+  (let ((read-quoted-char-radix 8))
+    (kmacro-tests-run-step-edit "\C-cxq17051i there"
+                                :events '(act automatic)
+                                :result "ḩi there"
+                                :macro-result "\C-cxq17051i there")
+    (kmacro-tests-run-step-edit "g\C-cxq17051i"
+                                :events '(act insert-1 automatic)
+                                :sequences '("-")
+                                :result "g-ḩi"
+                                :macro-result "g-\C-cxq17051i")))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-can-replace-meta-keys ()
+  "Replacing C-w with M-w produces the expected result.
+This is a regression for item 7 in Bug#24991."
+  (:expected-result :failed)
+  (kmacro-tests-run-step-edit "abc\C-b\C-b\C-SPC\C-f\C-w\C-e\C-y"
+                              :events '(act-repeat act-repeat
+                                                   act-repeat act-repeat
+                                                   replace automatic)
+                              :sequences '("\M-w" "\C-j")
+                              :result "abcb"
+                              :macro-result "abc\C-b\C-b\C-SPC\C-f\M-w\C-e\C-y")
+  (kmacro-tests-should-insert "abcb" (kmacro-call-macro nil)))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-ignores-qr-map-commands ()
+  "Unimplemented commands from `query-replace-map' are ignored."
+  (kmacro-tests-run-step-edit "yep"
+                              :events '(edit-replacement
+                                        act-and-show act-and-exit
+                                        delete-and-edit
+                                        recenter backup
+                                        scroll-up scroll-down
+                                        scroll-other-window
+                                        scroll-other-window-down
+                                        exit-prefix
+                                        act act act)
+                              :result "yep"
+                              :macro-result "yep"))
+
+(kmacro-tests-deftest
+    kmacro-tests-step-edit-edits-macro-with-extended-command ()
+  "Step-editing a macro which uses the minibuffer can change the macro."
+  (let ((mac (vconcat [?\M-x] "eval-expression" '[return]
+                      "(insert-char (+ ?a \C-e" [?1] "))" '[return]))
+        (mac-after (vconcat [?\M-x] "eval-expression" '[return]
+                            "(insert-char (+ ?a \C-e" [?2] "))" '[return])))
+
+    (kmacro-tests-run-step-edit mac
+                                :events '(act act-repeat
+                                              act act-repeat act
+                                              replace-1 act-repeat act)
+                                :sequences '("2")
+                                :result "c"
+                                :macro-result mac-after)))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-step-through-isearch ()
+  "Step-editing can edit a macro which uses `isearch-backward' (Bug#22488)."
+  (:expected-result :failed)
+  (let ((mac (vconcat "test Input" '[return]
+                      [?\C-r] "inp" '[return] "\C-cxu"))
+        (mac-after (vconcat "test input" '[return]
+                            [?\C-r] "inp" '[return] "\C-cd")))
+
+    (kmacro-tests-run-step-edit mac
+                                :events '(act-repeat act act
+                                                     act-repeat act
+                                                     replace-1)
+                                :sequences '("\C-cd")
+                                :result "test input\n"
+                                :macro-result mac-after)))
+
+(kmacro-tests-deftest kmacro-tests-step-edit-cleans-up-hook ()
+  "Step-editing properly cleans up `post-command-hook.' (Bug #18708)"
+  (:expected-result :failed)
+  (let (post-command-hook)
+    (setq-local post-command-hook '(t))
+    (kmacro-tests-run-step-edit "x"
+                                :events '(act)
+                                :result "x"
+                                :macro-result "x")
+    (kmacro-tests-simulate-command '(beginning-of-line))))
+
+(cl-defun kmacro-tests-run-step-edit
+    (macro &key events sequences result macro-result)
+  "Set up and run a test of `kmacro-step-edit-macro'.
+
+Run `kmacro-step-edit-macro' with MACRO defined as a keyboard macro
+and `read-event' and `read-key-sequence' set up to return items from
+EVENTS and SEQUENCES respectively.  SEQUENCES may be nil, but
+EVENTS should not be.  EVENTS should be a list of symbols bound
+in `kmacro-step-edit-map' or `query-replace' map, and this function
+will do the keymap lookup for you. SEQUENCES should contain
+return values for `read-key-sequence'.
+
+Before running the macro, the current buffer will be erased.
+RESULT is the string that should be inserted during the
+step-editing process, and MACRO-RESULT is the expected value of
+`last-kbd-macro' after the editing is complete."
+
+  (let* ((kmacro-tests-events (mapcar #'kmacro-tests-get-kmacro-step-edit-key events))
+         (kmacro-tests-sequences sequences))
+
+    (kmacro-tests-define-macro (string-to-vector macro))
+    (use-local-map kmacro-tests-keymap)
+    (erase-buffer)
+    (kmacro-step-edit-macro)
+    (when result
+      (should (equal result (buffer-string))))
+    (when macro-result
+      (should (equal last-kbd-macro (string-to-vector macro-result))))))
+
+;;; Utilities:
+
+(defun kmacro-tests-simulate-command (command &optional arg)
+  "Call `ert-simulate-command' after setting `current-prefix-arg'.
+Sets `current-prefix-arg' to ARG if it is non-nil, otherwise to
+the second element of COMMAND, before executing COMMAND using
+`ert-simulate-command'."
+  (let ((current-prefix-arg (or arg (cadr command))))
+    (ert-simulate-command command)))
+
+(defun kmacro-tests-define-macro (mac)
+  "Define MAC as a keyboard macro using kmacro commands."
+  (push mac kmacro-tests-macros)
+  (kmacro-tests-simulate-command '(kmacro-start-macro nil))
+  (should defining-kbd-macro)
+  (kmacro-tests-simulate-command '(kmacro-end-macro nil))
+  (should (equal mac last-kbd-macro)))
+
+(defun kmacro-tests-get-kmacro-key (sym)
+  "Look up kmacro command SYM in kmacro's keymap.
+Return the integer key value found."
+  (aref (where-is-internal sym kmacro-keymap t) 0))
+
+(defun kmacro-tests-get-kmacro-step-edit-key (sym)
+  "Return the first key bound to SYM in `kmacro-step-edit-map'."
+  (let ((where (aref (where-is-internal sym kmacro-step-edit-map t) 0)))
+    (if (consp where)
+        (car where)
+      where)))
+
+(provide 'kmacro-tests)
+
+;;; kmacro-tests.el ends here
-- 
2.10.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#24939: [PATCH] Add tests for lisp/kmacro.el
  2016-12-31 17:42   ` Gemini Lasswell
@ 2017-02-04 11:57     ` Eli Zaretskii
  0 siblings, 0 replies; 11+ messages in thread
From: Eli Zaretskii @ 2017-02-04 11:57 UTC (permalink / raw)
  To: Gemini Lasswell; +Cc: 24939-done

> From: Gemini Lasswell <gazally@runbox.com>
> Cc: 24939@debbugs.gnu.org
> Date: Sat, 31 Dec 2016 09:42:25 -0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > Allow me a few comments to your proposed patch.
> 
> Here is an updated version taking your comments into account. I was able
> to remove the use of advice on all the functions in macros.c except for
> end-kbd-macro, because I couldn't come up with another way to make
> kmacro-end-macro recognize a keyboard macro defined by a test.
> kmacro-end-macro tests last-kbd-macro right after it calls
> end-kbd-macro, and end-kbd-macro is going to leave last-kbd-macro empty
> unless it has something in current_kboard->kbd_macro_buffer, which Lisp
> can't access. If there is another strategy you'd like me to try there,
> let me know.

Thanks, I pushed this now.

> This patch contains kmacro-tests-with-message-capture which is the same
> as the macro proposed as an addition to ert-x.el in bug#25158, so if
> that patch is adopted, it could be removed from this patch.

I've replaced the macro with ert-with-message-capture.





^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-02-04 11:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-13 21:23 bug#24939: [PATCH] Add tests for lisp/kmacro.el Gemini Lasswell
2016-11-14 15:49 ` Eli Zaretskii
2016-11-14 18:26   ` Gemini Lasswell
2016-11-14 18:47     ` Eli Zaretskii
2016-11-29 20:56       ` Gemini Lasswell
2016-11-30  9:08         ` Michael Albinus
2016-11-30 15:51           ` Eli Zaretskii
2016-11-30 16:51             ` Michael Albinus
2016-12-10 17:46             ` Gemini Lasswell
2016-12-31 17:42   ` Gemini Lasswell
2017-02-04 11:57     ` 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).