Re: emacs-29 9c0d7bb73b 2/2: Add automated tests for Eglot

[João Távora]

Michael,

Feel free to do all these changes in the test/lisp/progmodes/eglot-tests.el

FWIW I didn't write many of these tests, but I didn't care to

import the full Git history as I did in lisp/progmodes/eglot.el.

João

On Mon, Dec 12, 2022 at 10:59 AM Michael Albinus <michael.albinus@gmx.de> wrote:

João Távora <joaotavora@gmail.com> writes:

Hi João,

(sorry for sending this email accidentially when not finished)

> +(require 'tramp-sh)

Do you really need this?

> +(defun eglot--call-with-tramp-test (fn)
> +  ;; Set up a loopback TRAMP method that’s just a shell so the remote
> +  ;; host is really just the local host.
> +  (let ((tramp-remote-path (cons 'tramp-own-remote-path tramp-remote-path))
> +        (tramp-histfile-override t)
> +        (tramp-methods '(("loopback"
> +                          (tramp-login-program "/bin/sh")
> +                          (tramp-remote-shell "/bin/sh")
> +                          (tramp-remote-shell-login ("-l"))
> +                          (tramp-remote-shell-args ("-c")))))
> +        (temporary-file-directory (concat "/loopback::"
> +                                          temporary-file-directory)))

Pls don't do this. The better choice is to use

     (let* ((temporary-file-directory ert-remote-temporary-file-directory) ...)

See for example test/lisp/filenotify-tests.el, macro file-notify--deftest-remote

> +      ;; Tramp leave some buffers behind, and some time later,
> +      ;; `project-buffers' will trip over them causing a hard to debug
> +      ;; intermittent test failure somewhere else.
> +      (dolist (buf (buffer-list))
> +        (when (string-match-p "^\\*tramp" (buffer-name buf))
> +          (kill-buffer buf))))))

Just killing the buffers is not sufficient. The better way is to call
tramp-cleanup-connection, like done in that macro of filenotfy-tests.el

> +(ert-deftest eglot--tramp-test ()
> +  "Ensure LSP servers can be used over TRAMP."
> +  (skip-unless (executable-find "clangd"))

> +(ert-deftest eglot--tramp-test-2 ()
> +  "Ensure LSP servers can be used over TRAMP."
> +  (skip-unless (executable-find "clangd"))

Shouldn't the test be performed with bound remote default-directory? By
this, you could test Tramp + Eglot also in other variatioons, like a
real ssh connection.

Best regards, Michael.

--

João Távora