add Message.equals(), and correctly check for equality in failing tests

hardbyte · hardbyte · Sep 28, 2018 · Sep 6, 2018 · Sep 6, 2018 · Sep 6, 2018
commit 196cd89d49cf72701c5cf581bb509b8c27e8736a
diff --git a/can/message.py b/can/message.py
@@ -10,7 +10,7 @@
 """
 
 from __future__ import absolute_import, division
-
+        
 import warnings
 
 
@@ -24,7 +24,7 @@ class Message(object):
     data and may be associated to a channel.
 
     Messages are always compared by identity and never by value, because that
-    may introduce unexpected behaviour.
+    may introduce unexpected behaviour. See also :meth:`~can.Message.equals`.
     Hashing uses all fields without exceptions.
     :func:`~copy.copy`/:func:`~copy.deepcopy` is supported as well.
 
@@ -298,3 +298,40 @@ def _check(self):
         if not self.is_fd:
             assert not self.bitrate_switch, "bitrate switch is only allowed for CAN FD frames"
             assert not self.error_state_indicator, "error stat indicator is only allowed for CAN FD frames"
+
+    def equals(self, other, timestamp_delta=1.0e-6):
+        """
+        Compares a given message with this one.
+
+        :param can.Message other: the message to compare with
+
+        :type timestamp_delta: float or int or None
+        :param timestamp_delta: the maximum difference at which two timestamps are
+                                still considered equal or None to not compare timestamps
+
+        :rtype: bool
+        :return: True iff the given message equals this one
+        """
+        # see https://github.com/hardbyte/python-can/pull/413 for a discussion
+        # on why a delta of 1.0e-6 was chosen
+        return (
+            # check for identity first
+            self is other or
+            # then check for equality by value
+            (
+                (
+                    timestamp_delta is None or
+                    abs(self.timestamp - other.timestamp) <= timestamp_delta
+                ) and
+                self.arbitration_id == other.arbitration_id and
+                self.is_extended_id == other.is_extended_id and
+                self.dlc == other.dlc and
+                self.data == other.data and
+                self.is_remote_frame == other.is_remote_frame and
+                self.is_error_frame == other.is_error_frame and
+                self.channel == other.channel and
+                self.is_fd == other.is_fd and
+                self.bitrate_switch == other.bitrate_switch and
+                self.error_state_indicator == other.error_state_indicator
+            )
+        )
diff --git a/test/logformats_test.py b/test/logformats_test.py
@@ -34,23 +34,23 @@
 from .data.example_data import TEST_MESSAGES_BASE, TEST_MESSAGES_REMOTE_FRAMES, \
                                TEST_MESSAGES_ERROR_FRAMES, TEST_COMMENTS, \
                                sort_messages
+from .message_helper import ComparingMessagesTestCase
 
 logging.basicConfig(level=logging.DEBUG)
 
 
-class ReaderWriterTest(unittest.TestCase):
+class ReaderWriterTest(unittest.TestCase, ComparingMessagesTestCase):
     """Tests a pair of writer and reader by writing all data first and
     then reading all data and checking if they could be reconstructed
     correctly. Optionally writes some comments as well.
-
     """
 
     __test__ = False
 
     __metaclass__ = ABCMeta
 
     def __init__(self, *args, **kwargs):
-        super(ReaderWriterTest, self).__init__(*args, **kwargs)
+        unittest.TestCase.__init__(self, *args, **kwargs)
         self._setup_instance()
 
     @abstractmethod
@@ -62,7 +62,7 @@ def _setup_instance_helper(self,
             writer_constructor, reader_constructor, binary_file=False,
             check_remote_frames=True, check_error_frames=True, check_fd=True,
             check_comments=False, test_append=False,
-            round_timestamps=False, preserves_timestamp_exact=True,
+            allowed_timestamp_delta=0.0,
             preserves_channel=True, adds_default_channel=None):
         """
         :param Callable writer_constructor: the constructor of the writer class
@@ -78,10 +78,7 @@ def _setup_instance_helper(self,
                                     but deterministically, which makes the test reproducible.
         :param bool test_append: tests the writer in append mode as well
 
-        :param bool round_timestamps: if True, rounds timestamps using :meth:`~builtin.round`
-                                      to integers before comparing
-        :param bool preserves_timestamp_exact: if True, checks that timestamps match exactly
-                                               in this case, no rounding is performed
+        :param float or int or None allowed_timestamp_delta: directly passed to :meth:`can.Message.equals`
         :param bool preserves_channel: if True, checks that the channel attribute is preserved
         :param any adds_default_channel: sets this as the channel when not other channel was given
                                          ignored, if *preserves_channel* is True
@@ -114,10 +111,10 @@ def _setup_instance_helper(self,
         self.binary_file = binary_file
         self.test_append_enabled = test_append
 
-        self.round_timestamps = round_timestamps
-        self.preserves_timestamp_exact = preserves_timestamp_exact
-        self.preserves_channel = preserves_channel
-        self.adds_default_channel = adds_default_channel
+        ComparingMessagesTestCase.__init__(self,
+            allowed_timestamp_delta=allowed_timestamp_delta,
+            preserves_channel=preserves_channel)
+            #adds_default_channel=adds_default_channel # TODO inlcude in tests
 
     def setUp(self):
         with tempfile.NamedTemporaryFile('w+', delete=False) as test_file:
@@ -300,47 +297,8 @@ def assertMessagesEqual(self, messages_1, messages_2):
         """
         self.assertEqual(len(messages_1), len(messages_2))
 
-        for index, (message_1, message_2) in enumerate(zip(messages_1, messages_2)):
-            try:
-                self.assertMessageEqual(message_1, message_2)
-            except AssertionError as e:
-                print("Comparing: message 1: {!r}".format(message_1))
-                print("           message 2: {!r}".format(message_2))
-                self.fail("messages are not equal at index #{}:\n{}".format(index, e))
-
-    def assertMessageEqual(self, message_1, message_2):
-        """
-        Checks that two messages are equal, according to the current rules.
-        """
-        # try conventional
-        if message_1 == message_2:
-            return
-
-        # check the timestamp
-        if self.preserves_timestamp_exact:
-            self.assertEqual(message_1.timestamp, message_2.timestamp)
-        else:
-            t1 = round(message_1.timestamp) if self.round_timestamps else message_1.timestamp
-            t2 = round(message_2.timestamp) if self.round_timestamps else message_2.timestamp
-            self.assertAlmostEqual(message_2.timestamp, message_1.timestamp, delta=1e-6,
-                msg="message timestamps are not almost_equal ({!r} !~= {!r}{})"
-                    .format(message_1.timestamp, message_2.timestamp,
-                            "; rounded" if self.round_timestamps else ""))
-
-        # check the rest
-        self.assertEqual(message_1.arbitration_id, message_2.arbitration_id)
-        self.assertEqual(message_1.is_extended_id, message_2.is_extended_id)
-        self.assertEqual(message_1.is_remote_frame, message_2.is_remote_frame)
-        self.assertEqual(message_1.is_error_frame, message_2.is_error_frame)
-        if self.preserves_channel:
-            self.assertEqual(message_1.channel, message_2.channel)
-        else:
-            self.assertEqual(message_2.channel, self.adds_default_channel)
-        self.assertEqual(message_1.dlc, message_2.dlc)
-        self.assertEqual(message_1.data, message_2.data)
-        self.assertEqual(message_1.is_fd, message_2.is_fd)
-        self.assertEqual(message_1.bitrate_switch, message_2.bitrate_switch)
-        self.assertEqual(message_1.error_state_indicator, message_2.error_state_indicator)
+        for message_1, message_2 in zip(messages_1, messages_2):
+            self.assertMessageEqual(message_1, message_2)
 
     def assertIncludesComments(self, filename):
         """
@@ -367,7 +325,6 @@ def _setup_instance(self):
             can.ASCWriter, can.ASCReader,
             check_fd=False,
             check_comments=True,
-            #round_timestamps=True, # TODO is this required?
             preserves_channel=False, adds_default_channel=0
         )
 
@@ -383,7 +340,7 @@ def _setup_instance(self):
             binary_file=True,
             check_fd=False,
             check_comments=False,
-            preserves_timestamp_exact=False,
+            allowed_timestamp_delta=1.0e-6,
             preserves_channel=False, adds_default_channel=0
         )
 
@@ -394,10 +351,12 @@ def test_read_known_file(self):
 
         expected = [
             can.Message(
+                timestamp=1.0,
                 extended_id=False,
                 arbitration_id=0x64,
                 data=[0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8]),
             can.Message(
+                timestamp=73.0,
                 extended_id=True,
                 arbitration_id=0x1FFFFFFF,
                 is_error_frame=True,)

diff --git a/test/message_helper.py b/test/message_helper.py
@@ -0,0 +1,41 @@
+#!/usr/bin/env python
+# coding: utf-8
+
+from __future__ import absolute_import, print_function
+
+from copy import copy
+
+
+class ComparingMessagesTestCase(object):
+    """Must be extended by a class also extending a unittest.TestCase.
+    """
+
+    def __init__(self, allowed_timestamp_delta=0.0, preserves_channel=True):
+        """
+        :param float or int or None allowed_timestamp_delta: directly passed to :meth:`can.Message.equals`
+        :param bool preserves_channel: if True, checks that the channel attribute is preserved
+        """
+        self.allowed_timestamp_delta = allowed_timestamp_delta
+        self.preserves_channel = preserves_channel
+
+    def assertMessageEqual(self, message_1, message_2):
+        """
+        Checks that two messages are equal, according to the given rules.
+        """
+
+        if message_1.equals(message_2, timestamp_delta=self.allowed_timestamp_delta):
+            return
+        elif self.preserves_channel:
+            print("Comparing: message 1: {!r}".format(message_1))
+            print("           message 2: {!r}".format(message_2))
+            self.fail("messages are unequal with allowed timestamp delta {}".format(self.allowed_timestamp_delta))
+        else:
+            message_2 = copy(message_2) # make sure this method is pure
+            message_2.channel = message_1.channel
+            if message_1.equals(message_2, timestamp_delta=self.allowed_timestamp_delta):
+                return
+            else:
+                print("Comparing: message 1: {!r}".format(message_1))
+                print("           message 2: {!r}".format(message_2))
+                self.fail("messages are unequal with allowed timestamp delta {} even when ignoring channels" \
+                          .format(self.allowed_timestamp_delta))
diff --git a/test/serial_test.py b/test/serial_test.py
@@ -7,14 +7,18 @@
 Copyright: 2017 Boris Wenzlaff
 """
 
+from __future__ import division
+
 import unittest
 from mock import patch
 
 import can
 from can.interfaces.serial.serial_can import SerialBus
 
+from .message_helper import ComparingMessagesTestCase
+
 
-class SerialDummy:
+class SerialDummy(object):
     """
     Dummy to mock the serial communication
     """
@@ -36,9 +40,13 @@ def reset(self):
         self.msg = None
 
 
-class SimpleSerialTestBase(object):
+class SimpleSerialTestBase(ComparingMessagesTestCase):
+
     MAX_TIMESTAMP = 0xFFFFFFFF / 1000
 
+    def __init__(self):
+        ComparingMessagesTestCase.__init__(self, allowed_timestamp_delta=None, preserves_channel=True)
+
     def test_rx_tx_min_max_data(self):
         """
         Tests the transfer from 0x00 to 0xFF for a 1 byte payload
@@ -47,7 +55,7 @@ def test_rx_tx_min_max_data(self):
             msg = can.Message(data=[b])
             self.bus.send(msg)
             msg_receive = self.bus.recv()
-            self.assertEqual(msg, msg_receive)
+            self.assertMessageEqual(msg, msg_receive)
 
     def test_rx_tx_min_max_dlc(self):
         """
@@ -59,7 +67,7 @@ def test_rx_tx_min_max_dlc(self):
             msg = can.Message(data=payload)
             self.bus.send(msg)
             msg_receive = self.bus.recv()
-            self.assertEqual(msg, msg_receive)
+            self.assertMessageEqual(msg, msg_receive)
 
     def test_rx_tx_data_none(self):
         """
@@ -68,7 +76,7 @@ def test_rx_tx_data_none(self):
         msg = can.Message(data=None)
         self.bus.send(msg)
         msg_receive = self.bus.recv()
-        self.assertEqual(msg, msg_receive)
+        self.assertMessageEqual(msg, msg_receive)
 
     def test_rx_tx_min_id(self):
         """
@@ -77,7 +85,7 @@ def test_rx_tx_min_id(self):
         msg = can.Message(arbitration_id=0)
         self.bus.send(msg)
         msg_receive = self.bus.recv()
-        self.assertEqual(msg, msg_receive)
+        self.assertMessageEqual(msg, msg_receive)
 
     def test_rx_tx_max_id(self):
         """
@@ -86,7 +94,7 @@ def test_rx_tx_max_id(self):
         msg = can.Message(arbitration_id=536870911)
         self.bus.send(msg)
         msg_receive = self.bus.recv()
-        self.assertEqual(msg, msg_receive)
+        self.assertMessageEqual(msg, msg_receive)
 
     def test_rx_tx_max_timestamp(self):
         """
@@ -96,7 +104,7 @@ def test_rx_tx_max_timestamp(self):
         msg = can.Message(timestamp=self.MAX_TIMESTAMP)
         self.bus.send(msg)
         msg_receive = self.bus.recv()
-        self.assertEqual(msg, msg_receive)
+        self.assertMessageEqual(msg, msg_receive)
         self.assertEqual(msg.timestamp, msg_receive.timestamp)
 
     def test_rx_tx_max_timestamp_error(self):
@@ -113,7 +121,7 @@ def test_rx_tx_min_timestamp(self):
         msg = can.Message(timestamp=0)
         self.bus.send(msg)
         msg_receive = self.bus.recv()
-        self.assertEqual(msg, msg_receive)
+        self.assertMessageEqual(msg, msg_receive)
         self.assertEqual(msg.timestamp, msg_receive.timestamp)
 
     def test_rx_tx_min_timestamp_error(self):
@@ -126,6 +134,10 @@ def test_rx_tx_min_timestamp_error(self):
 
 class SimpleSerialTest(unittest.TestCase, SimpleSerialTestBase):
 
+    def __init__(self, *args, **kwargs):
+        unittest.TestCase.__init__(self, *args, **kwargs)
+        SimpleSerialTestBase.__init__(self)
+
     def setUp(self):
         self.patcher = patch('serial.Serial')
         self.mock_serial = self.patcher.start()
@@ -141,6 +153,10 @@ def tearDown(self):
 
 class SimpleSerialLoopTest(unittest.TestCase, SimpleSerialTestBase):
 
+    def __init__(self, *args, **kwargs):
+        unittest.TestCase.__init__(self, *args, **kwargs)
+        SimpleSerialTestBase.__init__(self)
+
     def setUp(self):
         self.bus = SerialBus('loop://')
 

diff --git a/test/simplecyclic_test.py b/test/simplecyclic_test.py
@@ -13,8 +13,14 @@
 import can
 
 from .config import *
+from .message_helper import ComparingMessagesTestCase
 
-class SimpleCyclicSendTaskTest(unittest.TestCase):
+
+class SimpleCyclicSendTaskTest(unittest.TestCase, ComparingMessagesTestCase):
+
+    def __init__(self, *args, **kwargs):
+        unittest.TestCase.__init__(self, *args, **kwargs)
+        ComparingMessagesTestCase.__init__(self, allowed_timestamp_delta=None, preserves_channel=True)
 
     @unittest.skipIf(IS_CI, "the timing sensitive behaviour cannot be reproduced reliably on a CI server")
     def test_cycle_time(self):
@@ -30,7 +36,7 @@ def test_cycle_time(self):
         self.assertTrue(80 <= size <= 120,
                         '100 +/- 20 messages should have been transmitted. But queue contained {}'.format(size))
         last_msg = bus2.recv()
-        self.assertEqual(last_msg, msg)
+        self.assertMessageEqual(last_msg, msg)
 
         bus1.shutdown()
         bus2.shutdown()