ICR
ICR

Reputation: 14162

How to unit test constructors

I have a class I am adding unit tests to. The class has several constructors which take different types and converts them into a canonical form, which can then be converted into other types.

public class Money {
    public Money(long l) {
        this.value = l;
    }

    public Money(String s) {
        this.value = toLong(s);
    }

    public long getLong() {
        return this.value;
    }

    public String getString() {
        return toString(this.value);
    }
}

In reality there are a couple of other types it accepts and converts to.

I am trying to work out what the most appropriate way to test these constructors is.

Should there be a test per-constructor and output type:

@Test
public void longConstructor_getLong_MatchesValuePassedToConstructor() {
    final long value = 1.00l;

    Money m = new Money(value);
    long result = m.getLong();

    assertEquals(value, result);
}

This leads to a lot of different tests. As you can see, I'm struggling to name them.

Should there be multiple asserts:

@Test
public void longConstructor_outputsMatchValuePassedToConstructor() {
    final long longValue = 1.00l;
    final String stringResult = "1.00";

    Money m = new Money(longValue);

    assertEquals(longValue, m.getLong());
    assertEquals(stringResult, m.getString());
}

This has multiple asserts, which makes me uncomfortable. It is also testing getString (and by proxy toString) but not stating that in the test name. Naming these are even harder.

Am I going about it completely wrong by focussing on the constructors. Should I just test the conversion methods? But then the following test will miss the toLong method.

@Test
public void getString_MatchesValuePassedToConstructor() {
    final long value = 1.00;
    final String expectedResult = "1.00";

    Money m = new Money(value);
    String result = m.getLong();
    assertEquals(expectedResult, result);
}

This is a legacy class and I can't change the original class.

Upvotes: 44

Views: 115177

Answers (7)

stranger777
stranger777

Reputation: 11

Constructor is an object initializer. Constructor testing is a test of the fact of initialization. That is, verification that the object's fields were initialized by the values passed to the constructor. And this can only be verified by reflection.

Upvotes: -1

Marc
Marc

Reputation: 3560

You have multiple inputs (through the constructors) and multiple outputs (through different getX() methods). But the number of members that it has internally seem to be fewer (in your example, 1 long value). Wouldn't it be easier to first test the different inputs by creating x different objects using the x different constructors. Then you can check if these are all equal, by using an implemented equals() method. This can be done in a single test method.

Then you can check the possible getter methods one by one without using all the different constructors.

Of course, this requires you to implement (an seperately test) the equals method.

In your example I would create the following testcases:

@Test
public void testEquals() {
    Money m1 = new Money(1);
    Money m2 = new Money(1);
    Money m3 = new Money(2);

    assertEquals(m1, m2);
    assertEquals(m2, m1);
    assertNotEquals(m1, m3);
    assertNotEquals(m3, m1);
    assertNotEquals(m1, null);
}

private void testConstructors(long lValue, String sValue) {
    Money m1 = new Money(lValue);
    Money m2 = new Money(sValue);

    assertEquals(m1, m2);
}

@Test
public void testConstructorsPositive() {
    testConstructors(1, "1");
}

@Test
public void testConstructorsNegative() {
    testConstructors(-1, "-1");
}

@Test
public void testConstructorsZero() {
    testConstructors(0, "0");
}

@Test
public void testGet...() { /* etc... */ }

Upvotes: 0

Andreas Dolk
Andreas Dolk

Reputation: 114787

The expected result from a constructor test is: an instance has been created

Following this idea you could limit the work in constructor tests to pure instantiation:

@Test public void testMoneyString() {
    try {
      new Money("0");
      new Money("10.0");
      new Money("-10.0");
    } catch (Exception e) {
      fail(e.getMessage());
    }
}

@Test public void testMoneyStringIllegalValue() {
    try {
      new Money(null);
      fail("Exception was expected for null input");
    } catch (IllegalArgumentException e) {          
    }

    try {
      new Money("");
      fail("Exception was expected for empty input");
    } catch (IllegalArgumentException e) {          
    }

    try {
      new Money("abc");
      fail("Exception was expected for non-number input");
    } catch (IllegalArgumentException e) {          
    }

}

A test to verify if the conversions work could be assigned to the getters.

Upvotes: 16

Robin
Robin

Reputation: 24262

I think you are spending too much time thinking about it. All of your options work just fine so simply pick the one you like best. Don't forget that the purpose is to give you confidence that the code will work as designed/expected. Each of these scenarios will provide that.

Personally, in such a simple case, I would go with a single test case that validates the constructors. It eliminates the need for the excessive and rather unwieldy method names.

Upvotes: 3

RMT
RMT

Reputation: 7070

Its not a BAD idea to test constructors just to make sure that the take in the data as required, but if you really dont like multiple asserts, split them up and name the method with that they are doing example: CanContructorAcceptString or: CanConstructorAcceptNonLongStringValue

Something like that.

Upvotes: 2

Jon Skeet
Jon Skeet

Reputation: 1501133

It looks like you've got a canonical way of getting the "raw" value (toLong in this case) - so just test that all the constructors are correct when you fetch that value. Then you can test other methods (such as getString()) based on a single constructor, as you know that once the various constructors have finished, they all leave the object in the same state.

This is assuming somewhat white-box testing - i.e. you know that toLong is really a simple reflection of the internal state, so it's okay to test that + a constructor in a test.

Upvotes: 18

skaffman
skaffman

Reputation: 403501

A test per constructor seems most appropriate to me. Don't be afraid to use long, elaborate and wordy names for your test methods, it makes them obvious and descriptive.

@Test
public void moneyConstructorThatTakesALong {

Upvotes: 12

Related Questions