bug#35018: 26.1; Use diff as en ert-explainer for string=

[Lars Ingebrigtsen]

Pierre Neidhardt <mail@ambrevar.xyz> writes:

> I've just committed webfeeder.el to ELPA.  In his review, Stefan Monnier
> suggested we merged the following code snippet upstream:

I've adapted this to ert to see how it would look, and...  I'm not sure
it's all that helpful as is.  The following test:

(ert-deftest test-test-test ()
  (should (string= "foo\nbar" "zot\nbar")))

then gives this output:

    (ert-test-failed
     ((should
       (string= "foo\12bar" "zot\12bar"))
      :form
      (string= "foo\12bar" "zot\12bar")
      :value nil :explanation "diff -u /tmp/webfeeder1kS4io 
/tmp/webfeederwYoKdx\12--- /tmp/webfeeder1kS4io\0112021-06-23 
15:37:54.407053381 +0200\12+++ /tmp/webfeederwYoKdx\0112021-06-23 
15:37:54.407053381 +0200\12@@ -1,2 +1,2 @@\12-foo\12+zot\12 bar\12\\ No newline 
at end of file\12\12Diff finished.  Wed Jun 23 15:37:54 2021\12"))

Which isn't all that nice.

But I guess we could massage the output.  Removing the header and
trailer, we get:

    (ert-test-failed
     ((should
       (string= "foo\12bar" "zot\12bar"))
      :form
      (string= "foo\12bar" "zot\12bar")
      :value nil :explanation "@@ -1,2 +1,2 @@\12-foo\12+zot\12 bar\12\\ "))

But...  that's still not exactly readable.

So I don't know.  Anybody got an opinion here, or an easy fix that'll
make this readable in this context?

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 6793b374ee..004bc81b4a 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -63,6 +63,7 @@
 (require 'ewoc)
 (require 'find-func)
 (require 'pp)
+(require 'diff)
 
 ;;; UI customization options.
 
@@ -535,6 +536,34 @@ ert--explain-equal
     (ert--explain-equal-rec a b)))
 (put 'equal 'ert-explainer 'ert--explain-equal)
 
+(defun ert--explain-string (string-a string-b)
+  "Return the diff output of STRING-A and STRING-B"
+  (unless (or (string= string-a string-b)
+              ;; Only do diffs if there are newlines.
+              (not (string-match-p "\n" string-a))
+              (not (string-match-p "\n" string-b)))
+    (let (file-a file-b)
+      (unwind-protect
+          (let (result)
+            (setq file-a (make-temp-file "ert")
+                  file-b (make-temp-file "ert"))
+            (with-temp-file file-a
+              (insert string-a))
+            (with-temp-file file-b
+              (insert string-b))
+            (setq result
+                  (with-temp-buffer
+                    ;; The following generates a *Diff* buffer which is
+                    ;; convenient for coloration.
+                    (diff-no-select file-a file-b nil 'no-async)
+                    (diff-no-select file-a file-b nil 'no-async
+                                    (current-buffer))
+                    (buffer-string)))
+            result)
+        (delete-file file-a)
+        (delete-file file-b)))))
+(put 'string= 'ert-explainer #'ert--explain-string)
+
 (defun ert--significant-plist-keys (plist)
   "Return the keys of PLIST that have non-null values, in order."
   (cl-assert (zerop (mod (length plist) 2)) t)


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no