Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect unnecessary stubbings #558

Closed
Raibaz opened this issue Jan 11, 2021 · 8 comments
Closed

Detect unnecessary stubbings #558

Raibaz opened this issue Jan 11, 2021 · 8 comments

Comments

@Raibaz
Copy link
Collaborator

Raibaz commented Jan 11, 2021

Mockito throws an exception when a function is stubbed and then not actually invoked.

This is particularly useful when doing TDD, or generally when writing tests first, because it allows to stub a function, have the test fail because it was not invoked, and then make it pass by implementing the invocation.

We could add a parameter to the mockk() function and the @MockK annotation, or a configuration flag to make it global, so that it works somewhat like this:

class Test {
    @MockK(detectUnnecessaryStubbing = true)
    private lateinit var collaborator: Collaborator

    private lateinit var service: Service

    @BeforeEach
    fun setup() {
        MockKAnnotations.init(this) // Perhaps this could be another good place to enable the "unnecessary stubbing detection mode"

        service = Service(collaborator)
    }

    @Test
    fun test() {
        every { collaborator.function() } returns "foo"

        service.anotherFunction() // Not executing collaborator.function in here will fail
    }
}

A potential issue I see in this is the case when there are two tests executing service.anotherFunction(), one stubbing and invoking collaborator.function() and one not stubbing nor invoking it.

We need to find a way to detect that it's ok for the second test not to invoke collaborator.function() even if it was stubbed in another test.

@xpepper
Copy link

xpepper commented Jan 12, 2021

Thanks @Raibaz for opening this issue.
I think this is a feature that people migrating to MockK from Mockito or jMock may expect to find here too, because it's a standard behaviour in those tools (they call it "strict mode", which is enabled by default), see for example https://www.baeldung.com/mockito-unnecessary-stubbing-exception or http://blog.mockito.org/2017/01/clean-tests-produce-clean-code-strict.html.

@wnuccio
Copy link

wnuccio commented Jan 12, 2021

It would be great to have a default value of 'detectUnnecessaryStubbing' set to true, so to avoid setting it everywhere.
The most frequent case is that when a mock is configured it has to be invoked. In case there are two tests that require two different behaviours for the mock, better to create two different mocks.

@Raibaz
Copy link
Collaborator Author

Raibaz commented Feb 7, 2021

In the end, this is basically about having [verification confirmation] executed by default.

Perhaps it may be just about having the MockkExtension implement AfterEachCallback (see here) and call confirmVerified if necessary.

@Raibaz
Copy link
Collaborator Author

Raibaz commented Feb 7, 2021

Turns out this is actually a duplicate of #334.

@Raibaz Raibaz closed this as completed Feb 7, 2021
@xpepper
Copy link

xpepper commented Feb 8, 2021

@Raibaz so, is there already a solution to this issue? Thanks!

@Raibaz
Copy link
Collaborator Author

Raibaz commented Feb 8, 2021

Not yet, but as it turns out implementing it is likely going to be a bit easier than I thought :)

@xpepper
Copy link

xpepper commented Feb 8, 2021

Great, I'm willing to test a version of MockK with this new behaviour as soon as is available!

@Raibaz
Copy link
Collaborator Author

Raibaz commented May 11, 2022

Implemented in 1.12.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants