Reputation: 14162
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
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
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
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
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
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
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
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