mockito / mockito

Most popular Mocking framework for unit tests written in Java
http://mockito.org
MIT License
14.81k stars 2.55k forks source link

Verification using After and ArgumentCaptor returns unexpected size of captured values list #345

Open tarnowskijan opened 8 years ago

tarnowskijan commented 8 years ago

Hi, I found weird behavior of Mockito (ver. 1.10.19) when verifying method invocation with after() and ArgumentCaptor. The list of captured values returned by ArgumentCaptor has size much bigger than I expect. Moreover this behavior in connection with long timeout leads to exceeding memory limit. Example below. Is it expected behavior or bug?

package org.mockito.test;

import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;

import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.after;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;

public class MockitoAfterFeatureTest {

    private RequestSender requestSender;
    private RequestProcessor requestProcessor;
    private ArgumentCaptor<String> stringArgumentCaptor;

    @Before
    public void doBeforeTest() {
        requestSender = mock(RequestSender.class);
        requestProcessor = new RequestProcessor(requestSender);
        stringArgumentCaptor = ArgumentCaptor.forClass(String.class);
    }

    @Test
    public void shouldVerifyUsingAfterAndArgumentCaptor() {
        // when
        requestProcessor.process("first_request");
        requestProcessor.process("second_request");

        // then
        verify(requestSender, after(2500).times(2)).send(stringArgumentCaptor.capture());
        assertEquals("first_request", stringArgumentCaptor.getAllValues().get(0));
        assertEquals("second_request", stringArgumentCaptor.getAllValues().get(1));

        assertEquals(2, stringArgumentCaptor.getAllValues().size());
    }

    public static class RequestProcessor {
        private final RequestSender requestSender;

        public RequestProcessor(RequestSender requestSender) {
            this.requestSender = requestSender;
        }

        public void process(String request) {
            requestSender.send(request);
        }
    }

    public interface RequestSender {
        void send(String request);
    }
}
thesnowgoose commented 8 years ago

Issue #345 resolves this for atMost verification. What about other verification modes (i.e. times, atLeast)? Are you working on that, @tarnowskijan? Was the issue only for the atMost case? Because in the code from the description you used times instead of atMost.

tarnowskijan commented 8 years ago

@thesnowgoose, unfortunately you're right, the issue is still there when you use times() in combination with after(). I only fixed the problem with after() and atMost() and I don't know why I was focused only on this case. I'll try to fix the whole issue in the near future, but feel free to propose your own solution if you already have one.

thesnowgoose commented 8 years ago

Actually I'm working on it. There is also an issue when you use atLeast(), I'll try to fix it too. Regards!!. @tarnowskijan

jmborer commented 8 years ago

Personally, I use extensively timeout and after in my tests, because their use is significantly different. Replace those by a single await can seem interesting at a first glance, but it is actually not.

bric3 commented 8 years ago

@jmborer Could you elaborate ?

jmborer commented 8 years ago

Well I have to test in a lot of asynch situations. Sometimes I need to wait for a interaction, continue quickly if it occurs before a given amount of time and fail otherwise. There I use timeout(). Then I have situations where I want to wait a given time and THEN only do the check. This is where I use after() instead of Thread.sleep()+verify() which I find less fluent to read...

bric3 commented 8 years ago

@jmborer Thanks for the feedback I linked your comment in #472

ChristianSchwarz commented 7 years ago

will be fixed with #936 aka within(duration,timeunit)

cschwier commented 6 years ago

Will this be fixed? Mockito 2.19.0 still has this problem and the mentioned pull request which could resolve this (https://github.com/mockito/mockito/pull/936) was put on hold. Please either fix this or provide a way to express the after(x).atLeast(y) without having the argument captor returning a collection with millions [sic] of items via ArgumentCaptor#getAllValues() although the method was called only a couple of times.

ChristianSchwarz commented 6 years ago

@Trinova #936 needs to be reviewed by a core member, there is nothing I can do as contributor. I don't know if it will ever happen since the core-team is busy and doesn't plan to hire new core member. Maybe the community should fork Mockito and release it with an other group and artefact-id to overcome the prolonged review process.