4
\$\begingroup\$

I've have experience in unit testing but I'm fairly new to acceptance testing and REST-assured. I would like to get some feedback on my style. This is not a complete list of all my test but a few that I felt didn't need context. In the code there are two methods that need describing. validHolidayScheme is a JSON builder that creates a valid holiday scheme with some default data. aHolidayScheme that is JSON builder that creates a holiday scheme with some name specified without any other defaults.

When doing unit testing I use the AAA pattern but it feels kind of silly adding comments like //Arrange, //Act and //Assert when you already have given(), when(), then() inside the tests. How would you use the AAA pattern with REST-assured tests or how would you structure your code?

Also, my tests have a lot of code duplication but still simple and I don't want to extract to much into helper methods making the tests obscure. Would you, and if so, what would you extract into helper methods?

Any other suggestions on how I could improve my code?

public class HolidaySchemeTest {

    @Test
    public void shouldAddSchemeToList() {
        int schemeId = given().body(validHolidayScheme())
                .post("/rest/holidayscheme").jsonPath().getInt("id");

        given().get("/rest/holidayscheme")
                .then()
                .statusCode(Response.Status.OK.getStatusCode())
                .body("id", hasItem(schemeId));
    }

    @Test
    public void shouldGetHolidayScheme() {
        int schemeId = given().body(aHolidayScheme("The holiday"))
                .post("/rest/holidayscheme").jsonPath().getInt("id");

        given().get("/rest/holidayscheme/" + schemeId)
                .then()
                .statusCode(Response.Status.OK.getStatusCode())
                .body("id", equalTo(schemeId))
                .body("name", equalTo("The holiday"));
    }

    @Test
    public void shouldUpdateHolidayScheme() {
        int schemeId = given().body(validHolidayScheme().withName("my holidays"))
                .post("/rest/holidayscheme").jsonPath().getInt("id");

        given().body(validHolidayScheme().withName("alt holidays").withId(schemeId))
                .put("/rest/holidayscheme/" + schemeId)
                .then()
                .statusCode(Response.Status.OK.getStatusCode());

        given().get("/rest/holidayscheme/" + schemeId)
                .then()
                .statusCode(Response.Status.OK.getStatusCode())
                .body("name", equalTo("alt holidays"));
    }

    @Test
    public void shouldDeleteHolidayScheme() {
        int schemeId = given().body(validHolidayScheme().withName("my holidays"))
                .post("/rest/holidayscheme").jsonPath().getInt("id");

        given().delete("/rest/holidayscheme/" + schemeId)
                .then()
                .statusCode(Response.Status.OK.getStatusCode());

        given().get("/rest/holidayscheme/" + schemeId)
                .then().statusCode(Response.Status.NOT_FOUND.getStatusCode());
    }

    @Test
    public void shouldUpdateFixedHolidayForHolidayScheme() {
        //Arrange
        int schemeId = given().body(validHolidayScheme())
                .post("/rest/holidayscheme").jsonPath().getInt("id");

        int holidayId = given().body(validHoliday())
                .post("/rest/holidayscheme/" + schemeId + "/days/fixed")
                .jsonPath().getInt("id");

        //Act
        given().body(aHoliday().withName("Xmas").withDate("25/Dec").withDuration("3h"))
                .put("/rest/holidayscheme/" + schemeId + "/days/fixed/" + holidayId)
                .then()
                .statusCode(Response.Status.OK.getStatusCode());

        //Assert
        given().get("/rest/holidayscheme/" + schemeId + "/days/fixed/" + holidayId)
                .then()
                .statusCode(Response.Status.OK.getStatusCode())
                .body("name", equalTo("Xmas"))
                .body("date", equalTo("25/Dec"))
                .body("duration", equalTo("3h"));
    }

    @Test
    public void shouldDeleteHolidayFromHolidayScheme() {
        //Arrange
        int schemeId = given().body(validHolidayScheme())
                .post("/rest/holidayscheme").jsonPath().getInt("id");

        int holidayId = given().body(validHoliday())
                .post("/rest/holidayscheme/" + schemeId + "/days/fixed").jsonPath().getInt("id");

        //Act
        given().delete("/rest/holidayscheme/" + schemeId + "/days/fixed/" + holidayId)
                .then()
                .statusCode(Response.Status.OK.getStatusCode());

        //Assert
        given().get("/rest/holidayscheme/" + schemeId + "/days/fixed")
                .then()
                .statusCode(Response.Status.OK.getStatusCode())
                .body("", hasSize(0));
    }

    @Test
    public void shouldAssignMemberToHolidayScheme() {
        int schemeId = given().body(validHolidayScheme())
                .post("/rest/holidayscheme").jsonPath().getInt("id");

        given().put("/rest/holidayscheme/" + schemeId + "/member/admin")
                .then()
                .statusCode(Response.Status.OK.getStatusCode());

        given().get("/rest/holidayscheme/" + schemeId + "/members")
                .then()
                .statusCode(Response.Status.OK.getStatusCode())
                .body("username", hasItems("admin"));
    }

    @Test
    public void shouldNotDeleteHolidaySchemeWithMember() {
        int schemeId = given().body(validHolidayScheme())
                .post("/rest/holidayscheme").jsonPath().getInt("id");

        given().put("/rest/holidayscheme/" + schemeId + "/member/admin")
                .then()
                .statusCode(Response.Status.OK.getStatusCode());

        given().delete("/rest/holidayscheme/" + schemeId)
                .then() //Assert
                .statusCode(Response.Status.BAD_REQUEST.getStatusCode());
    }

    @Test
    public void shouldCountMembersOfHolidayScheme() {
        //Arrange
        int schemeId = given().body(validHolidayScheme())
                .post("/rest/holidayscheme").jsonPath().getInt("id");

        given().put("/rest/holidayscheme/" + schemeId + "/member/adam")
                .then()
                .statusCode(Response.Status.OK.getStatusCode());
        given().put("/rest/holidayscheme/" + schemeId + "/member/eve")
                .then()
                .statusCode(Response.Status.OK.getStatusCode());

        //Act
        given().get("/rest/holidayscheme")
                .then() //Assert
                .body("count", contains(2));
    }

    @Test
    public void shouldCountMembersOfHolidaySchemeWhenMemberIsMoved() {
        //Arrange
        int firstSchemeId = given().body(validHolidayScheme())
                .post("/rest/holidayscheme").jsonPath().getInt("id");

        given().put("/rest/holidayscheme/" + firstSchemeId + "/member/adam")
                .then()
                .statusCode(Response.Status.OK.getStatusCode());
        given().put("/rest/holidayscheme/" + firstSchemeId + "/member/eve")
                .then()
                .statusCode(Response.Status.OK.getStatusCode());

        int secondSchemeId = given().body(validHolidayScheme())
                .post("/rest/holidayscheme").jsonPath().getInt("id");

        given().put("/rest/holidayscheme/" + secondSchemeId + "/member/eve");

        //Act
        given().get("/rest/holidayscheme")
                .then() //Assert
                .statusCode(Response.Status.OK.getStatusCode())
                .body("count", contains(1, 1));
    }
}
\$\endgroup\$
2
\$\begingroup\$
  • Yeah I'd leave out the comments, the code should be self-evident.
  • Any way to get the paths from somewhere instead of hardcoding them? You could also extract them into constants if there's no good way and/or extract methods to fill in values.
  • Possibly extract other constant patterns (...getStatusCode) to reduce the number of tokens to read.
\$\endgroup\$
  • \$\begingroup\$ Thank you for your answer. 1. The comments and format is according to the AAA pattern which has helped me with clarity in many of my unit tests. Does not mix very well with given/when/then. 2. Not without losing clarity I think. 3. Yes this might help. \$\endgroup\$ – Jakob Nov 21 '16 at 21:49
1
\$\begingroup\$

There is a lot of duplication in your code. You can extract all of that duplication in private methods:

private int schemeIdFrom(JSONBuilderOrWhatever b) {
    return doPost(b, "/rest/holidayscheme");
}

private int holidayIdFrom(JSONBuilderOrWhatever b, int schemeId) {
    return doPost(b, "/rest/holidayscheme/" + schemeId + "/days/fixed");
}

private void doGet(String resourceId, Body... bodies) {
    X x = given().get(resourceId)
                 .then()
                 .statusCode(Response.Status.OK.getStatusCode());

    Arrays.stream(bodies).forEach(b -> x = x.body(b.id, b.test.get()));
}

private void doPut(JSONBuilderOrWhatever b, String resourceId) {
    given().body(b)
           .put(resourceId)
           .then()
           .statusCode(Response.Status.OK.getStatusCode());
}

private void doDelete(String resourceId) {
    given().delete(resourceId)
           .then()
           .statusCode(Response.Status.OK.getStatusCode());
}

private int doPost(JSONBuilderOrWhatever b, String resourceId) {
    return given().body(b)
                  .post(resourceId)
                  .jsonPath()
                  .getInt("id");
}

Note that I created a small helper class Body:

public final class Body {
    public final String id;
    public final Supplier<Boolean> test;

    public Body(String id, Supplier<Boolean> test) {
        this.id = id;
        this.test = test;
    }
}

Then your tests could be simplified like this:

@Test
public void shouldAddSchemeToList() {
    int schemeId = schemeIdFrom(validHolidayScheme());

    doGet("/rest/holidayscheme", 
            new Body("id", () -> hasItem(schemeId)));
}

@Test
public void shouldGetHolidayScheme() {
    int schemeId = schemeIdFrom(aHolidayScheme("The holiday"));

    doGet("/rest/holidayscheme/" + schemeId,
            new Body("id", () -> equalTo(schemeId)),
            new Body("name", () -> equalTo("The holiday")));
}

@Test
public void shouldUpdateHolidayScheme() {
    int schemeId = schemeIdFrom(validHolidayScheme().withName("my holidays"));

    doPut(validHolidayScheme().withName("alt holidays").withId(schemeId), "/rest/holidayscheme/" + schemeId);

    doGet("/rest/holidayscheme/" + schemeId, 
            new Body("name", () -> equalTo("alt holidays")));
}

@Test
public void shouldDeleteHolidayScheme() {
    int schemeId = schemeIdFrom(validHolidayScheme().withName("my holidays"));

    doDelete("/rest/holidayscheme/" + schemeId);

    given().get("/rest/holidayscheme/" + schemeId)
           .then()
           .statusCode(Response.Status.NOT_FOUND.getStatusCode());
}

@Test
public void shouldUpdateFixedHolidayForHolidayScheme() {
    int schemeId = schemeIdFrom(validHolidayScheme());
    int holidayId = holidayIdFrom(validHoliday(), schemeId)

    doPut(aHoliday().withName("Xmas").withDate("25/Dec").withDuration("3h"), "/rest/holidayscheme/" + schemeId + "/days/fixed/" + holidayId);

    doGet("/rest/holidayscheme/" + schemeId + "/days/fixed/" + holidayId,
            new Body("name", () -> equalTo("Xmas")),
            new Body("date", () -> equalTo("25/Dec")),
            new Body("duration", () -> equalTo("3h")));
}

//etc.

There's still a bit of duplication between the doXXX but I think you get the idea. The goal is to avoid duplication in order to have tests that are easy to understand and maintain (clean tests are as important as clean production code).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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