8000 Why does a stubOnly mock keeps reference of the last invocation? · Issue #348 · mockito/mockito · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Why does a stubOnly mock keeps reference of the last invocation? #348

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.

Sign up for GitHub

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

Jump to bottom
Closed
romainpiel opened this issue Feb 2, 2016 · 5 comments
Closed

Why does a stubOnly mock keeps reference of the last invocation? #348

romainpiel opened this issue Feb 2, 2016 · 5 comments
Labels
invalid

Comments

@romainpiel
Copy link

I was looking at a memory leak caused by a mock in our tests and I noticed that creating a mock this way

Foo foo = mock(Foo.class, withSettings().stubOnly());

will still keep the last invocation in memory:

mockito/src/main/java/org/mockito/internal/stubbing/InvocationContainerImpl.java

Line 141 in c303e49

return mockSettings.isStubOnly()

mockito/src/main/java/org/mockito/internal/verification/SingleRegisteredInvocation.java

Line 16 in c303e49

private Invocation invocation;

I was wondering if that was on purpose? I thought the whole point of stubOnly() was to prevent allocating too much memory for nothing by preventing verify() on a mock. I think something like that would make more sense:

public class EmptyRegisteredInvocation implements RegisteredInvocations, Serializable {

    public void add(Invocation invocation) {
        // do nothing
    }

    public void removeLast() {
        // do nothing
    }

    public List<Invocation> getAll() {
        return Collections.emptyList();
    }

    public boolean isEmpty() {
        return true;
    }
}
@TimvdLippe
Copy link
Contributor

This seems reasonable. I don't think we do this for Mockito 2.0, but 2.1 instead. It does require an investigation if this is backwards incompatible though.

@TimvdLippe TimvdLippe added this to the 2.1 milestone Aug 21, 2016
@TimvdLippe TimvdLippe added the bug label Aug 21, 2016
@bric3
Copy link
Contributor
bric3 commented Aug 24, 2016

This is not a bug, otherwise mockito can't record the stub itself.

@bric3 bric3 closed this as completed Aug 24, 2016
@bric3 bric3 removed this from the 2.1 milestone Aug 24, 2016
@davidburstromspotify
Copy link

@bric3 If you don't mind, could you elaborate on this (5 years later)? I'm also facing a memory leak like this and found this ticket.

bric3 added a commit to bric3/mockito that referenced this issue Feb 18, 2022
@bric3
Fixes mockito#348 In stub mode even the last invocation is irrelevant
3ae0ad1
@bric3
Copy link
Contributor
bric3 commented Feb 18, 2022 •
edited
Loading

I wonder why I did say that back then, this incorrect. 🧐
Nevertheless, indeed I believe SingleRegisteredInvocation should be replaced by an empty invocation variant.

@bric3 bric3 mentioned this issue Feb 18, 2022
Draft
8 tasks
bric3 added a commit to bric3/mockito that referenced this issue Feb 18, 2022
@bric3
Fixes mockito#348 In stub mode even the last invocation is irrelevant
e50823b
@davidburstromspotify
Copy link
davidburstromspotify commented Feb 19, 2022 •
edited
Loading

Yeah, that looks like what I'd expect too. Considering there are test cases that fail, I guess there's a risk that some projects accidentally use the incorrect behaviour when pure stubs are used. In that case, I suppose the migration is easy: Don't use stubOnly. ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
invalid
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@romainpiel @bric3 @davidburstromspotify @TimvdLippe
0