I’ve been thinking about unit testing best practices and have come across the one assertion per unit test rule. I can see where this idea would help to isolate pieces of complex operations or verification when you have code like so:
private Place place;
@Test
public void test_whiteHouse(){
place = new place(KnownPlaces.WHITEHOUSE);
assertEquals("1600 Pennsylvania Avenue", place.getAddress());
assertEquals("Washington", place.getCity());
assertEquals("DC", place.getState());
}
I see where you would generally want to break it into smaller tests like this:
private Place place;
@Before
public void setUp(){
place = new place(KnownPlaces.WHITEHOUSE);
}
@Test
public void test_whiteHouse_street(){
assertEquals("1600 Pennsylvania Avenue", place.getAddress());
}
@Test
public void test_whiteHouse_city(){
assertEquals("Washington", place.getCity());
}
@Test
public void test_whiteHouse_state(){
assertEquals("DC", place.getState());
}
However, if I’m testing a setter, the code might look like this:
@Test
public void test_place_setCity(){
String newCity = "timbuktu";
Place place = new Place();
place.setCity(newCity);
assertEquals(newCity, place.getCity());
}
The test will run properly, but if there’s a shared setup, I’m making assumptions about the initial state. What would be the downside on running a check on the initial state something like:
@Test
public void test_place_setCity(){
String newCity = "timbuktu";
Place place = new Place();
assertFalse(place.getCity().equals(newCity));
place.setCity(newCity);
assertEquals(newCity, place.getCity());
}
This check would verify that the setter method actually changed something by verifying the initial state was different than the final state. Otherwise, a change or malfunction in a constructor or setup method could cause the test to become ineffective. Are there any downsides to violating the one assert per constraint rule in this way?
We have a policy where I work that uses an Assume class (provided by Nunit) for all these types of initial checks. Further, we’ve made a wrapper around the assume class that forces the test writer to reference another test which explicitly tests the assumption. I think I can better describe it with code:
@Test
public void test_place_setCity(){
// Arrange
String newCity = "timbuktu";
Place place = new Place();
Assume.That(place.getCity(), Is.Not.EqualTo(newCity), validatingTest:"test_new_place_should_not_have_city_set");
// Act
place.setCity(newCity);
// Assert
assertEquals(newCity, place.getCity());
}
@Test
public void test_new_place_should_not_have_city_set(){
// Arrange
// Nothing to Arrange
// Act
Place place = new Place();
// Assert
Assert.That(place.getCity(), Is.Empty, message:"New cities should not have a city initialized");
}
So here, in our test_place_setCity test, we call Assume, and specify the name of the test that SHOULD fail if the assumption fails. NUnit’s assume class returns an “Inconclusive” rather than a “Failing” result when the condition does not succeed. I think these pre-checks are important in many regards. They let future maintainers know WHAT THE TEST ASSUMES to pass up front in an easy to understand way, they FAIL FAST when the assumption doesn’t hold because they are at the top of the tests, and they give SEPERATE RESULTS for reporting purposes and to eliminate noise. The name of another test is also a bonus for debugging. Instead of trying to debug why a test won’t run because an assumption fails, you can instead, easily navigate to the test that focuses on the assumption and debug using the more focused test.
Hopefully, you can find something in your testing framework to achieve all of these ideals, or else roll something yourself. We’ve been using this methodology for a while and it’s proved very useful.
3
I wouldn’t call it a rule but rather a guideline from the past. The common misunderstanding relates to what “one assertion” really means. Unfortunately, some people tend to bend it towards “one assert method call” without really giving it any thought. This is to some extent understandable, after all one of the first guidelines related to unit testing you’ll probably hear that “unit test should be atomic, small and test one thing“.
However, one assertion is more about logical concept than number of method calls in test. In your example, if all the getters provide simple value that has little to no sense existing outside of its entity, then all 3 asserts relate to one logical concept – your entity. If the getter produces more complex value, it might be worth having its own test. It’s always a judgement call.
The code in your example uses well known pattern called guard assertion, and it’s nothing wrong with that. Keep in mind however, that guard assertion might be always replaced by extra unit test, as noted here.
4
In your example, I would not make an assertion about the state of the object before calling the setter. That is outside of the scope of what you are testing. That should have been checked in the initialization tests. The setter/getter test is checking only that functionality so that would be the only assertion that you need.