I'm at work now, staring at Eclipse and a method of approximately 150 lines. It's a method for performing sort order updates. Someone else, long gone, wrote it. It's not ugly code by any means... just complex. And there's a bug. A big part of me wants to work through this bug by testing it in the interface. Change the sort orders in the form, hit Submit, add dumps and aborts, comment out the actual database update in the code, and struggle through till I fix the sucker. But damn, I just know that's gonna be painful and slow. And if I'm going to spend the time fixing it, I might as well capture my work. So a-unit-testin' I will go. This is the first time I've seen this code, and since I gotta fix a bug in it, I'm wondering.... how easy will it be to write a unit test for this code. As with most of our stuff, it's database-heavy. And I don't want to let it perform actual database updates and then test the database after-the-fact to confirm that the updates I wanted to happen actually happen. That right there is one way to describe hard-to-test code: you have to perform updates, then refetch the data to see if the results you expected are true. This code I'm looking at has several loops, many nested. I was hoping not to see a bunch of CFQueries inside the loops. This would've been a rat's nest, virtually impossible to test well. But instead, there's only a single CFQuery chunk, at the bottom of the method. The original programmer did a really smart thing: He built up a big ol' array of the updates he wanted to perform, and then in the end looped over the array and did the database work. This is great for 2 reasons:
- It's a lot easier to test arrays for the data they contain than it is to test the database after the fact
- Since there's only one chunk of code that performs a database Update, I can easily "turn the update off", thereby not updating the database at all.
#2 probably needs some more explanation, so let me show. Here's what the code basically looked like to start. I've omitted all the actual logic since it's not important here. The important part is that there's only a single block for the queries, and that block is at the end of the original method:
<cffunction name="moveSection" returntype="array" hint="resorts stuff" access="public">
<cfargument name="blah">
<cfset var sections = ArrayNew(1)>
<cfset var ii = 1>
<cfset var jj = 1>
<cfset var kk = 1>
<cfset var keyList = "">
<cfset var updateTemplateSections = "">
<!--- assload of logic to build up an array --->
.....
<!--- process array and extract sectionIDs in order --->
<cfloop from="1" to="#arrayLen(sections)#" index="ii">
<cfset keyList = listAppend(keyList,sections[ii].sectionID)>
<cfloop from="1" to="#arrayLen(sections[ii].children)#" index="jj">
<cfset keyList = listAppend(keyList,sections[ii].children[jj].sectionID) />
</cfloop>
</cfloop>
<!--- finally, do the update --->
<cfloop from="1" to="#listLen(keyList)#" index="kk">
<cfquery name="updateTemplateSections" datasource="#getDsn()#">
UPDATE templateSections
SET sortOrder = #kk#
WHERE templateSectionId = #listGetAt(keyList,kk)#
</cfquery>
</cfloop>
<cfreturn sections>
</cffunction>
As I said, in my test, I want to do two things: Test the array for its data, and ensure the database update doesn't occur. This method, as originally written,
almost gets me there. To the original programmer (my friend Mike), nice job! The obvious problem here, with respect to testing, is that the update will still happen. So what to do? Since the original code has a single block for the DB update, it's easy for me to pull out that chunk of code into a separate method, and then inside my test, override that method with one that simply doesn't perform any updates. I call this "Extract and Override". Here's my new code. It now contains 2 methods. I've simply extracted out that last chunk of DB updates into a separate method, and then I call that in the original function:
<cffunction name="moveSection" returntype="array" hint="resorts stuff" access="public">
<cfargument name="blah">
<cfset var sections = ArrayNew(1)>
<cfset var ii = 1>
<cfset var jj = 1>
<cfset var keyList = "">
<!--- assload of logic to build up an array --->
.....
<!--- process array and extract sectionIDs in order --->
.....
<!--- call my new method, which I've extracted --->
<cfset performMoveSectionUpdate(keyList)>
<cfreturn sections>
</cffunction>
<cffunction name="performMoveSectionUpdate" access="private">
<cfargument name="keyList">
<cfset var kk = 1>
<cfset var updateTemplateSections = "">
<!--- finally, do the update --->
<cfloop from="1" to="#listLen(keyList)#" index="kk">
<cfquery name="updateTemplateSections" datasource="#getDsn()#">
UPDATE templateSections
SET sortOrder = #kk#
WHERE templateSectionId = #listGetAt(keyList,kk)#
</cfquery>
</cfloop>
</cffunction>
Finally, the unit test. Since I've extracted out the actual code that performs the update, it's trivial to override that with MXUnit in the test. All I do is create a new, private function inside the test itself that does nothing. Then, I use
injectMethod to replace the existing function with my new for-testing-only function:
<cffunction name="moveSectionDownResortsCorrectly">
<cfset injectMethod(gw, this, "performMoveSectionUpdate", "performMoveSectionUpdate")>
<cfset sections = gw.moveSection(arg1,arg2,arg3)>
<!--- perform assertions as needed --->
</cffunction>
<cffunction name="performMoveSectionUpdate" access="private">
<cfreturn "">
</cffunction>
Bottom line:
- in my original object, extract out the behavior that I don't want to occur, during my test, into a separate method
- in the test, override that method in my original object with a new, innocuous method
I'm now free to test the arrays for their data without fear of corrupting the database since I'm no longer performing any updates in the method I want to test. This leads to a more predictable, less brittle, faster test. -- marc
3 comments:
Just wanted to mention that the example code omits var'ing things like the query name and index for loop. Sorry for being the var police, no offense intended -- MXUnit Rocks!
noted, and fixed. thanks Peter!
Post a Comment