Testing conventions

[victorolinasc]

Hi and thanks for this awesome style guide. It truly helps adding standards to this community and is also a place to discover best practices by established Elisp seniors.

One thing I miss from all this is testing conventions. It seems the community is just recently investing more time in CI pipelines with testing, coverage reports and so on. From my personal opinion, though, this topic is still not widely discussed nor standardized. Things like:

• What is a good practice for naming tests?
• When its good practice to create a wrapper macro to add functionality to ERT?
• Should we use test-helper.el and if yes, what is the recommended practices?
• How to assert failures with temp files without actually leaving them on the file system?
• etc (I think that even if we start this small it will grow bigger with the community input)

Maybe this should be a NEW style guide that could first recommend this one, but I think this will help a lot too.

Anyway, if this feels out of place feel free to simply close it. Once again, thanks for all your work!

[bbatsov]

There's also the question - should you use ERT in the first place? :-) I don't like it much and I think the solutions like Buttercup result in way nicer test suites.

But in general I agree that's an important topic that doesn't get enough coverage.

[victorolinasc]

Totally agree about having other frameworks too included in this kind of best-practices guide (which is a bit more than style only). There should be best practices for buttercup too I think. Either way if you think this is not the place to do it let's close it. Wdyt?

[Fuco1]

This is something I wanted to write for some time... I'm one of those perverts who enjoy testing :blush: . There's definitely a lot of approaches in elisp and quite often writing tests is the biggest pain when contributing. I've seen my share of arcane setups. Especially since ERT is very free-form by design. Buttercup is much nicer because it forces you to do things mostly one way.

I have written this for the smartparens project specifically but there is some general info which could be extracted: https://github.com/Fuco1/smartparens/wiki/Testing

The general remarks are sort of similar across all languages not just elisp/emacs:

• Write self-contained tests: CLEARLY specify all the preconditions in the test itself and don't spread it around 50 different helpers.
• DRY is (mostly) your enemy in tests. When someone is fixing your code they want to look at the one single test, not 10 layers of inheritance of beforeEach. This can become hell in BDD style tests.
• Test one thing per test. Really! This doesn't mean don't have multiple asserts, but have them all relate to a single behaviour. If you are testing two different setups (for example an option on true/false), make two tests.
• Delete useless tests.
• Write tests in such a way that you will not be sad when someone deletes them. Test code is simple and expendable, it's not your PhD thesis.
• Try not to depend on irrelevant bits. Only mock as much as required. A change in unrelated API should not break your test.

Obviously apply common sense, nothing is absolute.

About frameworks, I would push buttercup where possible. For "legacy" projects on ERT, migrations might be useful depending on the scope. For example in smartparens we have over 2000 test cases and migration would be very difficult. If you have 50 tests I would bite the bullet and switch.

For things like "run in a sandbox" I would write a helper (for example using unwind-protect to restore the state of world). Some macros with 0-2 arguments are OK especially with packages like macroexpand where you can quickly check what is the product. Avoid macros/helpers with 5+ arguments as those are just too unobvious. It's ok to write (expect-buffer "foo") as a shortcut for (expect (equal (buffer-string) "foo")). Just don't overdo it.

Naming of tests to me is not very important so long as it's unique :D For complicated setups describe the preconditions and why you are testing this thing (for example link to an issue). ERT tests can have docstrings, for example:

;; #699
(ert-deftest sp-test-get-thing-clojure-with-regexp-based-prefix nil
  "When the point is inside a prefix which is not a syntactic
prefix and we try to skip to previous symbol, the prefix might
stop the skip routine and prevent the previous token from being
picked up, causing `sp-get-thing' to take the 2nd previous one."
  (let ((sp-sexp-prefix '((clojure-mode regexp "#")))
        (sp-pairs '((t . ((:open "(" :close ")" :actions (insert wrap autoskip navigate))
                          (:open "{" :close "}" :actions (insert wrap autoskip navigate)))))))
    (sp-test-thing-parse-in-clojure "(atom #|{})" '(:beg 2 :end 6 :op "" :cl "" :prefix "" :suffix "") t)))

When I want to group multiple tests, I use prog1 in ERT (in buttercup you use describe natively):

(prog1 "#821 sp-backward-kill-word skips over prefix and does not
kill it as a word/symbol"
  (ert-deftest sp-test-ess-prefix-resolution-ignored-for-backward-kill-symbol--string ()
    (sp-test-with-temp-buffer "mean(\"foo|\")"
        (sp-test--ess-mode)
      (sp-backward-kill-word 1)
      (sp-buffer-equals "mean(\"|\")")))

  (ert-deftest sp-test-ess-prefix-resolution-ignored-for-backward-kill-symbol--function-call ()
    (sp-test-with-temp-buffer "ggplot(mtcars, aes(mpg, am)) + facet_wrap|()"
        (sp-test--ess-mode)
      (sp-backward-kill-word 1)
      (sp-buffer-equals "ggplot(mtcars, aes(mpg, am)) + facet_|()"))))

That's enough for now.