Support for ERT tests

[ericdallo]

Is there any way to run ert tests too?

[akirak]

ERT is not supported right now, but it wouldn't be hard to support it.

However, it would be necessary to determine whether the user wants to run buttercup or ert, which would require a schema change. Would you want the feature?

[ericdallo]

I see... It'd be great as emacs-lsp repos use ERT for tests and certainly would make this lib complete :)

[akirak]

This is an initial version of the major release, so I will extend the program as needed. I will add the ERT support soon.

[akirak]

Now ERT support has been added.

You have to update schema.dhall in your project and add the following testDrivers field to your package configuration:

let Schema = ./schema.dhall
let TestDriver = Schema.TestDriver
...
in [ Package :: {
  ...
  testDrivers = [ TestDriver.ert ],
  ...
}]

You can run tests using melpa-check test or melpa-check ert.

[akirak]

An initial support for ERT works now, but it lacks some packages needed for the test environment (on your lsp-dart package):

❯ melpa-check test
==========================================================
Tests on lsp-dart
==========================================================
File patterns: test?(s).el test-*.el *-test?(s).el test?(s)/*.el
Matched files: test/lsp-dart-test.el test/lsp-dart-utils-test.el test/test-helpe
r.el

GNU Emacs 28.0.50
Copyright (C) 2020 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.
----------------------------------------------------------
Running tests in test/lsp-dart-test.el...
[Treemacs] Warning: couldn't find hl-line-mode's background color for icons, fal
ling back on unspecified-bg.
[Treemacs Failure] Python3 not found, advanced git-mode and directory flattening
 features will be disabled.
dap-dart: Dart-Code.Dart-Code debug extension are not set. You can download it w
ith M-x dap-dart-setup
Cannot open load file: No such file or directory, el-mock
----------------------------------------------------------
Running tests in test/lsp-dart-utils-test.el...
[Treemacs] Warning: couldn't find hl-line-mode's background color for icons, fal
ling back on unspecified-bg.
[Treemacs Failure] Python3 not found, advanced git-mode and directory flattening
 features will be disabled.
dap-dart: Dart-Code.Dart-Code debug extension are not set. You can download it w
ith M-x dap-dart-setup
Cannot open load file: No such file or directory, el-mock
----------------------------------------------------------
Running tests in test/test-helper.el...
Cannot open load file: No such file or directory, el-mock
----------------------------------------------------------
Some Tests for lsp-dart have failed.

Error: 1/1 of the tasks have failed:
Exit code 1 from command "nix-shell --quiet /home/akirakomamura/projects/github/
lsp-dart/.melpa-check -A shellWithoutBuild --run 'set -e; melpaCheckNixBuild --n
o-build-output -A prepareAllTests.default >/dev/null && melpaCheckNixShell -A al
lTests.default'"

A solution would be to add a new field testDependencies which lets the user specify extra packages needed for running tests.

It is also necessary to exclude test-helper.el from the test targets. This will require another field like testExcludes which allow the user to specify files for test utilities.

I will implement the two fields. They will be effective both in buttercup and ERT tests.

[akirak]

I've implemented the features in the comment, but tests on lsp-dart still fail (using GNU Emacs 28.0.50).

I got the following message in all tests:

dap-dart: Dart-Code.Dart-Code debug extension are not set. You can download it with M-x dap-dart-setup

I've also seen the following error:

(void-function lsp-dart-test-from-dart-project)

What am I supposed to do to support this case? The tests are run in test directory in the repository, but is it wrong?

Also, in Emacs 25.2, the following error occurred (see this report for details):

----------------------------------------------------------
Running tests in test/lsp-dart-test.el...
Symbol's function definition is void: define-symbol-prop
----------------------------------------------------------
Running tests in test/lsp-dart-utils-test.el...
Symbol's function definition is void: define-symbol-prop
----------------------------------------------------------

The configuration is available at my fork: https://github.com/akirak/lsp-dart/tree/melpa-check-ci

[ericdallo]

Thanks for your hard work on this @akirak.

dap-dart: Dart-Code.Dart-Code debug extension are not set. You can download it with M-x dap-dart-setup

This is common when running elisp that uses dap-mode, is not an error.

the lsp-dart-test-from-dart-project is a helper function from test-helpers.el:38, you probably are not loading the test-helper before running the tests?

About the define-symbol-prop, I could not find anything related to this :pensive: the current GH Actions CI runs successfully on emacs 25.2

[akirak]

the lsp-dart-test-from-dart-project is a helper function from test-helpers.el:38, you probably are not loading the test-helper before running the tests?

I see. It turns out that preloading test-helper.el is a feature of ert-runner and thus cask exec ert-runner. Other alternatives such as makem.sh don't do that.

I wonder whether I should make it the default or add another option for enabling "ert-runner compatible" mode (or even "ert-runner" test driver). In the end, it may be a sensible choice to comply to ert-runner. Some people may load it explicitly in each test file, but require will prevent duplicate loading.

Thanks for your hard work on this @akirak.

Most people would stop using my package if it doesn't satisfy their requirements, but you've helped me grow my project. Not only I am using your project as a test bed, but you also give me insights on how to fix issues to make it more universal. Even if you end up with your current solution, it will be a good lesson for me. I must thank you for your extensive feedback.

[ericdallo]

Yeah, you are right, this is all ert-runner magic, I don't care too much about the default, but a option for use it could improve melpa-check coverability.

I spent a couple of days to make the current CI for lsp-dart/lsp-java/dap-mode/lsp-ui and others to work, and I feel is too verbose and with a lot of copy and paste. I'd like to change these packages to use melpa-check, starting with lsp-dart, if fits well, I could change the other projects, and even migrate lsp-mode, which is a very know project in emacs community, to use it (the currently CI is pretty flaky).

Most people would stop using my package if it doesn't satisfy their requirements, but you've helped me grow my project. Not only I am using your project as a test bed, but you also give me insights on how to fix issues to make it more universal. Even if you end up with your current solution, it will be a good lesson for me. I must thank you for your extensive feedback.

I was always searching for a project that does all this complex work for emacs packages automatically then I found this project which uses nix, which makes a lot of sense and gives Github Actions support :).

[akirak]

ert-runner support has been added, and your lsp-dart passes all tests still encounter the define-symbol-prop error, so I have to investigate it: https://github.com/akirak/lsp-dart/actions/runs/120414564

Because ert-runner (apparently) doesn't allow specifying test files (it seems to scan test directory), I have added a separate test driver named ert-runner.

I spent a couple of days to make the current CI for lsp-dart/lsp-java/dap-mode/lsp-ui and others to work, and I feel is too verbose and with a lot of copy and paste. I'd like to change these packages to use melpa-check, starting with lsp-dart, if fits well, I could change the other projects, and even migrate lsp-mode, which is a very know project in emacs community, to use it (the currently CI is pretty flaky).

An initial motivation behind this project was to support multi-package repositories well. I'm interested in how melpa-check can be made versatile, especially for supporting complex projects, and lsp-mode would be a really exciting case. It also helps me understand the ecosystem of Emacs from a different angle, so I will support your adoption if you would.

I was always searching for a project that does all this complex work for emacs packages automatically then I found this project which uses nix, which makes a lot of sense and gives Github Actions support :).

So far, melpa-check takes more time for running CI. I'm going to use it anyway, but to make it practical, I will probably have to implement some caching. That will be my job.

[akirak]

define-symbol-prop, which is defined in subr.el, is used in ert.el. Is this a bug with ERT in an old version?

[akirak]

define-symbol-prop was added in 2017 and first shipped with Emacs 26: https://github.com/emacs-mirror/emacs/commit/bfb8d33fd18b1d9fd5868204d472cb19f5bcafbe. ert.el was changed to use the function in the same commit. Perhaps ert-runner.el in Nixpkgs (and emacs-overlay) is using a newer version of ert.el that depends on the function which is not contained in Emacs 25.2:

;; ert-runner.el
(eval-and-compile
  (unless (require 'ert nil 'no-error)
    (load "ert-compat" nil 'no-message)))

[terlar]

That is quite strange as ert is bundled with Emacs, so I don't see how it could get the wrong version of ert/subr. Could you trace that error to see what path it takes and perhaps even resolve those elisp files to see the content and nix-store path?

[akirak]

@terlar

Store paths:

/nix/store/5f42yv7jbljiiywqmjxqxyhmnj0dqbwc-emacs-25.2/share/emacs/25.2/lisp/emacs-lisp/ert.el.gz /nix/store/har4f7cxzr7ngvhpfxr9cwha34k2hdam-emacs-packages-deps/share/emacs/site-lisp/elpa/ert-runner-20200526.642/ert-runner.el

Trace (through debug-on-error):

Debugger entered--Lisp error: (void-function define-symbol-prop) define-symbol-prop(lsp-range--pcase-macroexpander edebug-form-spec nil) byte-code("\300\301\302\303#\300\304\305\301#\303\207" [define-symbol-prop lsp-range--pcase-macroexpander edebug-form-spec nil lsp-range pcase-macroexpander] 5) require(lsp-mode) eval-buffer(#<buffer load-627030> nil "/run/user/1000/lsp-dart-ertMKv/lsp-dart-utils.el" nil t) ; Reading at buffer position 822 load-with-code-conversion("/run/user/1000/lsp-dart-ertMKv/lsp-dart-utils.el" "/run/user/1000/lsp-dart-ertMKv/lsp-dart-utils.el" nil t) require(lsp-dart-utils) eval-buffer(#<buffer load> nil "/run/user/1000/lsp-dart-ertMKv/test/lsp-dart-utils-test.el" nil t) ; Reading at buffer position 784 load-with-code-conversion("/run/user/1000/lsp-dart-ertMKv/test/lsp-dart-utils-test.el" "/run/user/1000/lsp-dart-ertMKv/test/lsp-dart-utils-test.el" nil t) load("/run/user/1000/lsp-dart-ertMKv/test/lsp-dart-utils-test.el" nil :nomessage) ert-runner--load("/run/user/1000/lsp-dart-ertMKv/test/lsp-dart-utils-test.el") -each(("/run/user/1000/lsp-dart-ertMKv/test/lsp-dart-utils-test.el" "/run/user/1000/lsp-dart-ertMKv/test/lsp-dart-test.el") ert-runner--load) ert-runner/run() apply(ert-runner/run nil) commander--handle-command(nil) commander-parse(nil)

/run/user/1000/lsp-dart-ertMKv/ is a temporary directory in which tests are run. Because the test suite contains a file-write operation, it copies all source files to a temporary directory. That would be irrelevant here, though.

[akirak]

This part seems to be throwing an error:

(pcase-defmacro lsp-range (region)
  "Build a `pcase' pattern that matches a LSP Range object.
Elements should be of the form (START . END), where START and END are bound
to the beginning and ending points in the range correspondingly."
  `(and (pred hash-table-p)
        (app (lambda (range) (lsp--position-to-point (gethash "start" range)))
             ,(car region))
        (app (lambda (range) (lsp--position-to-point (gethash "end" range)))
             ,(cdr region))))

pcase-defmacro has been using define-symbol-prop since the same commit as ert.el: https://github.com/emacs-mirror/emacs/commit/bfb8d33fd18b1d9fd5868204d472cb19f5bcafbe

[akirak]

Hello, I've changed melpa-check to install Emacs packages for testing using package.el, so the tests for lsp-dart passes on all Emacs versions now: https://github.com/akirak/lsp-dart/actions/runs/126809335

However, this approach has some restrictions, as described in this comment: https://github.com/akirak/melpa-check/issues/16#issuecomment-640138174

Thanks.

[terlar]

Great discovery, I didn’t think about the byte compilation being done by the default emacs if you access the packages directly like that.

I think you need to use this: https://github.com/nix-community/emacs-overlay/blob/master/default.nix#L109

I was planning to do a trial with this, but was not sure how to test this locally

[akirak]

@terlar I think I am already using emacsPackagesFor from the overlay, but it doesn't produce the desired result:

• https://github.com/akirak/melpa-check/blob/v3/default.nix#L13
• https://github.com/akirak/melpa-check/blob/v3/programs.nix#L24

Here pkgs is overridden by the emacs overlay, and emacsDerivation is Emacs of a given version from nix-emacs-ci.

To use a local version of melpa-check for testing, you can change melpa-check argument in .melpa-check/default.nix: https://github.com/akirak/melpa-check/blob/v3/.melpa-check/default.nix#L5

(This way, melpa-check allows overriding every argument at runtime without contributing to the project. emacs even accepts a derivation, not only a version string, and packageFile can be a Nix file, which has no precise specification. It tries to go beyond a typical case, which could be a selling point, but it has brought more complexity.)