Add support for lenient stubbed invocations

[Oracking]

Current behaviour: When we call verifyStubbedInvocationsAreUsed it checks that all stubbed invocations are called at least once, ensuring we don't have unnecessary stubbing. This is often useful to have in afterEach hook. However, there are cases where we want some specific mocks to be skipped by this check. Java mockito has lenient which allows a stubbed invocation to be set up without being called. This is useful in a case where we want to always mock some call in the beforeEach regardless of whether it is called in the test body (see example below). Currently, there isn't support for this.

class TestCase:
    def setUp(self):
        mockito.when("external_module").real_call_that_should_always_be_stubbed(...).thenReturn("Some response")

    def tearDown(self):
        mockito.verifyStubbedInvocationsAreUsed()

    def test_some_behaviour(self):
        # Do some stuff that don't call the stub

    def test_some_other_behaviour(self):
        # Do some stuff that call the stub

Proposed Behaviour: Thinking this can be solved in one of two ways:

1. Have an optional lenient argument added to when, when2, expect, patch. The actual name of the argument can be something other than lenient as we currently have strict and this can be confusing. Also Java Mockito's lenient argument seems to skip argument mismatch check so having the same name (but modified behaviour) could be misleading.
2. Relax the validity check on the atleast argument in expect so zero can be passed as a valid argument. Hence in beforeEach, user can specify mockito.expect("external_module", atleast=0).real_call_that_should_always_be_stubbed(...).thenReturn("Some response"). And this will still pass verifyStubbedInvocationsAreUsed in afterEach hook even if no calls are made to stubbed invocation in the test body.

I think Option 2 will be the least invasive but I'm keen to hear others thoughts.

I will also be happy to raise a PR with the changes required

[kaste]

I have something like this in the code: Look out for self.allow_zero_invocations whose value depends on

mockito-python/mockito/invocation.py

Lines 288 to 301 in 766ea5c

	def verification_has_lower_bound_of_zero(verification):
	if (
	isinstance(verification, verificationModule.Times)
	and verification.wanted_count == 0
	):
	return True
	if (
	isinstance(verification, verificationModule.Between)
	and verification.wanted_from == 0
	):
	return True
	return False

iirc when times=0 or between starts at 0, i.e. between=(0,3), the stubbed method counts always as used.

[kaste]

I forgot to ask

This is useful in a case where we want to always mock some call in the beforeEach regardless of whether it is called in the test body (see example below).

I see the example but why do we do this? Why stubbing when you don't use it? Is that just dev ergonomics?

[Oracking]

Oh great! Just checked and between can be used to achieve the desired behaviour. I had tried that previously but it did not work for a different reason (it had escaped me since I'm raising this issue a few days later).

To your question - "why do we do this?" - an example scenario will be:

Say I have a TestClass that has some test cases that test function_a. Inside function_a, it conditionally invokes function_b that sends an alert. And I want to guarantee that the real alert call is never made in a test context. In this case, it feels safer to plug the mock for function_b in the setUp method. But this means some test cases that call function_a will not necessarily invoke the mocked function_b. However, it is safer because anyone who extends the test class with new test cases doesn't have to remember to mock the external service call when developing, risking unintended alerts.

Also, the reason that between had not worked for me was because expect does not work when we pass the module name as a string. I see when resolves the module it receives using get_obj. However, the same is not being done for expect. Is this intentional?

[kaste]

Can't see anything why expect should be different here. Mistake? Usually just change it and see if some tests are failing. (I so rarely do the strings thing in python, I think.)

[Oracking]

Understood. I will make the change and check if the tests run fine. If all good I will raise a PR and close issue. Appreciate the quick responses :)

[kaste]

When we do something with the docs, it should be added to the docs of verifyStubbedInvocationsAreUsed. That alone makes it a lot more "findable". It also makes it clear where the tests for this should be located. (Okay, tests locations are wild in this repo. LOL)

Edit: We have a test for that exact use-case:

mockito-python/tests/instancemethods_test.py

Lines 303 to 313 in 22e0451

	class TestPassIfImplicitlyVerifiedViaExpect:
	@pytest.mark.parametrize('verification', [
	{'times': 0},
	{'between': [0, 3]}
	])
	def testPassIfImplicitlyVerified(self, verification):
	dog = mock()
	expect(dog, **verification).waggle().thenReturn('Sure')
	verifyStubbedInvocationsAreUsed(dog)

[kaste]

Just to note: we might get a change here. It seems that a bare expect(m).foo() (t.i. without any explicit verification/wanted argument) maybe should raise if unused. In other words, a bare expect should set atleast=1 or something semantically equivalent to that. An explicit expect(m, atleast=0).foo() would not be affected by this change.

Especially the form:

with expect(m).foo(...):
    ...

# Oh, the code did not use m.foo()

should raise (#55).