emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Eglot tests on EMBA


From: João Távora
Subject: Re: Eglot tests on EMBA
Date: Fri, 31 Mar 2023 14:35:43 +0100

On Fri, Mar 31, 2023 at 1:14 PM Michael Albinus <michael.albinus@gmx.de> wrote:
>
> João Távora <joaotavora@gmail.com> writes:
>
> Hi João,
>
> >> I'm not happy with this. EMBA installs a conservative list of software;
> >> they shall be available as Debian bullseye package.
> >
> > Then you should complain to Debian and uninstall pylsp on EMBA is the
> > meantime.
>
> Nothing to complain to Debian. They offer pylsp in the testing version;
> we have simply decided not to use it. We use the stable version.
>
> >> Since Debian bullseye does not offer the package python3-pylsp (which
> >> will be available with a later Debian release), I install on EMBA
> >> python3-pyls. This shall be sufficient, because (according to
> >> eglot-server-programs) "pyls" is supported.
> >>
> >> eglot-tests.el does not dertect this, it checks only for "pylsp". I
> >> believe this shall be changed,
> >
> > No, it shan't :-) I'm not even sure how to do so portably.
>
> The most simple way would be
>
> --8<---------------cut here---------------start------------->8---
> (skip-unless (or (executable-find "pylsp") (executable-find "pyls")))
> --8<---------------cut here---------------end--------------->8---
>
> But this is a sledge hammer approach I guess.

Hmm, I'm confused.  Why are we talking about 'pyls'?  The
test is for `pylsp`.  It used to be for `pyls`, but now it's
not.  'pyls' would probably have the same "provider" issue.

My point is that I don't know how to ask the `pylsp`
executable easily what "providers" it supports.  pylsp --version
doesn't hint at it. Maybe there is an easy way, but I don't know it.

Maybe you meant these kinds of changes to eglot-tests.el?


diff --git a/test/lisp/progmodes/eglot-tests.el
b/test/lisp/progmodes/eglot-tests.el
index b11ce942b7d..7215dc24b3e 100644
--- a/test/lisp/progmodes/eglot-tests.el
+++ b/test/lisp/progmodes/eglot-tests.el
@@ -570,7 +570,8 @@ eglot-test-non-unique-completions
       '(("project" . (("something.py" . "foo=1\nfoobar=2\nfoo"))))
     (with-current-buffer
         (eglot--find-file-noselect "project/something.py")
-      (should (eglot--tests-connect))
+      (let ((eglot-server-programs '((python-mode . ("pylsp")))))
+        (should (eglot--tests-connect)))
       (goto-char (point-max))
       (completion-at-point))
     ;; FIXME: `current-message' doesn't work here :-(


This makes sense to me.  It's an oversight that eglot-server-programs
isn't restricted there like it is in other tests.

To avoid hurting readability too much, this override could
be an option to the eglot--with-fixture macro.

> eglot-tests.el run for your environment. But there are people with other
> environments (like Debian stable), which could errors running
> eglot-tests.el.

Yes, I understand, but I also doubt the size of the set intesecting
people who have simplistic pylsp installations and people running
the Emacs test suite.  So I'm not very worried.  And most people are
using pyright these days anyway.

> >> the check shall be for all alternatives
> >> configured in eglot-server-programs. And not only for python, but also
> >> for other languages.
> >
> > No, that's an immense amount of work and that's not what these
> > tests are really for.  People add stuff to eglot-server-programs
> > liberally: it's a huge database now. I'm not crazy about that but
> > is has always been the most  fair practice to acommodate server
> > preferences, and people seem like to see their favourite server at
> > least represented  there.
>
> Is it that hard to extract all relevant server executables for a given
> major-mode, and iterate over them with executable-find?

That's not hard, but what do you want to do with this information?
What test do you want to design?  I'm not following.  Maybe I'm
missing something.

> > But some of these servers don't work, are deprecated, have
> > inconsistent executable names on different platforms: it's
> > a mess.  Testing that is way beyond the scope of eglot-tests.el.
>
> In such a case, the test shall fail. And that would be OK, by this you
> get feedback what doesn't work (given people write bug reports).

I don't follow.  What test fails?  What would be considered "failure"?
Not having all possible programs mentioned in `eglot-server-programs`
installed?  Can't be that, that's just tyranny :-)  There's a lot
of stuff in there.

> > You may lobby for a eglot-server-program-tests.el instead,
> > and I won't oppose it, but I personally don't have time for that,
> > nor do I see a lot of value at the moment.
> >
> > eglot-tests.el is testing the server-agnostic Elisp logic in
> > eglot.el -- Eglot _is_ server agnostic.  We just happen to need
> > someone speaking LSP to be able to exercise features.  We could just
> > as well have a (compliant) Elisp LSP server simulator and not worry
> > about  external language servers in eglot-tests.el at all.  But that's quite
> > a bit of work.  So some common installations of language servers are needed.
>
> Sure, possible. I was just speaking about the given state of eglot-tests.el.

It's not super, because of these outside dependencies, but it's not a
very bad state, I would say.  It helped me catch a bug the other day :-)

João



reply via email to

[Prev in Thread] Current Thread [Next in Thread]