Subject: bug#70093: Remove unnecessary `image-tests--map-equal'

From: Joseph Turner <[email-masked]>
Date: Sat, 30 Mar 2024 22:32:26 -0700

Eli Zaretskii <[email-masked]> writes:

>> Cc: Eli Zaretskii <[email-masked]>, David Ponce <[email-masked]>
>> From: Joseph Turner <[email-masked]>
>> Date: Sat, 30 Mar 2024 12:44:14 -0700
>>
>> With the resolution of bug#69992, it may be possible to remove the
>> `image-tests--map-equal' equality predicate from lisp/image-tests.el.
>
> Why is it a good idea to remove tests?  If the test tests some of our
> code which should work as the test assumes, we should not remove it.

Not removing tests, but rather making the tests more strict (if possible).

>> Eli and David, would you please apply the attached patch and then run
>> the tests to see if we still need to allow for tolerances in
>> `image--compute-map-and-original-map'?
>
> ENOPATCH

Sorry about that.  Please check if the tests still pass on your machine
when we use `equal' to compare image maps.

>From e85fd88a2943fa95d2d7eea3a542308adaa81582 Mon Sep 17 00:00:00 2001
From: Joseph Turner <[email-masked]>
Date: Sat, 30 Mar 2024 12:38:52 -0700
Subject: [PATCH] Compare image maps with `equal' in tests

With the fixes to image map transformation introduced in cc212ea314d,
the tests should now exactly predict calculated image maps.

* test/lisp/image-tests.el (image-tests--map-equal): Remove function.
(image--compute-map-and-original-map): Use equal to compare image maps.
---
 test/lisp/image-tests.el | 46 +++++-----------------------------------
 1 file changed, 5 insertions(+), 41 deletions(-)

diff --git a/test/lisp/image-tests.el b/test/lisp/image-tests.el
index 020781eff50..41b87b8d337 100644
--- a/test/lisp/image-tests.el
+++ b/test/lisp/image-tests.el
@@ -178,37 +178,6 @@ image-create-image-with-map
            (got-original-map (image-property image :original-map)))
       (should (equal got-original-map original-map-other)))))
 
-(defun image-tests--map-equal (a b &optional tolerance)
-  "Return t if maps A and B have the same coordinates within TOLERANCE.
-Since image sizes calculations vary on different machines, this function
-allows for each image map coordinate in A to be within TOLERANCE to the
-corresponding coordinate in B.  When nil, TOLERANCE defaults to 5."
-  (unless tolerance (setq tolerance 5))
-  (catch 'different
-    (cl-labels ((check-tolerance
-                  (coord-a coord-b)
-                  (unless (>= tolerance (abs (- coord-a coord-b)))
-                    (throw 'different nil))))
-      (dotimes (i (length a))
-        (pcase-let ((`((,type-a . ,coords-a) ,_id ,_plist) (nth i a))
-                    (`((,type-b . ,coords-b) ,_id ,_plist) (nth i b)))
-          (unless (eq type-a type-b)
-            (throw 'different nil))
-          (pcase-exhaustive type-a
-            ('rect
-             (check-tolerance (caar coords-a) (caar coords-b))
-             (check-tolerance (cdar coords-a) (cdar coords-b))
-             (check-tolerance (cadr coords-a) (cadr coords-b))
-             (check-tolerance (cddr coords-a) (cddr coords-b)))
-            ('circle
-             (check-tolerance (caar coords-a) (caar coords-b))
-             (check-tolerance (cdar coords-a) (cdar coords-b))
-             (check-tolerance (cdar coords-a) (cdar coords-b)))
-            ('poly
-             (dotimes (i (length coords-a))
-               (check-tolerance (aref coords-a i) (aref coords-b i))))))))
-    t))
-
 (ert-deftest image--compute-map-and-original-map ()
   "Test `image--compute-map' and `image--compute-original-map'."
   (skip-unless (display-images-p))
@@ -237,25 +206,20 @@ image--compute-map-and-original-map
                               :scale 2 :rotation 90 :flip t)))
     ;; Test that `image--compute-original-map' correctly generates
     ;; original-map when creating an already transformed image.
-    (should (image-tests--map-equal (image-property image :original-map)
-                                    original-map))
+    (should (equal (image-property image :original-map) original-map))
     (setf (image-property image :flip) nil)
     (setf (image-property image :rotation) 0)
     (setf (image-property image :scale) 2)
-    (should (image-tests--map-equal (image--compute-map image)
-                                    scaled-map))
+    (should (equal (image--compute-map image) scaled-map))
     (setf (image-property image :scale) 1)
     (setf (image-property image :rotation) 90)
-    (should (image-tests--map-equal (image--compute-map image)
-                                    rotated-map))
+    (should (equal (image--compute-map image) rotated-map))
     (setf (image-property image :rotation) 0)
     (setf (image-property image :flip) t)
-    (should (image-tests--map-equal (image--compute-map image)
-                                    flipped-map))
+    (should (equal (image--compute-map image) flipped-map))
     (setf (image-property image :scale) 2)
     (setf (image-property image :rotation) 90)
-    (should (image-tests--map-equal (image--compute-map image)
-                                    scaled-rotated-flipped-map))
+    (should (equal (image--compute-map image) scaled-rotated-flipped-map))
 
     ;; Uncomment to test manually by interactively transforming the
     ;; image and checking the map boundaries by hovering them.
-- 
2.41.0