subed icon indicating copy to clipboard operation
subed copied to clipboard

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

Open rndusr opened this issue 1 year ago • 10 comments

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.

rndusr avatar Jul 05 '24 13:07 rndusr

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.

rodrigo-morales-1 avatar Jul 07 '24 05:07 rodrigo-morales-1

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.

rndusr avatar Jul 07 '24 05:07 rndusr

@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 <[email protected]>
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

rodrigo-morales-1 avatar Jul 07 '24 13:07 rodrigo-morales-1

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.

rndusr avatar Jul 08 '24 08:07 rndusr

Great, does rodrigomorales1's fix make the main branch work for you now?

sachac avatar Jul 11 '24 00:07 sachac

Still the same issue with make test:

$ git clone --depth=1 https://github.com/sachac/subed.git
Cloning into 'subed'...
remote: Enumerating objects: 47, done.
remote: Counting objects: 100% (47/47), done.
remote: Compressing objects: 100% (42/42), done.
remote: Total 47 (delta 9), reused 26 (delta 4), pack-reused 0
Receiving objects: 100% (47/47), 219.17 KiB | 3.91 MiB/s, done.
Resolving deltas: 100% (9/9), done.
$ cd subed
$ make test
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
Loading /etc/emacs/site-start.d/00debian.el (source)...
Loading /etc/emacs/site-start.d/50dictionaries-common.el (source)...
Loading debian-ispell (native compiled elisp)...
Loading /var/cache/dictionaries-common/emacsen-ispell-default.el (source)...
Loading /var/cache/dictionaries-common/emacsen-ispell-dicts.el (source)...
Loading /tmp/subed/tests/undercover-init.el (source)...
Loading /tmp/subed/tests/undercover-init.el (source)...
Loading /tmp/subed/tests/undercover-init.el (source)...
Eager macro-expansion failure: (buttercup-dynamic-binding-error . "buttercup requires `lexical-binding' to be t")
make: *** [Makefile:17: test-coverage] Error 255

If I uninstall package-lint, the other tests run fine, but I'm getting the macro-expansion-failure for test-subed-mpv.el, test-subed-srt.el and test-subed-tsv.el.

test-subed-common.el runs fine even though the first three lines are identical.

$ diff <(head -n4 tests/test-subed-common.el) <(head -n4 tests/test-subed-mpv.el)
4c4
< (require 'subed-srt)
---
> (require 'subed-mpv)

make test-only runs all tests successfully but I'm getting these two errors:

Error: (file-missing "Opening output file" "No such file or directory" "/tmp/coverage/.resultset.json")
Error in kill-emacs-hook (subed-mpv-kill): (error "Process mock client process does not exist")

I'd be happy to help debug this, but as I said, it's fine if this stays broken for me.

rndusr avatar Jul 11 '24 06:07 rndusr

@rndusr Could you execute this one-liner and post the output in a new comment?

$ rm -rf /tmp/subed && cd /tmp && git clone --branch main --depth 1 'https://github.com/sachac/subed' && cd /tmp/subed && make test

I executed that command on my system and the tests run successfully, some warnings were shown and an error was thrown because I uninstalled package-lint (just to see what happens), but the tests still run successfully. See code block below.

$ rm -rf /tmp/subed && cd /tmp && git clone --branch main --depth 1 'https://github.com/sachac/subed' && cd /tmp/subed && make test
Cloning into 'subed'...
remote: Enumerating objects: 47, done.
remote: Counting objects: 100% (47/47), done.
remote: Compressing objects: 100% (42/42), done.
remote: Total 47 (delta 9), reused 26 (delta 4), pack-reused 0
Receiving objects: 100% (47/47), 219.17 KiB | 1.12 MiB/s, done.
Resolving deltas: 100% (9/9), done.
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
Unable to activate package ‘closql’.
Required package ‘emacsql-sqlite-3.0.0’ is unavailable
Unable to activate package ‘org-roam’.
Required package ‘emacsql-sqlite-1.0.0’ is unavailable
Unable to activate package ‘magit’.
Required package ‘git-commit-20230101’ is unavailable
Unable to activate package ‘orgit’.
Required package ‘magit-3.3.0’ is unavailable
Unable to activate package ‘magit’.
Required package ‘git-commit-20230101’ is unavailable
Unable to activate package ‘hydra’.
Required package ‘lv-0’ is unavailable
Unable to activate package ‘hydra’.
Required package ‘lv-0’ is unavailable
Unable to activate package ‘treemacs’.
Required package ‘hydra-0.13.2’ is unavailable
Unable to activate package ‘helm’.
Required package ‘helm-core-3.9.9’ is unavailable
Loading /tmp/subed/tests/undercover-init.el (source)...
Loading /tmp/subed/tests/undercover-init.el (source)...
Loading /tmp/subed/tests/undercover-init.el (source)...
Loading /tmp/subed/tests/undercover-init.el (source)...
Loading /tmp/subed/tests/undercover-init.el (source)...
Running 717 specs.

ASS
  Getting
    the subtitle start/stop time
      returns the time in milliseconds. (8.12ms)

(... omitted lines ...)

Get duration in milliseconds of a file with 1 video and 1 audio stream
    extension .mkv (217.80ms)
    extension .mp4 (212.89ms)
    extension .webm (575.76ms)
    extension .avi (192.16ms)
    extension .ts (179.63ms)
    extension .ogv (311.78ms)

Ran 717 specs, 0 failed, in 3.56s.
Error in kill-emacs-hook (subed-mpv-kill): (error "Process mock client process does not exist")
emacs --no-init-file -f package-initialize --batch \
          --eval "(require 'package-lint)" \
      --file package-lint-batch-and-exit \
      ./subed/subed.el
Unable to activate package ‘closql’.
Required package ‘emacsql-sqlite-3.0.0’ is unavailable
Unable to activate package ‘org-roam’.
Required package ‘emacsql-sqlite-1.0.0’ is unavailable
Unable to activate package ‘magit’.
Required package ‘git-commit-20230101’ is unavailable
Unable to activate package ‘orgit’.
Required package ‘magit-3.3.0’ is unavailable
Unable to activate package ‘magit’.
Required package ‘git-commit-20230101’ is unavailable
Unable to activate package ‘hydra’.
Required package ‘lv-0’ is unavailable
Unable to activate package ‘hydra’.
Required package ‘lv-0’ is unavailable
Unable to activate package ‘treemacs’.
Required package ‘hydra-0.13.2’ is unavailable
Unable to activate package ‘helm’.
Required package ‘helm-core-3.9.9’ is unavailable

Error: file-missing ("Cannot open load file" "No such file or directory" "package-lint")
  mapbacktrace(#f(compiled-function (evald func args flags) #<bytecode 0x1d146c0931aef185>))
  debug-early-backtrace()
  debug-early(error (file-missing "Cannot open load file" "No such file or directory" "package-lint"))
  require(package-lint)
  command-line-1(("-f" "package-initialize" "--eval" "(require 'package-lint)" "--file" "package-lint-batch-and-exit" "./subed/subed.el"))
  command-line()
  normal-top-level()
Cannot open load file: No such file or directory, package-lint
make: *** [Makefile:23: package-lint] Error 255
$ echo $?
2

I'd be happy to find the root cause that is making your system have a different behavior.

rodrigo-morales-1 avatar Jul 11 '24 18:07 rodrigo-morales-1

Thank you for helping me investigate this.

$ rm -rf /tmp/subed && cd /tmp && git clone --branch main --depth 1 'https://github.com/sachac/subed' && cd /tmp/subed && make test
Cloning into 'subed'...
remote: Enumerating objects: 47, done.
remote: Counting objects: 100% (47/47), done.
remote: Compressing objects: 100% (42/42), done.
remote: Total 47 (delta 9), reused 26 (delta 4), pack-reused 0
Receiving objects: 100% (47/47), 219.17 KiB | 3.98 MiB/s, done.
Resolving deltas: 100% (9/9), done.
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
Loading /etc/emacs/site-start.d/00debian.el (source)...
Loading /etc/emacs/site-start.d/50dictionaries-common.el (source)...
Loading debian-ispell (native compiled elisp)...
Loading /var/cache/dictionaries-common/emacsen-ispell-default.el (source)...
Loading /var/cache/dictionaries-common/emacsen-ispell-dicts.el (source)...
Loading /tmp/subed/tests/undercover-init.el (source)...
Loading /tmp/subed/tests/undercover-init.el (source)...
Loading /tmp/subed/tests/undercover-init.el (source)...
Eager macro-expansion failure: (buttercup-dynamic-binding-error . "buttercup requires `lexical-binding' to be t")
make: *** [Makefile:17: test-coverage] Error 255
$ echo $?
2

Looks pretty much like yours except I'm not getting any of these "Required package is unavailable" messages. I assume that has something to do with your ~/.emacs.d?

And you are missing the loading of several Debian-specific .el files. 00debian.el only sets gnus-nntpserver-file and mail-host-address. The ispell stuff also seems unrelated to me.

I've also tried adding --no-init-file, but it doesn't make any difference.

I'm stumped.

rndusr avatar Jul 11 '24 19:07 rndusr

Hmm... @rndusr, is this still happening for you? (No rush, please check when it's convenient for you.) Sorry about it!

sachac avatar Oct 18 '24 18:10 sachac

OK, so I've installed subed via package.el from melpa and the tests run fine now. Everything is green.

I'm getting this output from make test-coverage, but it seems to be only a warning:

mkdir -p coverage
UNDERCOVER_FORCE=true emacs -Q -batch -L . -f package-initialize -f buttercup-run-discover
Unable to activate package ‘undercover’.
Required package ‘dash-2.0.0’ is unavailable

I have dash installed as a Debian package because other packages depend on it, and the -Q switch in the emacs command line implies --no-site-file, which prevents loading of dash, so this seems to be expected.


But I got 371 lines of compile warnings when I installed subed. I'm not sure how relevant most of them are. For example, these seem to be caused by me not having org-mode installed (I know, I'm a heathen):

In org-subed-store-link:
ol-subed.el:41:5: Warning: reference to free variable ‘org-store-link-plist’

In end of data:
ol-subed.el:59:4: Warning: the function ‘org-link-set-parameters’ is not known
    to be defined.
ol-subed.el:39:6: Warning: the function ‘org-link-add-props’ is not known to
    be defined.
ol-subed.el:32:6: Warning: the function ‘org-link-store-props’ is not known to
    be defined.

But I'm also getting stuff like this:

subed-common.el:250:68: Warning: Unused lexical argument `sub-id'
subed-common.el:428:54: Warning: Unused lexical argument `comment'
subed-common.el:428:2: Warning: argument ‘_’ not left unused
subed-common.el:434:2: Warning: defalias `subed--set-subtitle-time-start'
    docstring has wrong usage of unescaped single quotes (use \= or different
    quoting)

Weirdly, when I run make compile, I only get 4 "docstring wider than 80 characters" and no other warnings.

Do you get any warnings if you install subed via package.el like a normal user?

rndusr avatar Oct 25 '24 09:10 rndusr