emacs-eldev / eldev

Elisp development tool
https://emacs-eldev.github.io/eldev/
GNU General Public License v3.0
226 stars 17 forks source link

potential Eldev buttercup framework invocation issue with latest buttercup 1.34 #100

Closed ikappaki closed 5 months ago

ikappaki commented 5 months ago

Describe the bug

The Eldev buttercup invocation framework appears to be broken with the latest buttercup release on melpa (1.34 at the time of this writing)

Steps to reproduce

  1. Pull latest eldev from master (be713ddddcaa7a569494ea5d7e43c9681a2a8aea at the time of this writing)
  2. Set ELDEV_LOCAL env variable to path of Eldev source code
    PS C:\src\eldev> $env:ELDEV_LOCAL="c:\src\eldev"
  3. Run the project-e integration tests which are about testing buttercup integration
    
    PS C:\src\eldev> eldev.bat test --omit-backtraces --test-type integration "project-e"
    Eldev tests are fairly slow, since they spawn lots of child processes
    Running 4 tests (2024-03-03 19:17:48+0000, selector `"project-e"')
    Ran Eldev as `c:/src/eldev/bin/eldev '--robust-mode=never' test' in directory `c:/src/eldev/test/project-e/'
    Stdout contents:
    Running 2 specs.

Project E tests has a dummy passing test FAILED (0.06ms) `eldev--buttercup-do-fail' to be nil FAILED (0.05ms)

======================================== Project E tests has a dummy passing test

Traceback (most recent call last): cl--assertion-failed((eq 'closure (car-safe oclosure))) error: (cl-assertion-failed ((eq 'closure (car-safe oclosure)) nil))

======================================== Project E tests `eldev--buttercup-do-fail' to be nil

Traceback (most recent call last): cl--assertion-failed((eq 'closure (car-safe oclosure))) error: (cl-assertion-failed ((eq 'closure (car-safe oclosure)) nil))

Ran 2 specs, 2 failed, in 3.30ms.


**Expected behavior**

The tests should pass

**Actual behavior**

The tests fail

``` powershell
PS C:\src\eldev> eldev.bat test --omit-backtraces --test-type integration "project-e"
Eldev tests are fairly slow, since they spawn lots of child processes
Running 4 tests (2024-03-03 19:17:48+0000, selector `"project-e"')
Ran Eldev as `c:/src/eldev/bin/eldev '--robust-mode=never' test' in directory `c:/src/eldev/test/project-e/'
Stdout contents:
Running 2 specs.

Project E tests
  has a dummy passing test  FAILED (0.06ms)
  `eldev--buttercup-do-fail' to be nil  FAILED (0.05ms)

========================================
Project E tests has a dummy passing test

Traceback (most recent call last):
  cl--assertion-failed((eq 'closure (car-safe oclosure)))
error: (cl-assertion-failed ((eq 'closure (car-safe oclosure)) nil))

========================================
Project E tests `eldev--buttercup-do-fail' to be nil

Traceback (most recent call last):
  cl--assertion-failed((eq 'closure (car-safe oclosure)))
error: (cl-assertion-failed ((eq 'closure (car-safe oclosure)) nil))

Ran 2 specs, 2 failed, in 3.30ms.

Stderr contents:
2 Buttercup tests failed

Process exit code: 1
Test eldev-test-buttercup-project-e-1 backtrace:
    [omitted]
Test eldev-test-buttercup-project-e-1 condition:
    (ert-test-failed
     ((should
       (= exit-code 0))
      :form
      (= 1 0)
      :value nil))
   FAILED  1/4  eldev-test-buttercup-project-e-1 (0.711365 sec) at test/integration/buttercup.el:6
   passed  2/4  eldev-test-buttercup-project-e-2 (0.654108 sec)
Ran Eldev as `c:/src/eldev/bin/eldev '--robust-mode=never' --setup '(defvar eldev--buttercup-do-fail t)' test 'dummy passing'' in directory `c:/src/eldev/test/project-e/'
Stdout contents:
Running 1 out of 2 specs. 
...

Environment

Additional context

The issue seems to be related to a recent buttercup change to support open closures in Emacs 29, that breaks Eldev support for buttercup.

image

The same works fine on Emacs 28

PS C:\src\eldev> eldev.bat emacs --version
GNU Emacs 28.2
Copyright (C) 2022 Free Software Foundation, Inc.
GNU Emacs comes with ABSOLUTELY NO WARRANTY.
You may redistribute copies of GNU Emacs
under the terms of the GNU General Public License.
For more information about these matters, see the file named COPYING.
PS C:\src\eldev> eldev.bat -d test --omit-backtraces --test-type integration "project-e"
Eldev tests are fairly slow, since they spawn lots of child processes
Running 4 tests (2024-03-03 19:26:32+0000, selector `"project-e"')
   passed  1/4  eldev-test-buttercup-project-e-1 (4.236234 sec)
   passed  2/4  eldev-test-buttercup-project-e-2 (0.698990 sec)
   passed  3/4  eldev-test-buttercup-project-e-3 (0.697801 sec)
   passed  4/4  eldev-test-buttercup-project-e-concise (0.719395 sec)

Ran 4 tests, 4 results as expected, 0 unexpected (2024-03-03 19:26:39+0000, 6.861437 sec)

Do you think you could confirm the issue and have a look if so please?

Thanks

doublep commented 5 months ago

I'm not sure, this looks like a bug in Buttercup to me. If I completely exclude Eldev out of the equation, it still fails with the same errors:

~/eldev/test/project-e$ emacs --batch -L ~/git/buttercup -L . -L ../dependency-a --eval "(progn (require 'buttercup) (load \"test/project-e.el\") (buttercup-run t))"
Loading /home/paul/eldev/test/project-e/test/project-e.el (source)...
Running 2 specs.

Project E tests
  has a dummy passing test  FAILED (5.30ms)
  `eldev--buttercup-do-fail' to be nil  FAILED (0.59ms)

========================================
Project E tests has a dummy passing test

Traceback (most recent call last):
  cl--assertion-failed((eq 'closure (car-safe oclosure)))
error: (cl-assertion-failed ((eq 'closure (car-safe oclosure)) nil))

========================================
Project E tests `eldev--buttercup-do-fail' to be nil

Traceback (most recent call last):
  cl--assertion-failed((eq 'closure (car-safe oclosure)))
error: (cl-assertion-failed ((eq 'closure (car-safe oclosure)) nil))

If I check out revision 3c6c888 in ~/git/buttercup, i.e. before all the OClosure stuff, then the tests pass as expected.

@snogge: Do you have an idea of what's going on?

ikappaki commented 5 months ago

Right, thanks for looking into this @doublep

It turns out the buttercup test files need to be set to lexical binding to make this work with the open closures

;;; -*-lexical-binding:t-*-
(require 'project-e)
(require 'buttercup)

;; All tests pass.

(defvar eldev--buttercup-do-fail nil)

(describe "Project E tests"
  (it "has a dummy passing test"
    (expect t :to-be t))

  (it "`eldev--buttercup-do-fail' to be nil"
    ;; Result of this test can be altered from command line.
    (expect eldev--buttercup-do-fail :to-be nil)))
c:/src/eldev/test/project-e $ emacs --batch -L c:/src/emacs-buttercup -L . -L ../dependency-a --eval "(progn (require 'buttercup) (load \"test/project-e.el\") (buttercup-run t))"

Loading c:/src/eldev/test/project-e/test/project-e.el (source)...
Running 2 specs.

Project E tests
  has a dummy passing test (8.13ms)
  `eldev--buttercup-do-fail' to be nil (0.10ms)

Ran 2 specs, 0 failed, in 8.35ms.

I'll open an issue at buttercup ... https://github.com/jorgenschaefer/emacs-buttercup/issues/244

snogge commented 5 months ago

As stated in the docs https://github.com/jorgenschaefer/emacs-buttercup/blob/master/docs/writing-tests.md#its-just-functions lexical-binding is a requirement for buttercup, at least for the tests themselves. But it obviously needs to be made more apparent. I'll follow up in https://github.com/jorgenschaefer/emacs-buttercup/issues/244