CODEWITHSUNDEEP
javatestingjunit5
Nikolai
Nikolai

Reputation: 83

How to assert that a proper value was assigned to a private field?

I have the following class:

public class Game {

    @Getter
    private String gameId;

    private String player1Id;

    private String player2Id;

    private String currentPlayer;

    private Board board;

    public Game() {
        board = new Board();
        gameId = UUID.randomUUID().toString();
    }

    public void joinGame(String playerUUID) {
        if (player1 == null) {
            player1 = playerUUID;
            currentPlayer = player1;
        } else if (player2 == null) {
            player2 = playerUUID;
        } else {
            throw new IllegalArgumentException("Cannot join");
        }
    }
    .....
}

And I want to test the logic in the joinGame() method:

@Test
void testJoinGame() {
    String player1Id = UUID.randomUUID().toString();
    String player2Id = UUID.randomUUID().toString();
    game.joinGame(player1Id);
    game.joinGame(player2Id);
    // this won't compile of course as fields are private
    assertEquals(player1Id, game.player1Id);
    assertEquals(player2Id, game.player2Id);
    assertEquals(player1Id, game.currentPlayer);
}

There are two bad things I shouldn't do in my JUnit test as I can often read: 1. Change visibility of fields to make tests work. 2. Use reflections to get values of the private fields. (I can see both these approaches are being used a lot in projects I work on, but let's pretend they are not).

Also, I read sometimes that if I want to test my private fields, then it means that the interface of the class was defined wrong.

But I would say that in this example it's not a case: both user ids are used only inside the class and there is no need to expose them. Still I want to be sure that the order is right: the first user who tries to join the game will be player1 (+ he/she will be a current user, e.g. will make the first move), and the second - player2.

I would like to ask what would be a proper solution to test such method?

Upvotes: 1

Views: 896

Answers (2)

Stephen C
Stephen C

Reputation: 718768

I concur with Tom Hawtin's answers, but I would flip them around to say this:

  • Unit tests should generally avoid looking into the internal implementation details of a class under test. They should be testing how the class interacts with things outside the abstraction boundary.

  • If you look "inside the box" in your unit tests, then the tests are liable to break unnecessarily if the implementation changes. Yes, you can then fix the test, but you then have the problem of knowing if non-test code has also been broken.

On the other hand, looking "inside the box" may make it easier to write testcases. If you do decide to take this approach:

  • Adding a getter that your unit test case can use is going to allow other code to depend on the implementation details. This is a bad idea, even if the getter is implemented in a way that prevents modification. (You still have the potential unwanted coupling, and changes to your classes implementation details breaking things).

  • It is also possible to inspect private fields from a testcase using reflection. This is ugly, but arguably better than a getter, since "normal" code wouldn't do this. An implementation change is still liable to break unit tests ... but only unit tests.

Upvotes: 2

Tom Hawtin - tackline
Tom Hawtin - tackline

Reputation: 147154

There's two obvious approaches.

  • Test where the values are actually being used. If the variables are not used outside the class why are they there? Within the code presented you only need to test that you get an IllegalArgumentException appropriately.

  • Add "get" methods.

I would prefer the first.

Upvotes: 2

Related Questions