GNU bug report logs - #17851
Some ert tests fail when run compiled

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 17851 in the body.
You can then email your comments to 17851 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #3 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: submit <at> debbugs.gnu.org
Subject: Some ert tests fail when run compiled
Date: Thu, 26 Jun 2014 01:57:47 -0400

Package: emacs
Version: 24.4.50

The following tests fail when run from a compiled file ert-tests.elc:

  ert-test-record-backtrace
  ert-test-should-failure-debugging

This was never apparent before because even though the files in
test/automated were compiled, the tests were always run from the
uncompiled file.

I'll simply add no-byte-compile to this file for now.

Message #6 received at 17851 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Glenn Morris <rgm <at> gnu.org>
Cc: 17851 <at> debbugs.gnu.org
Subject: Re: bug#17851: Some ert tests fail when run compiled
Date: Mon, 22 Aug 2016 21:31:49 -0400

[Message part 1 (text/plain, inline)]

tags 17851 patch
quit

Glenn Morris <rgm <at> gnu.org> writes:

> The following tests fail when run from a compiled file ert-tests.elc:
>
>   ert-test-record-backtrace
>   ert-test-should-failure-debugging
>
> This was never apparent before because even though the files in
> test/automated were compiled, the tests were always run from the
> uncompiled file.
>
> I'll simply add no-byte-compile to this file for now.

First one is an overly implementation specific test, second seems to be
a bug in the `should' macro.  Here's a patch to fix both (and also
another nearby test that was too implementation specific):

[v1-0001-Fix-ert-tests-when-running-compiled.patch (text/plain, inline)]

From a943e65f7ed09b843d99bcb93aa4c47ec1815276 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sun, 21 Aug 2016 22:58:37 -0400
Subject: [PATCH v1] Fix ert-tests when running compiled

* test/lisp/emacs-lisp/ert-tests.el (ert-test-deftest): Don't test for
specific macroexpansion, just check result of evaluation.
(ert-test-record-backtrace): Don't hardcode representation of closure in
expected backtrace, this lets the test succeed even when the test code
is compiled (Bug #17851).

* lisp/emacs-lisp/ert.el (ert--expand-should-1): Also pass
`byte-compile-macro-environment' to `macroexpand', this allows the
`should' macro to properly handle macroexpansion of macros that were
defined in the same file when it's being compiled (Bug #17851).
---
 lisp/emacs-lisp/ert.el            | 11 ++++---
 test/lisp/emacs-lisp/ert-tests.el | 68 +++++++++++++--------------------------
 2 files changed, 29 insertions(+), 50 deletions(-)

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 0308c9c..89f83dd 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -276,11 +276,12 @@ ert--special-operator-p
 (defun ert--expand-should-1 (whole form inner-expander)
   "Helper function for the `should' macro and its variants."
   (let ((form
-         (macroexpand form (cond
-                            ((boundp 'macroexpand-all-environment)
-                             macroexpand-all-environment)
-                            ((boundp 'cl-macro-environment)
-                             cl-macro-environment)))))
+         (macroexpand form (append byte-compile-macro-environment
+                                   (cond
+                                    ((boundp 'macroexpand-all-environment)
+                                     macroexpand-all-environment)
+                                    ((boundp 'cl-macro-environment)
+                                     cl-macro-environment))))))
     (cond
      ((or (atom form) (ert--special-operator-p (car form)))
       (let ((value (cl-gensym "value-")))
diff --git a/test/lisp/emacs-lisp/ert-tests.el b/test/lisp/emacs-lisp/ert-tests.el
index 5d36755..38d6f65 100644
--- a/test/lisp/emacs-lisp/ert-tests.el
+++ b/test/lisp/emacs-lisp/ert-tests.el
@@ -344,53 +344,35 @@ ert--test-my-list
        ((error)
         (should (equal actual-condition expected-condition)))))))
 
+(defun ert-test--which-file ()
+  "Dummy function to help test `symbol-file' for tests.")
+
 (ert-deftest ert-test-deftest ()
-  ;; FIXME: These tests don't look very good.  What is their intent, i.e. what
-  ;; are they really testing?  The precise generated code shouldn't matter, so
-  ;; we should either test the behavior of the code, or else try to express the
-  ;; kind of efficiency guarantees we're looking for.
-  (should (equal (macroexpand '(ert-deftest abc () "foo" :tags '(bar)))
-		 '(progn
-		    (ert-set-test 'abc
-				  (progn
-				    "Constructor for objects of type `ert-test'."
-				    (vector 'cl-struct-ert-test 'abc "foo"
-					    #'(lambda nil)
-					    nil ':passed
-					    '(bar))))
-		    (setq current-load-list
-			  (cons
-			   '(ert-deftest . abc)
-			   current-load-list))
-		    'abc)))
-  (should (equal (macroexpand '(ert-deftest def ()
-                                 :expected-result ':passed))
-		 '(progn
-		    (ert-set-test 'def
-				  (progn
-				    "Constructor for objects of type `ert-test'."
-				    (vector 'cl-struct-ert-test 'def nil
-					    #'(lambda nil)
-					    nil ':passed 'nil)))
-		    (setq current-load-list
-			  (cons
-			   '(ert-deftest . def)
-			   current-load-list))
-		    'def)))
+  (ert-deftest ert-test-abc () "foo" :tags '(bar))
+  (let ((abc (ert-get-test 'ert-test-abc)))
+    (should (equal (ert-test-tags abc) '(bar)))
+    (should (equal (ert-test-documentation abc) "foo")))
+  (should (equal (symbol-file ert-test-deftest 'ert-deftest)
+                 (symbol-file ert-test--whichfile 'defun)))
+
+  (ert-deftest ert-test-def () :expected-result ':passed)
+  (let ((def (ert-get-test 'ert-test-def)))
+    (should (equal (ert-test-expected-result-type def) :passed)))
   ;; :documentation keyword is forbidden
   (should-error (macroexpand '(ert-deftest ghi ()
                                 :documentation "foo"))))
 
 (ert-deftest ert-test-record-backtrace ()
-  (let ((test (make-ert-test :body (lambda () (ert-fail "foo")))))
-    (let ((result (ert-run-test test)))
-      (should (ert-test-failed-p result))
-      (with-temp-buffer
-        (ert--print-backtrace (ert-test-failed-backtrace result))
-        (goto-char (point-min))
-	(end-of-line)
-	(let ((first-line (buffer-substring-no-properties (point-min) (point))))
-	  (should (equal first-line "  (closure (ert--test-body-was-run t) nil (ert-fail \"foo\"))()")))))))
+  (let* ((test-body (lambda () (ert-fail "foo")))
+         (test (make-ert-test :body test-body))
+         (result (ert-run-test test)))
+    (should (ert-test-failed-p result))
+    (with-temp-buffer
+      (ert--print-backtrace (ert-test-failed-backtrace result))
+      (goto-char (point-min))
+      (end-of-line)
+      (let ((first-line (buffer-substring-no-properties (point-min) (point))))
+        (should (equal first-line (format "  %S()" test-body)))))))
 
 (ert-deftest ert-test-messages ()
   :tags '(:causes-redisplay)
@@ -837,7 +819,3 @@ ert--test-my-list
 (provide 'ert-tests)
 
 ;;; ert-tests.el ends here
-
-;; Local Variables:
-;; no-byte-compile: t
-;; End:
-- 
2.9.2

Message #11 received at 17851 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Glenn Morris <rgm <at> gnu.org>
Cc: 17851 <at> debbugs.gnu.org
Subject: Re: bug#17851: Some ert tests fail when run compiled
Date: Wed, 07 Sep 2016 21:15:48 -0400

[Message part 1 (text/plain, inline)]

npostavs <at> users.sourceforge.net writes:
>
> First one is an overly implementation specific test, second seems to be
> a bug in the `should' macro.  Here's a patch to fix both (and also
> another nearby test that was too implementation specific):

Had some minor problems in the test (I must have run only an
intermediate version before), here's a new patch.

[v2-0001-Fix-ert-tests-when-running-compiled.patch (text/plain, inline)]

From 8ee7db328a275710fd9999757b5578514a83be6c Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sun, 21 Aug 2016 22:58:37 -0400
Subject: [PATCH v2] Fix ert-tests when running compiled

* test/lisp/emacs-lisp/ert-tests.el (ert-test-deftest): Don't test for
specific macroexpansion, just check result of evaluation.
(ert-test-record-backtrace): Don't hardcode representation of closure in
expected backtrace, this lets the test succeed even when the test code
is compiled (Bug #17851).

* lisp/emacs-lisp/ert.el (ert--expand-should-1): Also pass
`byte-compile-macro-environment' to `macroexpand', this allows the
`should' macro to properly handle macroexpansion of macros that were
defined in the same file when it's being compiled (Bug #17851).
---
 lisp/emacs-lisp/ert.el            | 11 ++++---
 test/lisp/emacs-lisp/ert-tests.el | 68 +++++++++++++--------------------------
 2 files changed, 29 insertions(+), 50 deletions(-)

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 0308c9c..89f83dd 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -276,11 +276,12 @@ ert--special-operator-p
 (defun ert--expand-should-1 (whole form inner-expander)
   "Helper function for the `should' macro and its variants."
   (let ((form
-         (macroexpand form (cond
-                            ((boundp 'macroexpand-all-environment)
-                             macroexpand-all-environment)
-                            ((boundp 'cl-macro-environment)
-                             cl-macro-environment)))))
+         (macroexpand form (append byte-compile-macro-environment
+                                   (cond
+                                    ((boundp 'macroexpand-all-environment)
+                                     macroexpand-all-environment)
+                                    ((boundp 'cl-macro-environment)
+                                     cl-macro-environment))))))
     (cond
      ((or (atom form) (ert--special-operator-p (car form)))
       (let ((value (cl-gensym "value-")))
diff --git a/test/lisp/emacs-lisp/ert-tests.el b/test/lisp/emacs-lisp/ert-tests.el
index 5d36755..83fddd1 100644
--- a/test/lisp/emacs-lisp/ert-tests.el
+++ b/test/lisp/emacs-lisp/ert-tests.el
@@ -344,53 +344,35 @@ ert--test-my-list
        ((error)
         (should (equal actual-condition expected-condition)))))))
 
+(defun ert-test--which-file ()
+  "Dummy function to help test `symbol-file' for tests.")
+
 (ert-deftest ert-test-deftest ()
-  ;; FIXME: These tests don't look very good.  What is their intent, i.e. what
-  ;; are they really testing?  The precise generated code shouldn't matter, so
-  ;; we should either test the behavior of the code, or else try to express the
-  ;; kind of efficiency guarantees we're looking for.
-  (should (equal (macroexpand '(ert-deftest abc () "foo" :tags '(bar)))
-		 '(progn
-		    (ert-set-test 'abc
-				  (progn
-				    "Constructor for objects of type `ert-test'."
-				    (vector 'cl-struct-ert-test 'abc "foo"
-					    #'(lambda nil)
-					    nil ':passed
-					    '(bar))))
-		    (setq current-load-list
-			  (cons
-			   '(ert-deftest . abc)
-			   current-load-list))
-		    'abc)))
-  (should (equal (macroexpand '(ert-deftest def ()
-                                 :expected-result ':passed))
-		 '(progn
-		    (ert-set-test 'def
-				  (progn
-				    "Constructor for objects of type `ert-test'."
-				    (vector 'cl-struct-ert-test 'def nil
-					    #'(lambda nil)
-					    nil ':passed 'nil)))
-		    (setq current-load-list
-			  (cons
-			   '(ert-deftest . def)
-			   current-load-list))
-		    'def)))
+  (ert-deftest ert-test-abc () "foo" :tags '(bar))
+  (let ((abc (ert-get-test 'ert-test-abc)))
+    (should (equal (ert-test-tags abc) '(bar)))
+    (should (equal (ert-test-documentation abc) "foo")))
+  (should (equal (symbol-file 'ert-test-deftest 'ert-deftest)
+                 (symbol-file 'ert-test--which-file 'defun)))
+
+  (ert-deftest ert-test-def () :expected-result ':passed)
+  (let ((def (ert-get-test 'ert-test-def)))
+    (should (equal (ert-test-expected-result-type def) :passed)))
   ;; :documentation keyword is forbidden
   (should-error (macroexpand '(ert-deftest ghi ()
                                 :documentation "foo"))))
 
 (ert-deftest ert-test-record-backtrace ()
-  (let ((test (make-ert-test :body (lambda () (ert-fail "foo")))))
-    (let ((result (ert-run-test test)))
-      (should (ert-test-failed-p result))
-      (with-temp-buffer
-        (ert--print-backtrace (ert-test-failed-backtrace result))
-        (goto-char (point-min))
-	(end-of-line)
-	(let ((first-line (buffer-substring-no-properties (point-min) (point))))
-	  (should (equal first-line "  (closure (ert--test-body-was-run t) nil (ert-fail \"foo\"))()")))))))
+  (let* ((test-body (lambda () (ert-fail "foo")))
+         (test (make-ert-test :body test-body))
+         (result (ert-run-test test)))
+    (should (ert-test-failed-p result))
+    (with-temp-buffer
+      (ert--print-backtrace (ert-test-failed-backtrace result))
+      (goto-char (point-min))
+      (end-of-line)
+      (let ((first-line (buffer-substring-no-properties (point-min) (point))))
+        (should (equal first-line (format "  %S()" test-body)))))))
 
 (ert-deftest ert-test-messages ()
   :tags '(:causes-redisplay)
@@ -837,7 +819,3 @@ ert--test-my-list
 (provide 'ert-tests)
 
 ;;; ert-tests.el ends here
-
-;; Local Variables:
-;; no-byte-compile: t
-;; End:
-- 
2.9.3

Message #14 received at 17851 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Glenn Morris <rgm <at> gnu.org>
Cc: 17851 <at> debbugs.gnu.org
Subject: Re: bug#17851: Some ert tests fail when run compiled
Date: Tue, 06 Dec 2016 22:32:32 -0500

tags 17851 fixed
close 17851 26.1
quit

npostavs <at> users.sourceforge.net writes:

> npostavs <at> users.sourceforge.net writes:
>>
>> First one is an overly implementation specific test, second seems to be
>> a bug in the `should' macro.  Here's a patch to fix both (and also
>> another nearby test that was too implementation specific):
>
> Had some minor problems in the test (I must have run only an
> intermediate version before), here's a new patch.

Pushed as 58e418d2.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.