sachac / subed

subed is a subtitle editor for Emacs
177 stars 16 forks source link

Running tests yields "buttercup requires `lexical-binding' to be t" #74

Open rndusr opened 3 days ago

rndusr commented 3 days ago

I'm trying to run the tests with make test, and most of then run fine, but there seems to be something wrong with these three files:

File failed to load correctly: ./tests/test-subed-mpv.el

Traceback (most recent call last):
  load("./tests/test-subed-mpv.el" nil t nil nil)
  load-with-code-conversion("/home/ich/.emacs.d/elpa/subed-1.2.11/tests/test...
  internal-macroexpand-for-load((describe "Starting mpv" (it "passes argumen...
  error("Eager macro-expansion failure: %S" (buttercup-dynamic-binding-error...
FAILED: Eager macro-expansion failure: (buttercup-dynamic-binding-error . "buttercup requires `lexical-binding' to be t")

========================================
File failed to load correctly: ./tests/test-subed-srt.el

Traceback (most recent call last):
  load("./tests/test-subed-srt.el" nil t nil nil)
  load-with-code-conversion("/home/ich/.emacs.d/elpa/subed-1.2.11/tests/test...
  internal-macroexpand-for-load((describe "SRT" (describe "Getting" (describ...
  error("Eager macro-expansion failure: %S" (buttercup-dynamic-binding-error...
FAILED: Eager macro-expansion failure: (buttercup-dynamic-binding-error . "buttercup requires `lexical-binding' to be t")

========================================
File failed to load correctly: ./tests/test-subed-tsv.el

Traceback (most recent call last):
  load("./tests/test-subed-tsv.el" nil t nil nil)
  load-with-code-conversion("/home/ich/.emacs.d/elpa/subed-1.2.11/tests/test...
  internal-macroexpand-for-load((describe "TSV" (describe "Getting" (describ...
  error("Eager macro-expansion failure: %S" (buttercup-dynamic-binding-error...
FAILED: Eager macro-expansion failure: (buttercup-dynamic-binding-error . "buttercup requires `lexical-binding' to be t")

Ran 481 specs, 3 failed, in 208.57ms.

The first line of these files contains the -*- lexical-binding: t; -*- like all the other files that don't have this issue.

This happens with and without undercover and package-lint installed.

rodrigomorales1 commented 2 days ago

The first line of these files contains the -- lexical-binding: t; -- like all the other files that don't have this issue.

@rndusr Those files didn't have the -*- lexical-binding: t; -*- like the other test files.

I am working on a branch related to #68 . I just made a commit to solve the issue that you mentioned. See https://github.com/rodrigomorales1/subed/commit/d03067e580d88c8563eee8f3e42ec75cc93d9757

This issue should be solved when my changes get merged.

rndusr commented 2 days ago

Right. I looked at the subed/ files and not at the tests/ files. My mistake.

But if I add ;; -*- lexical-binding: t; eval: (buttercup-minor-mode) -*- or ;; -*- eval: (buttercup-minor-mode); lexical-binding: t; -*-, nothing changes. I'm still getting the same errors.

rodrigomorales1 commented 1 day ago

@rndusr I'm not getting those errors with the commit I just made in my branch. See proof below.

$ git log -n 1
commit d03067e580d88c8563eee8f3e42ec75cc93d9757 (HEAD -> waveform-long-cue, origin/waveform-long-cue)
Author: Rodrigo Morales <rodrigo@morales.pe>
Date:   Sun Jul 7 00:21:42 2024 -0500

    Set lexical-binding: t in tests/* files
$ make test && echo Exit code: $?
emacs --quick --batch --eval "(progn (setq generated-autoload-file (expand-file-name \"subed-autoloads.el\" \"subed\") backup-inhibited t) \
(update-directory-autoloads \"./subed\"))"
Package autoload is deprecated
  INFO     Scraping files for subed-autoloads.el...
  INFO     Scraping files for subed-autoloads.el...done
mkdir -p coverage
UNDERCOVER_FORCE=true emacs -batch -L . -f package-initialize -f buttercup-run-discover

(... omitted lines ...)

Loading /my-storage/miscellaneous/subed-fix/fork/tests/undercover-init.el (source)...
Loading /my-storage/miscellaneous/subed-fix/fork/tests/undercover-init.el (source)...
Loading /my-storage/miscellaneous/subed-fix/fork/tests/undercover-init.el (source)...
Loading /my-storage/miscellaneous/subed-fix/fork/tests/undercover-init.el (source)...
Loading /my-storage/miscellaneous/subed-fix/fork/tests/undercover-init.el (source)...
Running 700 specs.

ASS
  Getting
    the subtitle start/stop time
      returns the time in milliseconds. (7.57ms)
      returns nil if time can't be found. (0.14ms)
    the subtitle text
      when text is empty
        and at the beginning with a trailing newline. (0.16ms)
    when text is not empty
      and has no linebreaks. (0.11ms)

(... omitted lines ...)

  Font-locking
    recognizes VTT syntax. (0.17ms)
  with cues
    parses properly. (0.35ms)
  conversion
    creates TXT. (0.33ms)
    includes comments in TXT if requested. (0.35ms)
  iterating over subtitles
    forwards
      handles headers. (0.15ms)
    backwards
      handles headers. (0.14ms)
      handles empty lines. (0.13ms)

Ran 700 specs, 0 failed, in 289.17ms.

(... omitted lines ...)

Exit code: 0
rndusr commented 23 hours ago

I can't figure out which branch the commit you linked belongs to. My guess would be it's in the waveform-long-cue branch? I see this branch on the GitHub website, but I can't see it on my computer. I tried git fetch https://github.com/sachac/subed/ waveform-long-cue, which worked, but the waveform-long-cue branch is still invisible.

Not sure if there is something weird going on or it's just me being a git with git.

If I make the changes in your commit manually, which is easy enough, there is no improvement on my side.

Anyway, my main reason for running the tests was to work on #71, but that seems to have been fixed by sachac, so I'm ok with closing this issue if I'm the only one having trouble running the tests.