jmockit / jmockit1

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

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

Closed patriziobruno closed 7 years ago

patriziobruno commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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