bug#65851: 29.1.50; `ert-simulate-command' and non-local exits

bug#65851: 29.1.50; `ert-simulate-command' and non-local exits

[Sebastian Miele]

In GNU Emacs 29.1.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.38, cairo version 1.17.8) of 2023-09-06 built on huette

When a command exits non-locally (e.g., by calling `user-error'), the
command loop seems to still do what `ert-simulate-command' in that case
currently does not (e.g., run `post-command-hook', and set
`last-command').

At the moment, the Emacs sources have less than 40 uses of
`ert-simulate-command'.  None of them seems to be wrapped in anything
like `(should-error …)'.  So this has not become an issue in Emacs
testing so far.

Also, in the tests I am currently writing, I could live without
`ert-simulate-command' handling non-local exits like the command loop
does, because I do not need `ert-simulate-command' in that particular
cases.

I do not really know the details of what the command loop actually does.

But wouldn't something like the following be more "in line" with what
the command loop does?

diff --git a/lisp/emacs-lisp/ert-x.el b/lisp/emacs-lisp/ert-x.el
index 98a017c8a8e..e7c670b27c9 100644
--- a/lisp/emacs-lisp/ert-x.el
+++ b/lisp/emacs-lisp/ert-x.el
@@ -173,28 +173,26 @@ ert-simulate-command
   (cl-assert (listp command) t)
   (cl-assert (commandp (car command)) t)
   (cl-assert (not unread-command-events) t)
-  (let (return-value)
-    ;; For the order of things here see command_loop_1 in keyboard.c.
-    ;;
-    ;; The command loop will reset the command-related variables so
-    ;; there is no reason to let-bind them. They are set here,
-    ;; however, to be able to test several commands in a row and how
-    ;; they affect each other.
-    (setq deactivate-mark nil
-          this-original-command (car command)
-          ;; remap through active keymaps
-          this-command (or (command-remapping this-original-command)
-                           this-original-command))
-    (run-hooks 'pre-command-hook)
-    (setq return-value (apply (car command) (cdr command)))
+  ;; For the order of things here see command_loop_1 in keyboard.c.
+  ;;
+  ;; The command loop will reset the command-related variables so
+  ;; there is no reason to let-bind them. They are set here,
+  ;; however, to be able to test several commands in a row and how
+  ;; they affect each other.
+  (setq deactivate-mark nil
+        this-original-command (car command)
+        ;; remap through active keymaps
+        this-command (or (command-remapping this-original-command)
+                         this-original-command))
+  (run-hooks 'pre-command-hook)
+  (unwind-protect (apply (car command) (cdr command))
     (run-hooks 'post-command-hook)
     (setq real-last-command (car command)
           last-command this-command)
     (when (boundp 'last-repeatable-command)
       (setq last-repeatable-command real-last-command))
     (when (and deactivate-mark transient-mark-mode) (deactivate-mark))
-    (cl-assert (not unread-command-events) t)
-    return-value))
+    (cl-assert (not unread-command-events) t)))
 
 (defmacro ert-simulate-keys (keys &rest body)
   "Execute BODY with KEYS as pseudo-interactive input."