[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#24939: [PATCH] Add tests for lisp/kmacro.el
From: |
Gemini Lasswell |
Subject: |
bug#24939: [PATCH] Add tests for lisp/kmacro.el |
Date: |
Mon, 14 Nov 2016 10:26:42 -0800 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (darwin) |
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.
- bug#24939: [PATCH] Add tests for lisp/kmacro.el, Gemini Lasswell, 2016/11/13
- bug#24939: [PATCH] Add tests for lisp/kmacro.el, Eli Zaretskii, 2016/11/14
- bug#24939: [PATCH] Add tests for lisp/kmacro.el,
Gemini Lasswell <=
- bug#24939: [PATCH] Add tests for lisp/kmacro.el, Eli Zaretskii, 2016/11/14
- bug#24939: [PATCH] Add tests for lisp/kmacro.el, Gemini Lasswell, 2016/11/29
- bug#24939: [PATCH] Add tests for lisp/kmacro.el, Michael Albinus, 2016/11/30
- bug#24939: [PATCH] Add tests for lisp/kmacro.el, Eli Zaretskii, 2016/11/30
- bug#24939: [PATCH] Add tests for lisp/kmacro.el, Michael Albinus, 2016/11/30