Add support for capturing all the arguments when there are multiple invocations

[shashankrnr32]

Hi @kaste

First of all, Thank you for this project. I have been using mockito-python for a while now and really like its features (and its similarity with Java framework).

I went through the issues in the repo and could not find one that is related to what my ask is. So creating this issue. If this was discussed before, please feel free to close it and link it as duplicate tagging the older issue.

Issue

Currently the ArgumentCaptor available in the matchers module allows capturing the method arguments. The limitation of this is that it just captures the last value when there are multiple invocations. For a usecase when a particular method is called multiple times with different values, its good to verify that the invocations were done with the correct arguments

Solution

There are 2 solutions I think of after going through the repo

1. Add a new class called MultipleInvocationsArgumentCaptor that does this job. I also have an implementation ready (along with tests) for the same in this branch. I am happy to raise this as a PR. (Also, feel free to choose a new name for this class if its too long. 😄 )

2. Modify the current ArgumentCaptor to take in multiple matchers and change certain aspects with this. This would be a backward incompatible change and I personally go with the 1st option keeping the current code as it is. But I added this option in case you want to maintain close similarity with Java framework, then this would be the preferred option.

As I said, I already have changes ready for the 1st solution I added(See branch). I will raise the PR once I get your response on the 1st solution. If you opt to go with the other option, I will be happy to work on it in my free time and raise a PR in a couple of days.

Thank you!

[kaste]

Hi 👋, do you have an example of where that is useful? If I remember correctly the captor version here (in Python) is already a bit different from the Java one. Don't think I ever used it.

[shashankrnr32]

Hey. Thanks for the response. Here is a very simple piece of code that I came up with just for the issue. In the actual code logic I called the subprocess.Popen 2 times with different set of arguments.

from typing import Union
from mockito import mock, when
from mockito.matchers import ArgumentCaptor

# A class from some Library (eg. requests, subprocess etc)
class TestClassFromSomeLibrary:

    def method(self, foo: Union[int, str]):
        return foo

# My Program Logic
def my_method(test: TestClassFromSomeLibrary):
    test.method("5")
    test.method(6)
    test.method(7)

# My Unittest
def testing():
    captor = ArgumentCaptor()
    test = mock()
    when(test).method(captor).thenReturn("mockedvalue")
    my_method(test)
    print(captor.value)
    # prints 7

With the current ArgumentCaptor, the last statement in my unittest prints 7 since that is the last invocation happening in my code logic. With a simple verify statement, I can assert that the method was called 2 times. But, with the current arg captor, its not possible to capture different arguments that were passed during multiple invocations of the method.

[shashankrnr32]

Modifying the above unittest to use MultipleInvocationsArgumentCaptor

def testing():
    captor = MultipleInvocationsArgumentCaptor()
    test = mock()
    when(test).method(captor).thenReturn("mockedvalue")
    my_method(test)
    print(captor.values)
    # prints ['5', 6, 7]

This will capture all arguments that were passed to the method in a list and arguments that were passed in individual invocation can be verified later on with a few assert statements.

[kaste]

But this can be done https://replit.com/join/vfilexscqa-kaste1 without any captor.

def testing():
    test = mock()
    when(test).method(...).thenReturn("mockedvalue")
    my_method(test)
    verify(test).method("5")
    verify(test).method(6)
    verify(test).method(7)

We want to avoid assert statements as long as possible. Maybe you have a different usage in mind? Can you provide a different example?

[shashankrnr32]

Hey. Sorry for replying a bit late on your comment. I particularly didn't know about the above usage. I used the captor approach to inspect the attributes and methods of a particular object passed to the method. I guess your approach works for simple objects like int, str, but is not compatible when passing objects.

Adding a small example here (Apologies for extending the same example with some modifications)

from mockito import mock, when
from mockito.matchers import ArgumentCaptor

class Dummy:
    a: int

# A class from some Library (eg. requests, subprocess etc)
class ClassFromSomeLibrary:
    def method(self, foo: Dummy):
        return foo

# My Program Logic
def program(test: ClassFromSomeLibrary):
    d1, d2 = Dummy(), Dummy()
    d1.a = 2
    d2.a = 10
    test.method(d1)
    test.method(d2)

# My Unittest
def testing():
    captor = ArgumentCaptor()
    test = mock()
    when(test).method(captor).thenReturn("mockedvalue")
    program(test)
    print(captor.value.a)
    # prints 10

In this case, I can verify the inner attributes(Dummy.a in this case) of the objects passed to the method. Since I am calling the method with 2 different object and 2 different internal attribute, it would be good to inspect both the invocations (along with the attributes of that object)

Also, please feel free to correct me if this can be done without an argument captor. I am happy to learn.

Thank you!

[kaste]

In testing you don't pass captor around or use it in any other way atm. I don't think the example works as is, why should it print 10. Maybe you can try the replit link I gave. You can edit the tests/41_test.py file over there.

[shashankrnr32]

Thats on me. My bad. I just edited the comment and also edited the file on replit. You should be able to view the right example code now.

[kaste]

Ok, that's basically how do I test untestable code. The takeaway from TDD was to write code that is easy to test, and this is a clear violation. The test does not inform the design and API.

I can't think this though completely but usually if there is a hidden call like Dummy() we mock this call: when(module).Dummy().thenReturn(d1_mock).thenReturn(d2_mock). We can then inspect and verify the usage of d1_mock as usual.

EDIT: Updated the repl

def testing():
    d1_mock = mock()
    d2_mock = mock()
    when(Module).Dummy().thenReturn(d1_mock).thenReturn(d2_mock)

    test = mock()
    program(test)
    assert d1_mock.a == 2
    assert d2_mock.a == 10

[shashankrnr32]

I completely agree that the example I mentioned in this issue is a very bad design of code. If I wanted to verify such calls made from a package (like requests, subprocess) which isn't controllable by me, then its impossible for me to inspect these inner attributes in the object which is kind of solved by ArgumentCaptor (which I think is the only option available here).

The point where the ArgCaptor fails is if there are multiple invocations made to the same method with different objects.

[kaste]

The typical option/solution is to mock out the Dummy() call as I have shown. You can control what Dummy() actually returns.

[kaste]

That's maybe like the https://mockito-python.readthedocs.io/en/latest/recipes.html#classes-as-factories example which is very dense; I usually have to read it twice to understand what we mock and how we pass that around.

[kaste]

It is also possible to use arg_that

def testing():
    test = mock()
    program(test)

    verify(test).method(arg_that(lambda d1: d1.a == 2))
    verify(test).method(arg_that(lambda d2: d2.a == 10))

This is short enough. Although I prefer something like this:

def testing():
    test = ClassFromSomeLibrary()
    expect(test).method(arg_that(lambda d1: d1.a == 2))
    expect(test).method(arg_that(lambda d2: d2.a == 10))
    program(test)

[shashankrnr32]

I like the arg_that way of doing it, although I prefer the captor only for the fact that I can use those captured arguments for some other testing in my method. I understand that I can probably put that in the same predicate that is passed to arg_that. I can use this for now (for the usecase I am looking at), but I still think having a captor that captures all the invocations is a good addition to the package so that I dont have to write the same line receptively if there are more number of invocations. (in a loop).

Feel free to close this issue. The arg_that approach looks promising.

Thank you very much for spending time on the issue and timely replies.

[kaste]

I now think we should add this feature. Our standard captor should just save all values in a member .values or .all_values, whereby .value always returns the latest value just like now. This way we can also tell when the captor did not capture anything yet. We could update repr to show all values. We could throw when accessing value if nothing has been captured yet. I don't want to allow different matchers. I think to have matchers here was a mistake.

[shashankrnr32]

Perfect. I will be probably working on this and raising a PR ASAP. The approach you said sounds right with keeping the matcher similar to the Java implementation. I will have an update soon.