bug#71988: 31.0.50; ert-test-run-tests-batch-expensive runs out of memory if previous tests failed

bug#71988: 31.0.50; ert-test-run-tests-batch-expensive runs out of memory if previous tests failed

[Pip Cet via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

This bug report is mostly to get a bug number to put in the patch :-).

The test `ert-test-run-tests-batch-expensive' overrides print settings
and prints a full backtrace to a temporary buffer repeatedly. If a
previous test, such as `ert-test-run-tests-batch', failed, this
eventually runs out of memory even on a 64 GB system.

For example, one can add "(should nil)" to ert-test-run-tests-batch and
run:

$ make -C test lisp/emacs-lisp/ert-tests

The problem is the ert--stats structure's tests vector is being printed,
which recursively prints all previous tests' data.

A possible fix is to temporarily override cl-print-object for either
vectors or the ert--stats object to print nothing.

Unfortunately, we don't appear to have an official mechanism for
temporarily calling cl-defmethod, such as a generalized variable one can
cl-letf to. The patch does that by running cl-defmethod inside a
(cl-letf* (((symbol-function 'cl-print-object) (symbol-function
'cl-print-object))) ...) form. As future changes to cl-generic might
break that, an additional test is added to make sure our binding is
undone and die loudly if it hasn't been.

This also speeds up the test significantly and it might be possible to
remove the :unstable tag.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ert-tests-reproducer.patch --]
[-- Type: text/x-patch; name=0001-ert-tests-reproducer.patch, Size: 462 bytes --]

diff --git a/test/lisp/emacs-lisp/ert-tests.el b/test/lisp/emacs-lisp/ert-tests.el
index 1aff73d66f6..2f8e7382bea 100644
--- a/test/lisp/emacs-lisp/ert-tests.el
+++ b/test/lisp/emacs-lisp/ert-tests.el
@@ -583,6 +583,7 @@ ert-test-run-tests-batch
 		   (setq found-long (string-match long-text msg)))
 		 (unless found-complex
 		   (setq found-complex (string-match complex-text msg))))
+        (should nil)
 	(should found-long)
 	(should found-complex)))))
 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-ert-tests.patch --]
[-- Type: text/x-patch; name=0001-ert-tests.patch, Size: 559 bytes --]

diff --git a/test/lisp/emacs-lisp/ert-tests.el b/test/lisp/emacs-lisp/ert-tests.el
index 1aff73d66f6..4cb523d2ff1 100644
--- a/test/lisp/emacs-lisp/ert-tests.el
+++ b/test/lisp/emacs-lisp/ert-tests.el
@@ -569,7 +569,7 @@ ert-test-run-tests-batch
         (save-window-excursion
           (let ((case-fold-search nil)
                 (ert-batch-backtrace-right-margin nil)
-		(ert-batch-print-level 10)
+		(ert-batch-print-level 1)
 		(ert-batch-print-length 11))
             (ert-run-tests-batch
              `(member ,failing-test-1 ,failing-test-2)))))

bug#71988: 31.0.50; ert-test-run-tests-batch-expensive runs out of memory if previous tests failed

[Eli Zaretskii]

> Date: Mon, 08 Jul 2024 04:49:54 +0000
> From:  Pip Cet via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> This bug report is mostly to get a bug number to put in the patch :-).
> 
> The test `ert-test-run-tests-batch-expensive' overrides print settings
> and prints a full backtrace to a temporary buffer repeatedly. If a
> previous test, such as `ert-test-run-tests-batch', failed, this
> eventually runs out of memory even on a 64 GB system.
> 
> For example, one can add "(should nil)" to ert-test-run-tests-batch and
> run:
> 
> $ make -C test lisp/emacs-lisp/ert-tests
> 
> The problem is the ert--stats structure's tests vector is being printed,
> which recursively prints all previous tests' data.
> 
> A possible fix is to temporarily override cl-print-object for either
> vectors or the ert--stats object to print nothing.
> 
> Unfortunately, we don't appear to have an official mechanism for
> temporarily calling cl-defmethod, such as a generalized variable one can
> cl-letf to. The patch does that by running cl-defmethod inside a
> (cl-letf* (((symbol-function 'cl-print-object) (symbol-function
> 'cl-print-object))) ...) form. As future changes to cl-generic might
> break that, an additional test is added to make sure our binding is
> undone and die loudly if it hasn't been.
> 
> This also speeds up the test significantly and it might be possible to
> remove the :unstable tag.
> 
> 
> diff --git a/test/lisp/emacs-lisp/ert-tests.el b/test/lisp/emacs-lisp/ert-tests.el
> index 1aff73d66f6..2f8e7382bea 100644
> --- a/test/lisp/emacs-lisp/ert-tests.el
> +++ b/test/lisp/emacs-lisp/ert-tests.el
> @@ -583,6 +583,7 @@ ert-test-run-tests-batch
>  		   (setq found-long (string-match long-text msg)))
>  		 (unless found-complex
>  		   (setq found-complex (string-match complex-text msg))))
> +        (should nil)
>  	(should found-long)
>  	(should found-complex)))))

Stefan, any comments or suggestions?

bug#71988: 31.0.50; ert-test-run-tests-batch-expensive runs out of memory if previous tests failed

[Pip Cet via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

On Monday, July 8th, 2024 at 04:49, Pip Cet <pipcet@protonmail.com> wrote:
> This bug report is mostly to get a bug number to put in the patch :-).

Which I'm now attaching. Sorry about that.

Pip

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Bug-71988-Avoid-excessively-large-backtraces-in-ert-.patch --]
[-- Type: text/x-patch; name=0001-Bug-71988-Avoid-excessively-large-backtraces-in-ert-.patch, Size: 1902 bytes --]

From 8f7e9b50f0edd33f601fab0842c5b4a7e4398490 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@protonmail.com>
Date: Mon, 8 Jul 2024 19:50:16 +0000
Subject: [PATCH] (Bug#71988) Avoid excessively large backtraces in ert-tests

* test/lisp/emacs-lisp/ert-tests.el (ert-test-run-tests-batch-expensive):
Temporarily make `cl-print-object' not print out vectors while running
the test.
---
 test/lisp/emacs-lisp/ert-tests.el | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/test/lisp/emacs-lisp/ert-tests.el b/test/lisp/emacs-lisp/ert-tests.el
index 1aff73d66f6..4237d354f38 100644
--- a/test/lisp/emacs-lisp/ert-tests.el
+++ b/test/lisp/emacs-lisp/ert-tests.el
@@ -594,9 +594,13 @@ ert-test-run-tests-batch-expensive
 			 :body (lambda () (should (equal complex-list 1))))))
     (let ((ert-debug-on-error nil)
           messages)
-      (cl-letf* (((symbol-function 'message)
+      (cl-letf* (((symbol-function 'cl-print-object) (symbol-function 'cl-print-object))
+                 ((symbol-function 'message)
                   (lambda (format-string &rest args)
                     (push (apply #'format format-string args) messages))))
+        ;; don't print the ert--stats-tests vector, which would run out of
+        ;; memory if previous tests have failed.
+        (cl-defmethod cl-print-object ((object vector) stream))
         (save-window-excursion
           (let ((case-fold-search nil)
                 (ert-batch-backtrace-right-margin nil)
@@ -611,7 +615,9 @@ ert-test-run-tests-batch-expensive
 		 do
 		 (unless found-frame
 		   (setq found-frame (cl-search frame msg :test 'equal))))
-        (should found-frame)))))
+        (should found-frame))))
+  ;; ensure our temporary binding was undone.
+  (should (equal (cl-prin1-to-string [a]) "[a]")))
 
 (ert-deftest ert-test-special-operator-p ()
   (should (ert--special-operator-p 'if))
-- 
2.45.2