1
\$\begingroup\$

Here's a suite of integration tests for my REST controller. Among other things, it reflects my efforts to reduce code duplication

First, I introduced some class fields. token was initially a class field initialized in the constructor too (the fact that you see an explicit no-args is a relic of that), but for some reason, it didn't work. However, there's an idea that test methods should be as isolated as possible so any class fields are actually a bad design feature

Second, I wrote an interface called ExpectationTester. Its implementations basically encapsulate asserting code with dynamic variables. And yes, I know I should've written JavaDocs. In fact, I documented some of my classes, but since this project is done (it's not production code to be maintained), I don't think there's any reason to write more of them

What do you think?

public class UserQuestionCommentRestControllerTest extends IntegrationTestContext {
    private static final String testUsername = "mickey_m";
    private static final String testPassword = "mickey";
    private String token;
    private static final String BASE_URI = "/api/v1/user/question-comments/";
    private static final String BASE_SCRIPT_PATH = "/sql/UserQuestionCommentRestControllerTest/";
    private ExpectationTester expectationTester;

    public UserQuestionCommentRestControllerTest() {

    }

    @Sql(executionPhase = BEFORE_TEST_METHOD, value = BASE_SCRIPT_PATH + "CreateCommentTest/before.sql")
    @Sql(executionPhase = AFTER_TEST_METHOD, value = BASE_SCRIPT_PATH + "CreateCommentTest/after.sql")
    @Test
    public void createCommentTest() throws Exception {
        token = testUtil.getToken(testUsername, testPassword);
        QuestionCommentRequestDto dto =
                new QuestionCommentRequestDto("text", 1L);
        mockMvc.perform(post(BASE_URI)
                        .contentType(MediaType.APPLICATION_JSON)
                        .header(HttpHeaders.AUTHORIZATION, token)
                        .content(objectMapper.writeValueAsString(dto)))
                .andExpect(status().isCreated());
        assertTrue(entityManager.createQuery(
                        """
                                SELECT COUNT(qc.id) = 1
                                FROM QuestionComment qc
                                JOIN qc.owner ow
                                JOIN qc.question q
                                WHERE qc.text = 'text'
                                AND q.createdDate IS NOT NULL
                                AND q.modifiedDate IS NOT NULL
                                AND ow.id = 1
                                """, Boolean.class)
                .getSingleResult());
    }

    @Test
    @Sql(executionPhase = BEFORE_TEST_METHOD, value = BASE_SCRIPT_PATH + "DeleteCommentTest/before.sql")
    @Sql(executionPhase = AFTER_TEST_METHOD, value = BASE_SCRIPT_PATH + "DeleteCommentTest/after.sql")
    public void deleteCommentTest() throws Exception {
        token = testUtil.getToken(testUsername, testPassword);
        mockMvc.perform(delete(BASE_URI + 1)
                        .header(HttpHeaders.AUTHORIZATION, token))
                .andExpect(status().isOk());
        assertTrue(entityManager.createQuery("""
                        SELECT COUNT(qc.id) = 0
                        FROM QuestionComment qc
                        """, Boolean.class)
                .getSingleResult());
    }

    @Test
    @Sql(executionPhase = BEFORE_TEST_METHOD, value = BASE_SCRIPT_PATH + "GetCommentTest/before.sql")
    @Sql(executionPhase = AFTER_TEST_METHOD, value = BASE_SCRIPT_PATH + "GetCommentTest/after.sql")
    public void getCommentTest() throws Exception {
        token = testUtil.getToken(testUsername, testPassword);
        MockHttpServletResponse response = mockMvc.perform(get(BASE_URI + 1)
                        .header(HttpHeaders.AUTHORIZATION, token))
                .andExpect(status().isOk())
                .andReturn()
                .getResponse();

        expectationTester = new GetCommentExpectationTester.Builder(response)
                .setExpectedOwnerUsername("mickey_m")
                .build();

        expectationTester.test();
    }

    @Test
    @Sql(executionPhase = BEFORE_TEST_METHOD, value = BASE_SCRIPT_PATH + "GetPageTest/before.sql")
    @Sql(executionPhase = AFTER_TEST_METHOD, value = BASE_SCRIPT_PATH + "GetPageTest/after.sql")
    public void getPageOneDefaultTest() throws Exception {
        token = testUtil.getToken(testUsername, testPassword);
        MockHttpServletResponse response = mockMvc.perform(get(BASE_URI + "page/" + 1)
                        .header(HttpHeaders.AUTHORIZATION, token))
                .andExpect(status().isOk())
                .andReturn()
                .getResponse();

        expectationTester = new GetCommentPageExpectationTester.Builder(response)
                .setExpectedPageCount(10)
                .setExpectedPageDtoListSize(10)
                .setExpectedOwnerUsername("mickey_m")
                .build();

        expectationTester.test();
    }
    @Test
    @Sql(executionPhase = BEFORE_TEST_METHOD, value = BASE_SCRIPT_PATH + "GetPageTest/before.sql")
    @Sql(executionPhase = AFTER_TEST_METHOD, value = BASE_SCRIPT_PATH + "GetPageTest/after.sql")
    public void getPageOneSizeFiveSortByIdTest() throws Exception {
        token = testUtil.getToken(testUsername, testPassword);
        MockHttpServletResponse response = mockMvc.perform(get(BASE_URI + "page/" + 1)
                        .header(HttpHeaders.AUTHORIZATION, token)
                        .param("pageSize", "5"))
                .andExpect(status().isOk())
                .andReturn()
                .getResponse();

        expectationTester = new GetCommentPageExpectationTester.Builder(response)
                .setExpectedPageCount(10)
                .setExpectedPageDtoListSize(5)
                .setExpectedOwnerUsername("mickey_m")
                .build();

        expectationTester.test();
    }
// more tests
-- a before script for "get page" methods, to give an example
TRUNCATE TABLE permissions CASCADE;
TRUNCATE TABLE accounts CASCADE;
TRUNCATE TABLE questions CASCADE;
TRUNCATE TABLE tags CASCADE;
TRUNCATE TABLE question_comments CASCADE;

INSERT INTO permissions(id, name, created_date, modified_date)
VALUES (1, 'ADMIN', current_timestamp, current_timestamp),
       (2, 'MODERATOR', current_timestamp, current_timestamp),
       (3, 'USER', current_timestamp, current_timestamp);

INSERT INTO accounts(id, username, password, enabled, created_date, modified_date)
VALUES (1, 'mickey_m', '$2y$10$ZdqfOo1vwdHJJGnkmGrw/OUelZcU9ZfRFaX/RMN3XniXH96eTkB1e', true, current_timestamp,
        current_timestamp),
       (2, 'minerva_m', '$2y$10$PiZxGGi904rCLdTSGY1ycuQhcEtQrP1u74KvQ2IEuk5Jh18Ml.6xO', true, current_timestamp,
        current_timestamp);

INSERT INTO accounts_permissions(account_id, permission_id)
VALUES (1, 3),
       (2, 3);

INSERT INTO questions(id, created_date, modified_date, title, description, account_id)
VALUES (1, current_timestamp, current_timestamp, 'title', 'description', 2);

INSERT INTO tags(id, created_date, modified_date, name)
VALUES (1, current_timestamp, current_timestamp, 'tag1'),
       (2, current_timestamp, current_timestamp, 'tag2'),
       (3, current_timestamp, current_timestamp, 'tag3');

INSERT INTO questions_tags(question_id, tag_id)
VALUES (1, 1), (1, 2), (1, 3);

INSERT INTO question_comments(id, created_date, modified_date, text, account_id, question_id)
VALUES (1, '1970-01-01 06:00:00', '1970-01-01 06:00:00', 'text', 1, 1),
       (2, '1970-01-01 00:00:00', '1970-01-01 07:00:00', 'text', 1, 1),
       (3, '1970-01-01 01:00:00', '1970-01-01 00:00:00', 'text', 1, 1),
       (4, '1970-01-01 07:00:00', '1970-01-01 01:00:00', 'text', 1, 1),
       (5, '1970-01-01 02:00:00', '1970-01-01 08:00:00', 'text', 1, 1),
       (6, '1970-01-01 03:00:00', '1970-01-01 02:00:00', 'text', 1, 1),
       (7, '1970-01-01 08:00:00', '1970-01-01 03:00:00', 'text', 1, 1),
       (8, '1970-01-01 04:00:00', '1970-01-01 09:00:00', 'text', 1, 1),
       (9, '1970-01-01 05:00:00', '1970-01-01 04:00:00', 'text', 1, 1),
       (10, '1970-01-01 09:00:00', '1970-01-01 05:00:00', 'text', 1, 1);
-- an after script for "get page" methods, to give an example
TRUNCATE TABLE permissions CASCADE;
TRUNCATE TABLE accounts CASCADE;
TRUNCATE TABLE questions CASCADE;
TRUNCATE TABLE tags CASCADE;
TRUNCATE TABLE question_comments CASCADE;
public interface ExpectationTester {
    void test() throws JsonProcessingException;
}
public abstract class AbstractGetCommentExpectationTester implements ExpectationTester {
    protected final String serializedResponseBody;
    protected final String expectedCommentText;
    protected final int expectedQuestionId;
    protected final int expectedOwnerId;
    protected final String expectedOwnerUsername;
    protected final ObjectMapper objectMapper;
    protected AbstractGetCommentExpectationTester(Builder builder) {
        this.serializedResponseBody = builder.serializedResponseBody;
        this.expectedOwnerId = builder.expectedOwnerId;
        this.expectedOwnerUsername = builder.expectedOwnerUsername;
        this.expectedQuestionId = builder.expectedQuestionId;
        this.expectedCommentText = builder.expectedCommentText;
        this.objectMapper = new ObjectMapper()
                .registerModule(new JavaTimeModule())
                .registerModule(new ParameterNamesModule());
    }
    public abstract void test() throws JsonProcessingException;
    public abstract static class Builder {
        protected final String serializedResponseBody;
        protected String expectedCommentText = "text";
        protected int expectedQuestionId = 1;
        protected int expectedOwnerId = 1;
        protected String expectedOwnerUsername = "user";
        protected Builder(MockHttpServletResponse response) throws UnsupportedEncodingException {
            this.serializedResponseBody = response.getContentAsString();
        }
        public Builder setExpectedOwnerUsername(String expectedOwnerUsername) {
            this.expectedOwnerUsername = expectedOwnerUsername;
            return this;
        }
        public Builder setExpectedOwnerId(int expectedOwnerId) {
            this.expectedOwnerId = expectedOwnerId;
            return this;
        }
        public Builder setExpectedQuestionId(int expectedQuestionId) {
            this.expectedQuestionId = expectedQuestionId;
            return this;
        }
        public Builder setExpectedCommentText(String expectedCommentText) {
            this.expectedCommentText = expectedCommentText;
            return this;
        }
        public abstract AbstractGetCommentExpectationTester build();
    }
}
public class GetCommentExpectationTester extends AbstractGetCommentExpectationTester {
    public GetCommentExpectationTester(Builder builder) {
        super(builder);
    }
    @Override
    public void test() throws JsonProcessingException {
        TypeReference<Data<QuestionCommentResponseDto>> typeReference =
                new TypeReference<>() {};
        Data<QuestionCommentResponseDto> deserializedResponseBody =
                objectMapper.readValue(serializedResponseBody, typeReference);
        assertNotNull(deserializedResponseBody.getData());
        QuestionCommentResponseDto dto = deserializedResponseBody.getData();
        assertEquals(expectedQuestionId, dto.getQuestionId());
        assertEquals(expectedCommentText, dto.getText());
        assertNotNull(dto.getOwner());

        AccountResponseDto owner = dto.getOwner();
        assertEquals(expectedOwnerId, owner.getId());
        assertEquals(expectedOwnerUsername, owner.getUsername());
    }
    public static class Builder extends AbstractGetCommentExpectationTester.Builder {
        public Builder(MockHttpServletResponse response) throws UnsupportedEncodingException {
            super(response);
        }
        @Override
        public GetCommentExpectationTester build() {
            return new GetCommentExpectationTester(this);
        }
    }
}
public class GetCommentPageExpectationTester extends AbstractGetCommentExpectationTester {
    private final int expectedPageCount;
    private final int expectedPageDtoListSize;
    private final int expectedFirstCommentId;

    private GetCommentPageExpectationTester(Builder builder) {
        super(builder);
        this.expectedPageCount = builder.expectedPageCount;
        this.expectedPageDtoListSize = builder.expectedPageDtoListSize;
        this.expectedFirstCommentId = builder.expectedFirstCommentId;
    }
    @Override
    public void test() throws JsonProcessingException {
        TypeReference<Data<Page<QuestionCommentResponseDto>>> typeReference =
                new TypeReference<>() {};
        Data<Page<QuestionCommentResponseDto>> deserializedResponseBody =
                objectMapper.readValue(serializedResponseBody, typeReference);
        assertNotNull(deserializedResponseBody.getData());
        Page<QuestionCommentResponseDto> page = deserializedResponseBody.getData();
        assertEquals(expectedPageCount, page.getCount());
        assertNotNull(page.getDtos());
        List<QuestionCommentResponseDto> dtoList = page.getDtos();
        assertEquals(expectedPageDtoListSize, dtoList.size());

        QuestionCommentResponseDto firstDto = dtoList.get(0);
        assertEquals(expectedFirstCommentId, firstDto.getId());

        for (QuestionCommentResponseDto dto : dtoList) {
            assertEquals(expectedQuestionId, dto.getQuestionId());
            assertEquals(expectedCommentText, dto.getText());
            assertNotNull(dto.getCreatedDate());
            assertNotNull(dto.getModifiedDate());

            AccountResponseDto actualOwner = dto.getOwner();
            assertEquals(expectedOwnerId, actualOwner.getId());
            assertEquals(expectedOwnerUsername, actualOwner.getUsername());
        }
    }
    public static class Builder extends AbstractGetCommentExpectationTester.Builder {
        private int expectedPageCount = 10;
        private int expectedPageDtoListSize = 10;
        private int expectedFirstCommentId = 1;

        public Builder(MockHttpServletResponse response) throws UnsupportedEncodingException {
            super(response);
        }

        public Builder setExpectedPageCount(int expectedPageCount) {
            this.expectedPageCount = expectedPageCount;
            return this;
        }
        public Builder setExpectedPageDtoListSize(int expectedPageDtoListSize) {
            this.expectedPageDtoListSize = expectedPageDtoListSize;
            return this;
        }
        public Builder setExpectedFirstCommentId(int expectedFirstCommentId) {
            this.expectedFirstCommentId = expectedFirstCommentId;
            return this;
        }
        @Override
        public GetCommentPageExpectationTester build() {
            return new GetCommentPageExpectationTester(this);
        }
    }
}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

multiple interfaces

when you execute your test it seems very valid for me to test your response. It is all nice until you start digging deeper into your implementation logic. Especially when you try to test your REST endpoint make it a black box test, that does not know how things are internally implemented

public void createCommentTest() throws Exception {
    //given
    token = testUtil.getToken(testUsername, testPassword);
    QuestionCommentRequestDto dto = new QuestionCommentRequestDto("text", 1L);

    //when & then
    mockMvc.perform(post(BASE_URI)
        .contentType(MediaType.APPLICATION_JSON)
        .header(HttpHeaders.AUTHORIZATION, token)
        .content(objectMapper.writeValueAsString(dto)))
        .andExpect(status().isCreated()); 
    }

this would be sufficient enough! If you want to test your persisting logic, then write another test. Your test should (only) test the REST interface goint outside - you should not include a test down to the database.

See this picture from toolsqa that explains the idea of the interface

REST Layers

Additional benefit: your test is independ from your actual database - since you provide your own test database (mock or inmemory) and do not need a running databse for your test. and you could also mock your db to bring up with pre-defined behaviour, eg. if you want to verify your REST endpoint behaves correct, when you delete an non-existing entry

And even further, when your data model changes, your tests do not need any maintaince to update the queries, since you don't have any queries!

bloated Tester

your AbstractGetCommentExpectationTester is bloated, what annoyes me most is that it does two very important thing - parse data and validate data.

you really should separate these two thing - create a parser instead that gives you your desired dto and put the assertions back to the test, where they belong

public void getPageOneSizeFiveSortByIdTest() throws Exception {

    //given
    token = testUtil.getToken(testUsername, testPassword);

    //when
    MockHttpServletResponse response = mockMvc.perform(get(BASE_URI + "page/" + 1)
        .header(HttpHeaders.AUTHORIZATION, token)
        .param("pageSize", "5"))
        .andExpect(status().isOk())
        .andReturn()
        .getResponse();

    //then
    Page<QuestionCommentResponseDto> page = commentParser.parse(response);
    
    assertEquals(expectedPageCount, page.getCount());
    assertNotNull(page.getDtos());
    List<QuestionCommentResponseDto> dtoList = page.getDtos();
    assertEquals(expectedPageDtoListSize, dtoList.size());

    QuestionCommentResponseDto firstDto = dtoList.get(0);
    assertEquals(expectedFirstCommentId, firstDto.getId());

    for (QuestionCommentResponseDto dto : dtoList) {
        assertEquals(expectedQuestionId, dto.getQuestionId());
        assertEquals(expectedCommentText, dto.getText());
        assertNotNull(dto.getCreatedDate());
        assertNotNull(dto.getModifiedDate());

        AccountResponseDto actualOwner = dto.getOwner();
        assertEquals(expectedOwnerId, actualOwner.getId());
        assertEquals(expectedOwnerUsername, actualOwner.getUsername());
    }

}

if you do it this way you can literally see at one glance the totat test behaviour. you can see your test data (given), you see the executed code (when) and the desired behaviour (then)

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Thank you! But why do you think it was wrong to extract those assertions? It allowed me to reuse them instead of copy-and-pasting them multiple times with very few changes \$\endgroup\$
    – Sergey
    Commented Jun 26, 2023 at 13:52
  • \$\begingroup\$ @Sergey generelly speaking is DRY (don't repeat yourself) important - but having tests, that describe the behaviour on one page (without looking in other places) is far more valuable. Aagain speaking general: KISS > DRY (KISS=Keep it simple, stupid), meaning KISS is more important than DRY. \$\endgroup\$ Commented Jun 26, 2023 at 18:32
  • \$\begingroup\$ as side note i want to share my experience, where i have some legacy code (welll, we always have some legacy, noself pity here) - many of the unit tests in this project use data that is generated in some other place. and i have always look up which data is now under test and that costs much time and distracts me from the real bug finding. ok, in that case the data is off side, in your test the assertion are off side - but in any case you have to search for information that should be inside the test case. \$\endgroup\$ Commented Jun 26, 2023 at 18:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.