@Injectable instance gets replaced after first test in testclass #405

Closed
patriziobruno opened this Issue Apr 1, 2017 · 5 comments

Comments

2 participants
@patriziobruno

patriziobruno commented Apr 1, 2017

  • Version of JMockit that was used: 1.30
  • JUnit version: 4.12
  • JDK 1.8 update 121
  • Ubuntu Linux 16.10 kernel 4.8.0-41-generic
  • org.springframework spring-test:4.3.7.RELEASE

I have a test class using spring-test to test a spring-mvc controller. The controller has a @Autowired field, filled by an @Injectable field in the test class. The injectable field has been initialized using a MockUp instance to mock some methods. The first test executed from the class works, when the second test is launched the injectable instance has changed and it's not using my mock anymore, that results in a NullPointerException in the controller class.

I've been able to run the tests by some hack (by making the injectable field static) with JDK 1.8 update 66 on Windows 2008.

I included a zip of the project to help reproduce the problem.

test.zip

@rliesenfeld

This comment has been minimized.

Show comment
Hide comment
@rliesenfeld

rliesenfeld Apr 2, 2017

Member

In CrawlerControllerTest.java, there are no static @Injectable fields; "MockUp mocked" is static, but that is fine if the mock-up is to be reused between tests.

I think the tests should use a real "RunningRequests" object, though, since it's "just" a map:

    @Injectable
    final RunningRequests runningRequests = new RunningRequestsImpl();

With this change, three of the tests continue to pass and two fail (with a "404" result instead of the expected "200"). I didn't examine the tests to see why, but they can probably be changed so they pass with the real RunningRequests.

Member

rliesenfeld commented Apr 2, 2017

In CrawlerControllerTest.java, there are no static @Injectable fields; "MockUp mocked" is static, but that is fine if the mock-up is to be reused between tests.

I think the tests should use a real "RunningRequests" object, though, since it's "just" a map:

    @Injectable
    final RunningRequests runningRequests = new RunningRequestsImpl();

With this change, three of the tests continue to pass and two fail (with a "404" result instead of the expected "200"). I didn't examine the tests to see why, but they can probably be changed so they pass with the real RunningRequests.

@patriziobruno

This comment has been minimized.

Show comment
Hide comment
@patriziobruno

patriziobruno Apr 2, 2017

Why MockUp mocked has to be static? BTW having mocked static works only with JDK 1.8.66, not with 1.8.121: runningRequests is null after the first test.

There are no data in runningRequests, that's causing the 404 and that's one of the reasons I mocked it. The other reason is that I need CrawlerController to work no matter the RunningRequest implementation. RunningRequestsImpl must be tested by a test unit of its own.

Why MockUp mocked has to be static? BTW having mocked static works only with JDK 1.8.66, not with 1.8.121: runningRequests is null after the first test.

There are no data in runningRequests, that's causing the 404 and that's one of the reasons I mocked it. The other reason is that I need CrawlerController to work no matter the RunningRequest implementation. RunningRequestsImpl must be tested by a test unit of its own.

@rliesenfeld rliesenfeld self-assigned this Apr 8, 2017

@rliesenfeld rliesenfeld added bug and removed could not reproduce labels Apr 8, 2017

@rliesenfeld

This comment has been minimized.

Show comment
Hide comment
@rliesenfeld

rliesenfeld Apr 8, 2017

Member

There is no need for any static MockUp or @Injectable instance. The problem with CrawlerControllerTest is that, well, it's making poor use of the mocking/faking APIs. Here is a better version:

@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(locations = "file:src/main/webapp/WEB-INF/applicationContext.xml")
@WebAppConfiguration
public final class CrawlerController_noStatics_Test
{
    static final String testId = UUID.randomUUID().toString();
    URL testUrl;

    @Tested(availableDuringSetup = true) CrawlerController controller;
    @Injectable RunningRequests runningRequests;
    @Injectable CrawlerService service;

    @Autowired WebApplicationContext wac;
    MockMvc mockMvc;

    @Before
    public void setup() throws Exception {
        mockMvc = MockMvcBuilders.standaloneSetup(controller).build();
        testUrl = new URL("http://test.com");
    }

    @Test
    public void startReturnsUUID() throws Exception {
        CrawlingRequests requests = new CrawlingRequests();
        CrawlingRequest request = new CrawlingRequest();
        request.setUrl(testUrl.toString());
        requests.add(request);
        ObjectMapper mapper = new ObjectMapper();

        mockMvc.perform(post("/")
                .content(mapper.writeValueAsString(requests))
                .contentType(MediaType.APPLICATION_JSON)
        )
                .andExpect(status().isCreated())
                .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON))
                .andExpect(content().string(new BaseMatcher<String>() {
                    @Override
                    public boolean matches(Object item) {
                        try {
                            String result = mapper.readValue(item.toString(), String.class);
                            UUID.fromString(result);
                        } catch (IllegalArgumentException | IOException ex) {
                            Logger.getLogger(CrawlerController_noStatics_Test.class.getName()).log(Level.SEVERE, item.toString(), ex);
                            return false;
                        }
                        return true;
                    }

                    @Override
                    public void describeTo(Description description) {}
                }));
    }

    @Test
    public void listenReturnsSSE() throws Exception {
        provideARunningRequestGivenTestId();
        String path = "/" + testId;

        mockMvc.perform(get(path)
                .pathInfo(path)
                .accept(MediaType.TEXT_EVENT_STREAM)
                .header("Connection", "keep-alive")
                .header("Cache-Control", "no-cache")
        )
                .andExpect(status().isOk())
                .andExpect(request().asyncStarted());
    }

    void provideARunningRequestGivenTestId() {
        RunningRequest runningRequest = new RunningRequest(new SseEmitter());
        runningRequest.urls.add(testUrl);

        new Expectations() {{
            runningRequests.isRunning(testId); result = true;
            runningRequests.get(testId); result = runningRequest;
        }};
    }

    @Test
    public void whenListenOnNonExistingIdExpect404() throws Exception {
        mockMvc.perform(get("/doesnt-exist-" + testId)
                .accept(MediaType.TEXT_EVENT_STREAM)
                .header("Connection", "keep-alive")
                .header("Cache-Control", "no-cache")
        )
                .andExpect(status().isNotFound());
    }

    @Test
    public void getStatusReturnsExpectedJsonObject() throws Exception {
        CrawlingStatus status = new CrawlingStatus();
        status.setCode(CrawlingStatus.StatusCode.RUNNING);
        status.setId(testId);
        status.setUrl(testUrl.toString());
        ConcurrentMap<URL, CrawlingStatus> runningOperations = new ConcurrentHashMap<>();
        runningOperations.put(testUrl, status);

        new Expectations() {{ service.running(); result = runningOperations; }};
        provideARunningRequestGivenTestId();

        CrawlingRequestStatus expected = new CrawlingRequestStatus();
        expected.put(testUrl, new CrawlingStatus() {
            {
                setUrl(testUrl.toString());
                setCode(StatusCode.RUNNING);
                setId(testId);
            }
        });
        String path = "/status/" + testId;

        mockMvc.perform(get(path)
                .pathInfo(path)
                .accept(MediaType.APPLICATION_JSON)
        )
                .andExpect(status().isOk())
                .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON))
                .andExpect(content().string(new ObjectMapper().writeValueAsString(expected)));
    }

    @Test
    public void whenGetStatusOnNonExistingIdExpect404() throws Exception {
        String path = "/status/doesnt-exist-" + testId;

        mockMvc.perform(get(path)
                .pathInfo(path)
                .accept(MediaType.APPLICATION_JSON)
        )
                .andExpect(status().isNotFound());
    }
}
Member

rliesenfeld commented Apr 8, 2017

There is no need for any static MockUp or @Injectable instance. The problem with CrawlerControllerTest is that, well, it's making poor use of the mocking/faking APIs. Here is a better version:

@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(locations = "file:src/main/webapp/WEB-INF/applicationContext.xml")
@WebAppConfiguration
public final class CrawlerController_noStatics_Test
{
    static final String testId = UUID.randomUUID().toString();
    URL testUrl;

    @Tested(availableDuringSetup = true) CrawlerController controller;
    @Injectable RunningRequests runningRequests;
    @Injectable CrawlerService service;

    @Autowired WebApplicationContext wac;
    MockMvc mockMvc;

    @Before
    public void setup() throws Exception {
        mockMvc = MockMvcBuilders.standaloneSetup(controller).build();
        testUrl = new URL("http://test.com");
    }

    @Test
    public void startReturnsUUID() throws Exception {
        CrawlingRequests requests = new CrawlingRequests();
        CrawlingRequest request = new CrawlingRequest();
        request.setUrl(testUrl.toString());
        requests.add(request);
        ObjectMapper mapper = new ObjectMapper();

        mockMvc.perform(post("/")
                .content(mapper.writeValueAsString(requests))
                .contentType(MediaType.APPLICATION_JSON)
        )
                .andExpect(status().isCreated())
                .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON))
                .andExpect(content().string(new BaseMatcher<String>() {
                    @Override
                    public boolean matches(Object item) {
                        try {
                            String result = mapper.readValue(item.toString(), String.class);
                            UUID.fromString(result);
                        } catch (IllegalArgumentException | IOException ex) {
                            Logger.getLogger(CrawlerController_noStatics_Test.class.getName()).log(Level.SEVERE, item.toString(), ex);
                            return false;
                        }
                        return true;
                    }

                    @Override
                    public void describeTo(Description description) {}
                }));
    }

    @Test
    public void listenReturnsSSE() throws Exception {
        provideARunningRequestGivenTestId();
        String path = "/" + testId;

        mockMvc.perform(get(path)
                .pathInfo(path)
                .accept(MediaType.TEXT_EVENT_STREAM)
                .header("Connection", "keep-alive")
                .header("Cache-Control", "no-cache")
        )
                .andExpect(status().isOk())
                .andExpect(request().asyncStarted());
    }

    void provideARunningRequestGivenTestId() {
        RunningRequest runningRequest = new RunningRequest(new SseEmitter());
        runningRequest.urls.add(testUrl);

        new Expectations() {{
            runningRequests.isRunning(testId); result = true;
            runningRequests.get(testId); result = runningRequest;
        }};
    }

    @Test
    public void whenListenOnNonExistingIdExpect404() throws Exception {
        mockMvc.perform(get("/doesnt-exist-" + testId)
                .accept(MediaType.TEXT_EVENT_STREAM)
                .header("Connection", "keep-alive")
                .header("Cache-Control", "no-cache")
        )
                .andExpect(status().isNotFound());
    }

    @Test
    public void getStatusReturnsExpectedJsonObject() throws Exception {
        CrawlingStatus status = new CrawlingStatus();
        status.setCode(CrawlingStatus.StatusCode.RUNNING);
        status.setId(testId);
        status.setUrl(testUrl.toString());
        ConcurrentMap<URL, CrawlingStatus> runningOperations = new ConcurrentHashMap<>();
        runningOperations.put(testUrl, status);

        new Expectations() {{ service.running(); result = runningOperations; }};
        provideARunningRequestGivenTestId();

        CrawlingRequestStatus expected = new CrawlingRequestStatus();
        expected.put(testUrl, new CrawlingStatus() {
            {
                setUrl(testUrl.toString());
                setCode(StatusCode.RUNNING);
                setId(testId);
            }
        });
        String path = "/status/" + testId;

        mockMvc.perform(get(path)
                .pathInfo(path)
                .accept(MediaType.APPLICATION_JSON)
        )
                .andExpect(status().isOk())
                .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON))
                .andExpect(content().string(new ObjectMapper().writeValueAsString(expected)));
    }

    @Test
    public void whenGetStatusOnNonExistingIdExpect404() throws Exception {
        String path = "/status/doesnt-exist-" + testId;

        mockMvc.perform(get(path)
                .pathInfo(path)
                .accept(MediaType.APPLICATION_JSON)
        )
                .andExpect(status().isNotFound());
    }
}
@rliesenfeld

This comment has been minimized.

Show comment
Hide comment
@rliesenfeld

rliesenfeld Apr 8, 2017

Member

And here is a better version for DataControllerTest, which had a static @Injectable field:

public final class DataController_noStatics_Test
{
    final UUID testId = UUID.randomUUID();
    final List<Myprojizable> expected = asList(new Myprojizable("http://test.com", "news", 100, testId.toString()));

    @Tested(availableDuringSetup = true, fullyInitialized = true) DataController controller;
    @Injectable Data data;

    @Autowired WebApplicationContext wac;
    MockMvc mockMvc;

    @Before
    public void setup() {
        mockMvc = MockMvcBuilders.standaloneSetup(controller).build();
    }

    @Test
    public void list() throws Exception {
        new Expectations() {{ data.list(testId.toString()); result = expected; }};
        String path = "/list/" + testId;

        mockMvc.perform(get(path)
                .pathInfo(path)
                .accept(MediaType.APPLICATION_JSON)
        )
                .andExpect(status().isOk())
                .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON))
                .andExpect(content().string(new ObjectMapper().writeValueAsString(expected)));
    }

    @Test
    public void whenListOnNonExistingIdExpectEmptyList() throws Exception {
        String path = "/list/doesnt-exist-" + testId;

        mockMvc.perform(get(path)
                .pathInfo(path)
                .accept(MediaType.APPLICATION_JSON)
        )
                .andExpect(status().isOk())
                .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON))
                .andExpect(content().string("[]"));
    }

    @Test
    public void listAll() throws Exception {
        new Expectations() {{ data.list(null); result = expected; }};

        mockMvc.perform(get("/list")
                .accept(MediaType.APPLICATION_JSON)
        )
                .andExpect(status().isOk())
                .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON))
                .andExpect(content().string(new ObjectMapper().writeValueAsString(expected)));
    }
}
Member

rliesenfeld commented Apr 8, 2017

And here is a better version for DataControllerTest, which had a static @Injectable field:

public final class DataController_noStatics_Test
{
    final UUID testId = UUID.randomUUID();
    final List<Myprojizable> expected = asList(new Myprojizable("http://test.com", "news", 100, testId.toString()));

    @Tested(availableDuringSetup = true, fullyInitialized = true) DataController controller;
    @Injectable Data data;

    @Autowired WebApplicationContext wac;
    MockMvc mockMvc;

    @Before
    public void setup() {
        mockMvc = MockMvcBuilders.standaloneSetup(controller).build();
    }

    @Test
    public void list() throws Exception {
        new Expectations() {{ data.list(testId.toString()); result = expected; }};
        String path = "/list/" + testId;

        mockMvc.perform(get(path)
                .pathInfo(path)
                .accept(MediaType.APPLICATION_JSON)
        )
                .andExpect(status().isOk())
                .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON))
                .andExpect(content().string(new ObjectMapper().writeValueAsString(expected)));
    }

    @Test
    public void whenListOnNonExistingIdExpectEmptyList() throws Exception {
        String path = "/list/doesnt-exist-" + testId;

        mockMvc.perform(get(path)
                .pathInfo(path)
                .accept(MediaType.APPLICATION_JSON)
        )
                .andExpect(status().isOk())
                .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON))
                .andExpect(content().string("[]"));
    }

    @Test
    public void listAll() throws Exception {
        new Expectations() {{ data.list(null); result = expected; }};

        mockMvc.perform(get("/list")
                .accept(MediaType.APPLICATION_JSON)
        )
                .andExpect(status().isOk())
                .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON))
                .andExpect(content().string(new ObjectMapper().writeValueAsString(expected)));
    }
}
@patriziobruno

This comment has been minimized.

Show comment
Hide comment
@patriziobruno

patriziobruno Apr 11, 2017

Thanks for the advice about the test implementation and for providing a better implementation! :)

Thanks for the advice about the test implementation and for providing a better implementation! :)

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