jmockit / jmockit1

Advanced Java library for integration testing, mocking, faking, and code coverage
Other
465 stars 240 forks source link

Matcher called O(N^2) times and tests fail when they should pass #194

Closed dlipofsky closed 9 years ago

dlipofsky commented 9 years ago

Test 1 prints the following in 1.12

A A true
B B true
C C true
D D true

but prints this in 1.13 - 1.18

A A true
A B false
B B true
A C false
B C false
C C true
A D false
B D false
C D false
D D true

Also test2 passes in 1.12 but fails in 1.13 - 1.18. test1 passes in all versions. This is suprising since the difference is so small. Both tests seem to have the O(N^2) problem.

import mockit.Expectations;
import mockit.Injectable;
import org.hamcrest.Description;
import org.hamcrest.TypeSafeMatcher;
import org.junit.Test;

public class MyExTest {
    interface RemoteFoo {
        void sendMessage(String msg);
    }

    static class MessageMatcher extends TypeSafeMatcher<String> {
        private final String expected;

        public MessageMatcher(final String m) {
            this.expected = m;
        }

        @Override
        public void describeTo(final Description description) {
            description.appendText("Message with " + expected);

        }

        @Override
        protected boolean matchesSafely(final String msg) {
            System.out.println(expected + " " + msg + " " + expected.equals(msg));
            return expected.equals(msg);
        }
    }

    @Injectable
    private RemoteFoo remoteFoo;

    @Test
    public void test1() throws InterruptedException {
        new Expectations() {
            {
                remoteFoo.sendMessage(withArgThat(new MessageMatcher("A")));
                remoteFoo.sendMessage(withArgThat(new MessageMatcher("B")));
                remoteFoo.sendMessage(withArgThat(new MessageMatcher("C")));
                remoteFoo.sendMessage(withArgThat(new MessageMatcher("D")));
            }
        };
        remoteFoo.sendMessage("A");
        remoteFoo.sendMessage("B");
        remoteFoo.sendMessage("C");
        remoteFoo.sendMessage("D");
    }

    @Test
    public void test2() throws InterruptedException {
        new Expectations() {
            {
                remoteFoo.sendMessage(withArgThat(new MessageMatcher("A")));
                remoteFoo.sendMessage(withArgThat(new MessageMatcher("B")));
                remoteFoo.sendMessage(withArgThat(new MessageMatcher("C")));
                remoteFoo.sendMessage(withArgThat(new MessageMatcher("A")));
            }
        };
        remoteFoo.sendMessage("A");
        remoteFoo.sendMessage("B");
        remoteFoo.sendMessage("C");
        remoteFoo.sendMessage("A");
    }
}
rliesenfeld commented 9 years ago

The behavior of Expectations changed in release 1.13, as described in the release notes: http://jmockit.org/changes.html#1.13

It was changed because of frequent API misuse; users often were unaware of the "strict" semantics, or didn't really need it.

So, if you use StrictExpectations, the behavior from JMockit 1.12 is restored. But there are better solutions for tests like these, without using strict expectations.

The first option is to use a VerificationsInOrder block:

@Test
public void test1b() {
    remoteFoo.sendMessage("A");
    remoteFoo.sendMessage("B");
    remoteFoo.sendMessage("C");
    remoteFoo.sendMessage("D");

    new VerificationsInOrder() {{
        remoteFoo.sendMessage("A");
        remoteFoo.sendMessage("B");
        remoteFoo.sendMessage("C");
        remoteFoo.sendMessage("D");
    }};
}

A second and better option is to capture the sequence of messages sent (since there is a single method to be verified):

@Test
public void test1c() {
    remoteFoo.sendMessage("A");
    remoteFoo.sendMessage("B");
    remoteFoo.sendMessage("C");
    remoteFoo.sendMessage("D");

    new Verifications() {{
        List<String> messages = new ArrayList<String>();
        remoteFoo.sendMessage(withCapture(messages));
        assertEquals(asList("A", "B", "C", "D"), messages);
    }};
}