all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#51941: Detect duplication of ERT tests
@ 2021-11-18 10:16 Mattias Engdegård
  2021-11-18 10:35 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 19+ messages in thread
From: Mattias Engdegård @ 2021-11-18 10:16 UTC (permalink / raw)
  To: 51941

[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]

Duplicated `ert-deftest` forms (ie, two or more definitions with the same test name) is a serious problem: the older test is silently ignored, which has the consequence of the test covering less than intended.

A quick experiment in the Emacs tree immediately reveals 51 instances. There's no telling how many lie waiting in external code.

Having explored various options I've come to the conclusion that it needs to be a hard error, at run-time, when running non-interactively (see attached diff). Rationale:

* We can't just warn at run time because tests tend to spam a lot when run and users only care about whether the tests passed and won't even look at the logs otherwise.
* We can't just warn at compile time because many tests aren't compiled at all, and where they are (as in the Emacs tree) nobody cares much about the warnings, because they are "just tests".
* We can't issue a complaint when running interactively because it's natural to keep redefining tests during development.

Since `ert-deftest` forms are often generated as a result of macro-expansion, passive static textual linting won't do.

The effect of this change will be a visible and easily remedied failure in broken test code. I volunteer to fix the first-order errors in Emacs (by renaming the clashes); domain specialists may need to help fixing secondary failures (failures in previously ignored tests).


[-- Attachment #2: ert-deftest-redefine-error.diff --]
[-- Type: application/octet-stream, Size: 661 bytes --]

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 36b4408dc8..92a1315f13 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -151,6 +151,10 @@ ert-set-test
     ;; Note that nil is still a valid value for the `name' slot in
     ;; ert-test objects.  It designates an anonymous test.
     (error "Attempt to define a test named nil"))
+  (when (and noninteractive (get symbol 'ert--test))
+    ;; If a test is redefined when running noninteractively, treat it
+    ;; as a hard error to make sure it is discovered.
+    (error "Test `%s' redefined" symbol))
   (define-symbol-prop symbol 'ert--test definition)
   definition)
 

^ permalink raw reply related	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-11-24 12:16 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-18 10:16 bug#51941: Detect duplication of ERT tests Mattias Engdegård
2021-11-18 10:35 ` Lars Ingebrigtsen
2021-11-18 11:54   ` Mattias Engdegård
2021-11-18 11:58     ` Lars Ingebrigtsen
2021-11-18 17:44     ` Lars Ingebrigtsen
2021-11-18 17:53       ` Philipp Stephani
2021-11-18 19:13       ` Mattias Engdegård
2021-11-18 19:34         ` Lars Ingebrigtsen
2021-11-18 19:43           ` Mattias Engdegård
2021-11-18 22:51         ` Glenn Morris
2021-11-19  5:28           ` Lars Ingebrigtsen
2021-11-19  9:33             ` Mattias Engdegård
2021-11-20  8:38               ` Lars Ingebrigtsen
2021-11-20  8:49                 ` Mattias Engdegård
2021-11-24  7:30   ` Ihor Radchenko
2021-11-24  8:52     ` Mattias Engdegård
2021-11-24  9:29       ` Ihor Radchenko
2021-11-24 10:00         ` Mattias Engdegård
2021-11-24 12:16           ` Ihor Radchenko

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.