Monday, January 25, 2010

5 Words on Test Organization

Instead of this:


void testCallListReportData() {
controller.programService = programService
def program = Program.findByName(Keys.PROGRAM_ABC)
assertEquals Keys.PROGRAM_ABC, program.name
def student = Student.findByLastName(Keys.STUDENT)
assertEquals Keys.STUDENT, student.lastName
mockParams.id = program.id
def reportData = controller.callListReportData()
assertEquals Keys.PROGRAM_ABC, mockParams['PROGRAM_NAME']
assertNotNull reportData
def thisMap = reportData[0]
assertEquals student.fullName(), thisMap['STUDENT_NAME']
assertEquals Keys.CONTACT_EMAIL, thisMap['CONTACT_EMAIL']
}

Prefer this:

void testCallListReportData() {
controller.programService = programService
def program = Program.findByName(Keys.PROGRAM_ABC)
assertEquals Keys.PROGRAM_ABC, program.name
def student = Student.findByLastName(Keys.STUDENT)
assertEquals Keys.STUDENT, student.lastName
mockParams.id = program.id

// test
def reportData = controller.callListReportData()

assertEquals Keys.PROGRAM_ABC, mockParams['PROGRAM_NAME']
assertNotNull reportData
def thisMap = reportData[0]
assertEquals student.fullName(), thisMap['STUDENT_NAME']
assertEquals Keys.CONTACT_EMAIL, thisMap['CONTACT_EMAIL']
}

5 comments:

Michael Easter said...

This example is Groovy, FWIW. It is a test I wrote, so I'm not calling anyone out.

I encourage greater use of whitespace than shown in the 2nd example, but I wanted my point to really stand out.

Brian Gilstrap said...

Excellent example of where short oneline/inline comments in code are powerful and not intrusive nor likely to become stale.

Hamlet D'Arcy said...

I prefer William Wake's

// arrange

// act

// assert

and if they are out of order you have a smell! (which they invariably are with record+playback mocking frameworks)

James Carr said...

In addition to Bill Wake's AAA, this is also what we in the BDD community call Given, When, Then.

As for out of order, well... if you use Mockito you don't have to deal with that out of order nonsense brought on by EasyMock's playback feature. ;)

Michael Easter said...

Thanks for the comments, folks.

I once used


// pre


// test


// post



But ultimately decided that was silly. However, I can see the value of the AAA and GWT ideas. They read nicely.

My main thinking was (a) highlight the actual code being tested and (b) if there is more than one such line, think "code smell".