How Many Assertions in My Tests?

Friday, October 30, 2009

One Long Test

When I first started writing unit tests with JUnit, I didn't read any books, follow blogs, or otherwise attempt to be the best test writer I could be. I wanted to crank out code, crank out tests, and see the green bar. In fact, my tests looked a lot like this:

public void testGetRequestBeansByIDType() {
    Integer requestID=8;
    Integer statusID=4;
    Integer systemID=1;
    Integer processTypeID=1;
    String inList="0,4,5,6,7,8";
    String requestIDSQL = "Select count(*) as NumRequests from " +
        "requests where requestid="+requestID;
    String statusIDSQL = "Select count(*) as NumRequests from " +
        "requests where statusid="+statusID;
    String systemIDSQL = "Select count(*) as NumRequests from " +
        "requests where systemid="+systemID;
    String processIDSQL = "Select count(*) as NumRequests from " +
        "requests where processTypeID="+processTypeID;
    String inSQL = "Select count(*) as NumRequests from " +
        "requests where RequestID IN("+inList+")";
    try {
        //test requestID
        HashMap<String,Object> row = dbops.getRow(requestIDSQL,1);
        int count = ((Integer)row.get("numrequests")).intValue();
        ArrayList<RequestBean> requests 
            = dbops.getRequestBeansByIDType("RequestID",requestID.toString(),"");
        System.out.println("RequestID rows is " + requests.size());
        //test statusID
        HashMap<String,Object> row2 = dbops.getRow(statusIDSQL,1);
        int count2 = ((Integer)row2.get("numrequests")).intValue();
        ArrayList<RequestBean> requests2
            = dbops.getRequestBeansByIDType("StatusID",statusID.toString(),"");
        System.out.println("statusID rows is " + requests2.size());
        //test systemid
        HashMap<String,Object> row3 = dbops.getRow(systemIDSQL,1);
        int count3 = ((Integer)row3.get("numrequests")).intValue();
        ArrayList<RequestBean> requests3
            = dbops.getRequestBeansByIDType("SystemID",systemID.toString(),"");
        System.out.println("systemID rows is " + requests3.size());
        //test ProcessTypeID
        HashMap<String,Object> row4 = dbops.getRow(processIDSQL,1);
        int count4 = ((Integer)row4.get("numrequests")).intValue();
        ArrayList<RequestBean> requests4
            = dbops.getRequestBeansByIDType("ProcessTypeID",processTypeID.toString(),"");
        System.out.println("ProcessTypeID rows is " + requests4.size());
        //test IN() statements
        HashMap<String,Object> row5 = dbops.getRow(inSQL,1);
        int count5 = ((Integer)row5.get("numrequests")).intValue();
        ArrayList<RequestBean> requests5
            = dbops.getRequestBeansByIDType("RequestID",inList,"");
        System.out.println("IN rows is " + requests5.size());
        // test order by....
        ArrayList<RequestBean> requests6
            = dbops.getRequestBeansByIDType("StatusID","3","RequestID Desc");
        RequestBean rb = requests6.get(0);
        RequestBean rb2 = requests6.get(1);
        assertTrue(rb.getRequestID() > rb2.getRequestID());
        ArrayList<RequestBean> requests7
        = dbops.getRequestBeansByIDType("StatusID","3","RequestID asc");
        RequestBean rb3 = requests7.get(0);
        RequestBean rb4 = requests7.get(1);
        assertTrue(rb4.getRequestID() > rb3.getRequestID());
    } catch (Exception e) {
       fail("getRow should not have returned an error");

If you’re looking at that thinking “What’s wrong with that?”, then please read on. If you’re horrified… join the club.

What’s wrong with that?

testGetRequestBeansByIDType, how do I hate thee? Let me count the ways

  1. If some assertion fails up toward the top of this test, none of the other assertions run. So what happens is you get yourself in this constant loop of “it worked last month, now it’s not working. Why?” and so you go fix the failing test, satisfied everything is OK, and then you run em again and another assertion in the same test fails. This infuriates me
  2. Notice how I’m using comments for each of the different tests?  I’m trying to communicate requirements, but I’m doing so in a way that doesn’t much help. If each of these little chunks were separate tests, the test case itself would be much more communicative in my opinion
  3. It’s been my experience that the more things I do in a single test, the more likely I am to create/change some state that modifies the behavior of the subsequent assertions
  4. When developing in this manner, just cranking out tests and not writing descriptive test names that describe the behavior under test, I’m missing out on one of unit testing’s greatest benefits: using it as an aide to clarify thinking. This is a huge topic, and one that I don’t want to get into here. Suffice it to say, anyone who’s done TDD for a while knows exactly what I mean. Anyone who hasn’t done it will only know what I mean once you get yourself in the camp of the people who have done it. You have to earn this answer

So what’s a boy to do?  Personally, I’ve become fond of BDD-style naming. I prefer long, descriptive test names; I prefer as few assertions as necessary in a single test.  This leads to some additional lines of code, but I use snippets for this so I’m not spending any more time writing more tests than I would be if I were simply writing more assertions. The resultant clarity is worth it.

Ahhhh, That’s Better

Here’s a more recent example:

<!--- basic tests to make sure the 'outer' logic is right--->
<cffunction name="recipientsWithAffectedLetterCode_Should_GetPhoneNumberCoordinates" returntype="void" access="public">
	<cfset var coordinates = "">
	<cfloop list="#affectedLetterCodes#" index="code">			
		<cfset coordinates = getCoordinatesWithWCRBCD("JH89",code)>	
		<cfset assertTrue( StructCount(coordinates) GT 0,"Coordinates struct should not have been empty for #code# but was." )>

<cffunction name="recipientsWithoutAffectedLetterCode_ShouldNot_GetPhoneNumberCoordinates" returntype="void" >			
	<cfset var coordinates = "">
	<cfloop list="#unaffectedLetterCodes#" index="code">
		<cfset coordinates = getCoordinatesWithWCRBCD("junk",code)>	
		<cfset assertTrue( StructIsEmpty(coordinates),"Coordinates struct should have been empty for #code# but was not. It had keys: #StructKeyList(coordinates)#" )>

<!--- more granular tests for the specifics of which number to get;
yes, this duplicates the logic in the production code. So be it. --->
<cffunction name="WCRBCD_JH89_gets_2011_PhoneNumber" output="false" access="public" returntype="any" hint="">
	<cfset var coordinates = getCoordinatesWithWCRBCD("JH89")>
	<cfset assertEquals("866.420.2011",coordinates.PhoneNumber,"")>		

<cffunction name="WCRBCD_MK01_gets_9446_PhoneNumber" output="false" access="public" returntype="any" hint="">
	<cfset var coordinates = getCoordinatesWithWCRBCD("MK01")>
	<cfset assertEquals("866.356.9446",coordinates.PhoneNumber,"")>	

<cffunction name="WCRBCD_NotMK01AndNotJH89_gets_7436_PhoneNumber" output="false" access="public" returntype="any" hint="">
	<cfset var coordinates = getCoordinatesWithWCRBCD("NOTMK01")>
	<cfset assertEquals("866.281.7436",coordinates.PhoneNumber,"")>	

Ignore for a minute the weirdo codes (WCRBCD, JH89, etc) as they are domain-specific and well-known for people working on the project. The point here is that the names describe exactly the various expected behaviors of this single function under test (getCoordinatesWithWCRBCD()). When one test fails, I don’t have to spend time reasoning about whether previous logic or function calls are the culprit. I and other team members don’t have to learn about the behavior by reading comments because the function names themselves are the documentation.  I don’t have that “Fixed-this-assertion-and-now-another-one-is-failing” problem. If something breaks, I know very soon because we run everything in a continuous integration environment. Code changes, tests run, something breaks: I can go right into Hudson and see exactly the behavior that used to work and now no longer works. If multiple things break as a result of a code change, I see those multiple things, not just the first assertion that failed.

Is there a downside to this approach? Yes… if you’re typing out each function by hand, i.e. < c f f u …, then you’re going to be typing more. But if you’re typing out functions by hand, you’re doing it wrong! MXUnit comes bundled with a bunch of useful Eclipse snippets. Read the documentation, install them, and use them. Then, when you want to add a new test function, you type the word “test”, hit CTRL-J, and up pops this box:


From there, you type in your test name. For example: “thisFunction_should_DoFancyThings” and hit Enter. This code gets added to your editor:


Typing problem solved.

The Bottom Line

In the end, the fewer assertions you put into your tests  -- as opposed to fewer tests with more assertions -- the more descriptive, stable, and design-for-testability-helpful your tests will become.


Henry Ho said...

The downside is not much of a downside if you use CF9 script style. :)

Micky Dionisio said...

I've got to ask - how many unit tests are u up to for one given project?

Marc Esher said...

One thing I wanted to add to this: that second example I posted is an extreme example. I'd say most of my more recent tests usually have 2, perhaps 3 assertions. I follow these patterns pretty closely:

Micky, as for number of tests, on my "main" project, which has been in development for a very, very long time -- but for which I've only started writing unit tests in the past year and a half or so -- I have about 400 tests. Many of them look like the first example in the blog post, where a single test attempted to cover the entirety of that function's behavior. We have about 14 developers, and I'm the only one who writes tests. So our coverage is pitiful. But hey, it's better than nothing.

Mike said...

Marc, you are a trooper, buddy. I'm surprised people haven't glommed on to the testing thing at BR yet. Especially when they employ a cf personality!

On the other hand, I'm stil trying to get my team to write components, lol. I'm still wading through tons of pasta. Those big projects with lots of hands and years in them are definitely hard to move to a more regimented approach. I guess on those, I like Spolsky's approach that you don't need unit testing if you don't already have it and you have a production app. Just try to break off pieces and use new processes on those.

Great post on the wiki, btw.