pytest-dev / pytest

The pytest framework makes it easy to write small tests, yet scales to support complex functional testing
https://pytest.org
MIT License
12.09k stars 2.68k forks source link

Assertion error message compares arrays that aren't compared by the underlying client code #8930

Open dam5h opened 3 years ago

dam5h commented 3 years ago

Pytest is an amazing library, thank you all for giving this code and your time to the community!

We came across an interesting issue recently where we were getting an error message about comparing arrays, even though we had been intentionally avoiding doing so.

When using an attrs class that has numpy array attributes, I use a custom __eq__ method to avoid directly comparing arrays, and uses np.allclose() instead. However when the equality assertion fails due to a difference in attributes unrelated to the array, pytest is comparing the arrays in order to generate a helpful message. But that comparison fails since it's using a direct comparison under the covers (instead of the np.allclose approach).

Example (the arrays are the same, but the y attribute is different):

import attr
import numpy as np

@attr.define
class Data:
    x: int
    y: int
    a: np.ndarray

    def __eq__(self, o):
        return (self.x, self.y) == (o.x, o.y) and np.allclose(self.a, o.a)

def test():
    d1 = Data(1, 2, np.array([2, 3]))
    d2 = Data(1, 3, np.array([2, 3]))
    assert np.allclose(d1.a, d2.a)
    assert d1 == d2

Output:

    def test():
        d1 = Data(1, 2, np.array([2, 3]))
        d2 = Data(1, 3, np.array([2, 3]))
        assert np.allclose(d1.a, d2.a)
>       assert d1 == d2
E       assert Data(x=1, y=2...array([2, 3])) == Data(x=1, y=3...array([2, 3]))
E         (pytest_assertion plugin: representation of details failed: 
E .pyenv/versions/3.9.6/envs/edgar-3.9/lib/python3.9/site-packages/_pytest/assertion/util.py:430: 
E ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all().
E          Probably an object has a faulty __repr__.)

scratch/test.py:19: AssertionError

Python version: 3.9.6 OS: Linux pytest version: 6.2.4

nicoddemus commented 3 years ago

Hi @dam5h 👋

Indeed that is very confusing, thanks for reporting it.

Another example where the message is confusing:

import attr

@attr.define
class Data:
    x: int
    y: int

    def __eq__(self, o):
        return False

def test():
    d1 = Data(1, 2)
    d2 = Data(1, 3)
    assert d1 == d2
    def test():
        d1 = Data(1, 2)
        d2 = Data(1, 3)
>       assert d1 == d2
E       AssertionError: assert Data(x=1, y=2) == Data(x=1, y=3)
E
E         Omitting 1 identical items, use -vv to show
E         Differing attributes:
E         ['y']
E
E         Drill down into differing attribute y:
E           y: 2 != 3

Here our diff shows a message stating that d1 is different from d2 because of y, which is not the problem that caused the assertion to fail.

If all attributes are equal, the message is even more confusing:

    def test():
        d1 = Data(1, 2)
        d2 = Data(1, 2)
>       assert d1 == d2
E       assert Data(x=1, y=2) == Data(x=1, y=2)
E
E         Omitting 2 identical items, use -vv to show

Not sure what is the ideal solution here... perhaps we should give up on trying to provide a more rich assertion diff if the class has a custom __eq__? After all that eq might customize the comparison completely, not even comparing members, leading to a confusing messages.

dam5h commented 3 years ago

Howdy @nicoddemus :wave: !

So yes, this is all pretty confusing and clearly comes down to the custom __eq__ bit. A message that simply indicates there is a custom __eq__ and for the user to check there, would likely be more helpful than what is happening now, at least for these edge cases illustrated here.

nicoddemus commented 3 years ago

I think we can easily detect the custom __eq__ if the user uses @define(eq=False) in the class declaration, otherwise it is impossible for pytest to tell if that __eq__ method is generated by attrs or custom made.

So indeed skipping the rich assertion seems like a good compromise in this case.

nicoddemus commented 3 years ago

I think we can easily detect the custom eq

Actually I couldn't find how to do it. 😕

Opened an issue in attrs to see if they have some suggestion: https://github.com/python-attrs/attrs/issues/834

nicoddemus commented 3 years ago

Actually there was an issue related to this already which I missed: https://github.com/python-attrs/attrs/issues/602.

So unfortunately it doesn't seem to be a way to detect this for attrs atm, but we can detect for @dataclass at least.