deepdiff icon indicating copy to clipboard operation
deepdiff copied to clipboard

Detecting different message in similar Exception objects

Open Mark90 opened this issue 7 years ago • 2 comments

Hi,

First of all, thanks for writing a great library. I have used it in a lot of testing and it saved me from so many bugs.

I just ran into a small oddity; it seems that when comparing Exception objects the 'message' attribute is not checked for differences. Consider this snippet:

>>> from deepdiff import DeepDiff
>>>
>>> e1 = KeyError()
>>> e2 = KeyError('foo')
>>> class ClassA(object):
...   a = 1
...   def __init__(self, b):
...     self.b = b
...
>>> t1 = ClassA('')
>>> t2 = ClassA('foo')
>>> DeepDiff(e1, e2, verbose_level=2)
{}
>>> DeepDiff(t1, t2, verbose_level=2)
{'values_changed': {'root.b': {'new_value': 'foo', 'old_value': ''}}}
>>> e1.message
''
>>> e2.message
'foo'
>>> t1.b
''
>>> t2.b
'foo'

Tested with DeepDiff 3.3.0, same behavior on Python 2.7 and 3.6

I noticed this PR should theoretically have fixed this, as the message attribute is in fact returned by dir():

>>> {i: getattr(e1, i) for i in dir(e1) if not (i.startswith('__') and i.endswith('__'))}
{'message': '', 'args': ()}
>>> {i: getattr(e2, i) for i in dir(e2) if not (i.startswith('__') and i.endswith('__'))}
{'message': 'foo', 'args': ('foo',)}

But apparently it doesn't, so perhaps we don't get into __search_obj at all, or something's off in __search_dict... Debugging is fun, but don't have a lot of time for it now. Just posting the issue here and will check it later. :)

Cheers, Mark

Mark90 avatar Aug 01 '18 08:08 Mark90

Hi @Mark90 Happy to hear you like DeepDiff! Thanks for bringing this up. I will investigate. Sep

seperman avatar Aug 02 '18 19:08 seperman

Fix for this particular problem (based on dev/ branch) reusing the solution by Max Rothman in 54c7267d:

$ git diff deepdiff/diff.py
diff --git a/deepdiff/diff.py b/deepdiff/diff.py
index 073dccd..d6850c8 100644
--- a/deepdiff/diff.py
+++ b/deepdiff/diff.py
@@ -763,8 +763,11 @@ class DeepDiff(ResultDict):
                 t1 = level.t1._asdict()
                 t2 = level.t2._asdict()
             else:
-                t1 = level.t1.__dict__
-                t2 = level.t2.__dict__
+                # Object comparison fix from 54c7267d
+                t1 = {i: getattr(level.t1, i) for i in dir(level.t1)
+                      if not (i.startswith('__') and i.endswith('__'))}
+                t2 = {i: getattr(level.t2, i) for i in dir(level.t2)
+                      if not (i.startswith('__') and i.endswith('__'))}
         except AttributeError:
             try:
                 t1 = self.__dict_from_slots(level.t1)

Testcases in tests/test_diff_text.py to verify

    def test_attribute_change_customclass(self):
        class MyClass:
            a = 1
            def __init__(self, b):
                self.b = b
        t1 = MyClass('foo')
        t2 = MyClass('bar')
        result = {
            'values_changed': {
                'root.b': {
                    'old_value': 'foo',
                    'new_value': 'bar',
                }
            }
        }
        self.assertEqual(DeepDiff(t1, t2), result)

    def test_attribute_change_on_exception(self):
        t1 = KeyError('foo')
        t2 = KeyError('bar')
        result = {
            'values_changed': {
                'root.args[0]': {
                    'old_value': 'foo',
                    'new_value': 'bar',
                }
            }
        }
        self.assertEqual(DeepDiff(t1, t2), result)

But there is one side-effect. The testcases making use of test_diff_text.get_custom_objects_add_and_remove reported it;

# With old __diff_obj implementation, using __dict__
# the ddiff at test_diff_text.test_custom_objects_add_and_remove_method, line 910
ddiff == {
    'attribute_added': {
        'root.method_b': <function DeepDiffTextTestCase.get_custom_object_with_added_removed_methods.<locals>.method_b at 0x0304E198>,
        'root.method_a': <function DeepDiffTextTestCase.get_custom_object_with_added_removed_methods.<locals>.method_c at 0x0304E228>
    }
}

# With new __diff_obj implementation, using dir()
# the ddiff at test_diff_text.test_custom_objects_add_and_remove_method, line 910
ddiff == {
    'type_changes': {
        'root.method_a': {
            'old_type': <class 'method'>,
            'new_type': <class 'function'>,
            'old_value': <bound method DeepDiffTextTestCase.get_custom_object_with_added_removed_methods.<locals>.ClassA.method_a of <test_diff_text.DeepDiffTextTestCase.get_custom_object_with_added_removed_methods.<locals>.ClassA object at 0x037961B0>>,
            'new_value': <function DeepDiffTextTestCase.get_custom_object_with_added_removed_methods.<locals>.method_c at 0x037AE228>
        }
    },
    'attribute_added': {
        'root.method_b': <function DeepDiffTextTestCase.get_custom_object_with_added_removed_methods.<locals>.method_b at 0x037AE198>
    }
}

Seeing as get_custom_object_with_added_removed_methods() does not add attribute method_a but changes its reference from the instance method to a different function, I'm inclined to think this is for the better. What do you think?

Mark90 avatar Aug 04 '18 14:08 Mark90