jorgenschaefer / emacs-buttercup

Behavior-Driven Emacs Lisp Testing
GNU General Public License v3.0
362 stars 44 forks source link

Too strict argument check in `buttercup--enclosed-expr` #226

Closed hillu closed 1 year ago

hillu commented 1 year ago

In Debian bug 1022352 we can see that the test suite of lua-mode has started failing with buttercup 1.26. It used to run fine with buttercup 1.24.

It seems to me that the expect macro has become stricter about the values it accepts, resulting in error messages such as

Test indent-new-comment-line works when the comment is empty
error: (cl-assertion-failed ((cl-every #'buttercup--wrapper-fun-p (cons arg args)) nil ((lambda nil '(lua-buffer-strs (lua-insert-goto-<> '("-- <>")) (execute-kbd-macro (kbd "M-j"))) (buttercup--mark-stackframe) (butlast (split-string (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (rename-buffer "temp-buffer.lua" t) (let (noninteractive) (lua-mode) (font-lock-mode 1)) (set (make-local-variable 'lua-process) nil) (set (make-local-variable 'lua-process-buffer) nil) (pop-to-buffer (current-buffer)) (unwind-protect (progn (progn (lua-insert-goto-<> '("-- <>")) (execute-kbd-macro (kbd "M-j"))) (buffer-substring-no-properties (point-min) (point-max))) (if (buffer-live-p lua-process-buffer) (progn (lua-kill-process))))) (and (buffer-name temp-buffer) (kill-buffer temp-buffer))))) "\n" nil))) (lambda nil ''("--" "--") (buttercup--mark-stackframe) '("--" "--")))))

I have rewritten those tests to use let expressions, for example by changing …

  (it "works when the comment is empty"
    (expect (lua-buffer-strs
             (lua-insert-goto-<> '("-- <>"))
             (execute-kbd-macro (kbd "M-j")))
            :to-equal
            '("--"
              "--"))
    ;; ...

… to …

  (it "works when the comment is empty"
      (let ((result (lua-buffer-strs
             (lua-insert-goto-<> '("-- <>"))
             (execute-kbd-macro (kbd "M-j")))))
        (expect result
            :to-equal
            '("--"
              "--")))
      ;; ...

Was this change intended? Does my workaround seem sensible or is there maybe a simpler way to fix the breakage?

snogge commented 1 year ago

This is a bug in buttercup, and I have a fix coming.

snogge commented 1 year ago

I have no idea why some uses produce closures and some produce lambda functions. Maybe something with how macros are nested? But, I just noticed that lua-mode/test/test-generic.el does not have lexical-binding set. This is a documented requirement for buttercup. Maybe that's the reason for the discrepancy.

hillu commented 1 year ago

Thank you. I'll try if changing that fixes the errors.

hillu commented 1 year ago

I can report that updating to buttercup 1.28 fixed the problem I reported. Also, adding lexical-binding:t made the otherwise unmodified test suite work with buttercup 1.26. Perhaps it would be a good idea to add a check for lexical-binding to the code run by expect?

Thank you for your help!

snogge commented 1 year ago

Note that the fix in https://github.com/jorgenschaefer/emacs-buttercup/commit/ae884f10ad592ea4b832ea9c7541914d8ecfaf38 means the lua-mode tests pass now.