Skip to content

Detecting different message in similar Exception objects #109

@Mark90

Description

@Mark90

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

Activity

seperman

seperman commented on Aug 2, 2018

@seperman
Owner

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

self-assigned this
on Aug 2, 2018
added this to the V3.5 milestone on Aug 2, 2018
Mark90

Mark90 commented on Aug 4, 2018

@Mark90
Author

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

$ 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?

removed this from the V3.5 milestone on Mar 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    No branches or pull requests

      Participants

      @Mark90@seperman

      Issue actions

        Detecting different message in similar Exception objects · Issue #109 · seperman/deepdiff