kaste / mockito-python

Mockito is a spying framework
MIT License
123 stars 12 forks source link

Ellipsis in arg raises ValueError #19

Closed mazmrini closed 6 years ago

mazmrini commented 6 years ago

I'm on mockito 1.1.0

File "mockito/signature.py" line 45 in match_signature
    if Ellipsis in args:
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

Here's some code that reproduces the bug:

import numpy as np
import unittest

from mockito import when, arg_that

class MakeMeFail:
    def run(self, array: np.ndarray) -> int:
        return 0

class TestMakeMeFail(unittest.TestCase):
    def setUp(self) -> None:
        self.__a_np_array = np.array([1, 2, 3])
        self.__make_me_fail = MakeMeFail()

    def test_this_one_fails(self) -> None:
        expected_value = -1029
        when(self.__make_me_fail)\
            .run(arg_that(lambda arg: np.array_equal(arg, self.__a_np_array)))\
            .thenReturn(expected_value)

        result = self.__make_me_fail.run(self.__a_np_array)

        self.assertEqual(expected_value, result)

    def test_it_is_caused_by_this(self) -> None:
        with self.assertRaises(ValueError):
            result = None in (self.__a_np_array, 1, 2)

    def test_this_one_works_because_kwargs_are_not_getting_the_Ellipsis_in_kwargs_check(self) -> None:
        expected_value = -1029
        when(self.__make_me_fail)\
            .run(array=arg_that(lambda arg: np.array_equal(arg, self.__a_np_array)))\
            .thenReturn(expected_value)

        result = self.__make_me_fail.run(array=self.__a_np_array)

        self.assertEqual(expected_value, result)

Love the module btw

kaste commented 6 years ago

Ah well, numpy is very special because it basically forms a DSL and basically overrides all magic hooks. But you already explained it in your post.

So, to recap: numpy changes the type of __eq__ to be List[any] -> List[bool].

In [13]: np.array([1,2,3]) == 1
Out[13]: array([ True, False, False])

And then it implements __bool__ to throw that ValueError.

In [14]: bool(np.array([1,2,3]) == 1)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-14-1dab1739581a> in <module>()
----> 1 bool(np.array([1,2,3]) == 1)

ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

I don't know a good solution here. For now you should bypass the signature checks amd set strict to False.

    def test_this_one_fails(self):
        expected_value = -1029
        when(self.__make_me_fail, strict=False)\  # <------------ 
            .run(arg_that(lambda arg: np.array_equal(arg, self.__a_np_array)))\
            .thenReturn(expected_value)

        result = self.__make_me_fail.run(self.__a_np_array)

        self.assertEqual(expected_value, result)

This passes on my computer.

kaste commented 6 years ago

Okay, the problematic code in mockito is x in args which roughly translates to any(k == x for k in args), we could probably switch to identity tests here without changing mockito's behavior, e.g. any(k is x for k in args).

We could try this approach and see how far we can go.

The other perspective here is that on call/run time we actually shouldn't expect Ellipsis or the other placeholders (kwargs, args), so this feels like a cheap implementation which probably causes this bug.

kaste commented 6 years ago

I'll make a release 1.1.1 with some fixes now.

Now, generally, your example code should work. You can make this a bit easier on the eyes by patching the actual compare function. Like so:

https://github.com/kaste/mockito-python/blob/42a3fd7cd6fa3c5a5b0d2ef07b7b6c8a84d91211/mockito/tests/numpy_test.py#L1-L30