Initial erc tests

[svaante]

Basis for tests discussion

[joaotavora]

Looks just fine for a start. Be prepared to turn it on its head though soon, like everything.
Now put it in GitHub actions!

[joaotavora]

Make these BODY taking macros hygienic, delegate immediately to a function. Google "CALL-WITH idiom"

[svaante]

Sure, is it because of stack traces. Whats your reasoning? Just curious I am allergic to "best practices"

[joaotavora]

No, not specifically because of stack traces. One of the benefits is that you change the implementation of
the wrapper, you don't need to find all the users of the macro and recompile them: you just recompile the implementation. The other benefit has to do with hygiene and shadowing. By wrapping the body in a lambda, you guarantee that it is not affected by bindings int the macro preamble. Another way to solve this is with gensym or make-symbol.

This is actually not unlike the problems posed by the C preprocessor.

I am allergic to "best practices"

:-) That's a very healthy position. Indeed always question. "best practices" is often very close to "cargo cult"

[joaotavora]

So in your original macro I think you had something like

(defmacro dape--with-files (file-fixtures &rest body)
  `(let* ((temp-dir (make-temp-file "dape-tests-" t))
     ...,@body
     (remove-directory temp-dir)))

or something like that. If some use of this macro a body had a (setq temp-dir something) you would not get an error and the code would be silently (or loudly) broken. The alternative to call-with to make it hygeniec is to:

(defmacro dape--with-files (... &rest body)
   (let ((temp-dir-sym (gensym)))
     `(let ((,temp-dir-sym (make-temp-file...)))
          ,@body
          (remove-directory ,temp-dir-sym))))

But this is ugly to read and makes for funky macroexpansions. Very common though. The call with-idiom is much better
and has other benefits.

(defmacro dape--with-files (... &rest body)
   `(call-with-files (make-temp-file ...) (lambda () ,@body)))

(defun call-with-files (temp-dir fn)
   (funcall fn)
   (remove-directory temp-dir))

[joaotavora]

Emacs convention is to call this 'check'. Pretty arbitrary, I guess.

[joaotavora]

Interesting macro name and semantics! Will copy

[joaotavora]

You might want condition-case-unless-debug here, else you lock yourself out of useful backtraces if something does go wrong

[svaante]

Ah nice one, had know idea of the existence of condition-case-unless-debug

[svaante]

Reverted, ert enables debug on error which was the reson for it the first time around. As you know this code will be replaced by jsonrpc anyway.

The handling of js-debug termination is not handled a clean way, there is no orphaned process but process is dies during termination with an strange timing.

For some reason one of the test does not work for emacs 28.2, which I am not motivated at this time to solve.

[joaotavora]

Fair, and indeed ert enabling debug-on-error is a pain . You can rebind it in your tests tho. But you're right, this is soon to be replaced anyway.

[joaotavora]

You know what you be a really good idea this early in the dape-test.el file? Namespaces. I.e. make the new definitions of dape-test.el start with the prefix dape-test- (or dape-test--). Then it would be much easier to track what bits of dape.el you are actually probing/exercising.

I didn't do this for eglot-tests.el and I kind of regret it: it's a bit of a mess that periodically bites me.

Of course, I can hear you say already "but typing dape-tests-- everytime is a pain!! I'll never remember to do it" And if you are saying that I fully agree. So you could use shorthands in this file, they are supported since Emacs 28 and some extensions out there are using it.

So something like this footer in dape-test.el

;; Local Variables:
;; read-symbol-shorthands: (("dt-" . "dape-tests-"))
;; End:

All the symbols are still named dape-tests-<foo> but you can type them (in this file only, of course) as dt- (or test- or daniel- or t- or whatever you prefer).

[joaotavora]

Follow emacs make test target naming

Speaking of convention, another reasonably arbitrary one is that the file containing tests for package foo.el is named foo-tests.el plural, not foo-test.el. So if there's no good reason not to, I'd switch to that too.

[joaotavora]

Speaking of convention, another reasonably arbitrary one is that the file containing tests for package foo.el is named foo-tests.el plural, not foo-test.el. So if there's no good reason not to, I'd switch to that too.

Duh, nevermind, you already have it like that. 🤦

[svaante]

You know what you be a really good idea this early in the dape-test.el file? Namespaces. I.e. make the new definitions of dape-test.el start with the prefix dape-test- (or dape-test--). Then it would be much easier to track what bits of dape.el you are actually probing/exercising.

I didn't do this for eglot-tests.el and I kind of regret it: it's a bit of a mess that periodically bites me.

Of course, I can hear you say already "but typing dape-tests-- everytime is a pain!! I'll never remember to do it" And if you are saying that I fully agree. So you could use shorthands in this file, they are supported since Emacs 28 and some extensions out there are using it.

So something like this footer in dape-test.el

;; Local Variables:
;; read-symbol-shorthands: (("dt-" . "dape-tests-"))
;; End:

All the symbols are still named dape-tests-<foo> but you can type them (in this file only, of course) as dt- (or test- or daniel- or t- or whatever you prefer).

Will do, makes sense. Is this the best diss I have ever received daniel-? Im hoping you are referring to what I think you are referring to.

[joaotavora]

is this the best diss I have ever received daniel-

I'm lost 😬 You are Daniel right? (Or Svante?) I just meant daniel- as an example of the shorthand prefix you can use, like I could use joao-, but the actual symbols would be dape-test-foo. Sorry, I didn't mean to diss at all