bug#24939: [PATCH] Add tests for lisp/kmacro.el

[Eli Zaretskii]

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