Friday, January 12, 2007

Test smell == Design smell

One of the goals of Test Driven Development is to make sure that your services/object model meets the loose coupling/high cohesion goal of software design. If it is hard to write a unit test for one of your methods, the method has low "testability". Refer to the 'Testability' section of this article at Jeremy Millers' blog for more info about the relationship between testability and design.

Testability and reusability are two closely related aspects of a service/object model. If an operation is not easy to test, it is not easy to reuse. And if it is not easy to reuse, the operation is not well suited for use as part of a composable system. Thus a service with low testability will most likely be hard to reuse in a service-oriented architecture.

Services and operations need to be highly reusable, as a major benefit of truly service-oriented systems is agility: your boss have heard that you have e.g. a currency conversion service based on live exchange rates, and he needs just this functionality right now on the company web-site. If your operation has too many dependencies and cannot easily be reused stand-alone, i.e. the operation requires a complicated/smelly unit-test; the operation is just not ready for real-life SOA.

Low testability is easy to spot: the test method contains a lot of context setup code, session stuff, for-loops and if-else/switch, excessive reference data lookup, calling several other classes or operations, etc. I think of these test anti-patterns as 'test smells', after the term 'code smell' coined by Kent Beck. A test smell is usually an indication of bad design in the tested classes, thus the term 'design smell' comes into mind. A design smell is any anti-pattern to the existing software design best practices and patterns.

Note that test smells and design smells need not be code smells. E.g. for-loops in a test is smelly, for-loops in code is normal; methods with several parameters are OK in code, but is a design smell in service-oriented operations. And the other way; e.g. most code smells regarding the relationship between classes (feature envy, intimacy, tell-don't ask, etc) are also test smells.


These days I am writing some unit-tests to get to know some legacy services. The service was written by someone else, and this is a good way to review the design and reusability of the services. This unit-test code is an example of "the easiest way" to add a comment to an activity using the legacy services, including my "smelly" notes:

//TEST SMELL: CANNOT CONTROL TRANSACTION FROM TEST
//MAJOR DESIGN SMELL: DECLARATIVE TRANSACTIONS NOT SUPPORTED

using (TransactionScope transx = new TransactionScope (TransactionScopeOption.Suppress))
{
//TEST SMELL: FOR-LOOP FOR LOOKING UP REFERENCE DATA
string activityTypeId = null;
foreach (TableValue activityType in referenceData.activityTypeList)
{
//DESIGN SMELL: WHY ISN'T "TYPE" EXPOSED IN THE ACTIVITY OBJECT
if (String.Compare(activityType.name, activity.name)==0)
{activityTypeId = activityType.Id;break;}
}

Assert.IsTrue(activityTypeId != null, "activityTypeId not found in referenceData.activityTypeList collection.");

//DESIGN SMELL: WHY ISN'T "ACTIVITYID" EXPOSED IN THE ACTIVITY OBJECT
string activityId = Constants.NodeId;
string responsibleId = activity.responsible;

//MAJOR DESIGN SMELL: EVERYTHING IS A STRING
//CODE SMELL: PRIMITIVE OBSESSION
string text = "unit-test text";
. . .

string regDate = System.DateTime.Now.ToString(Constants.FORMAT_DATE);
string archive = "false";
string enumerate = "false";

//DESIGN SMELL: RETURNING ENTITIES FROM ADD OPERATION IMPOSES TWO-WAY OPERATION
//DESIGN SMELL: NOT A MESSAGE-BASED SERVICE OPERATION
Comment[] inserted = target.addComment(Constants.VesselGuid, path, activityTypeGuid, status, severity,id, text, responsibleGuid, regDate, commentTypeGuid, archive, enumerate);

Assert.IsTrue(inserted.Length>0, "addComment did not return the expected value.");

//transx.Complete(); //no commit == rollback
. . .

}

The example also shows one of the subtle test smells: the test cannot apply a transaction to the business operation as this will cause the test to fail. As this due to the implementation of the object model, this is a major design smell. This kind of logical design error can go undetected for a long time and cause hard to diagnose errors, as the code will not cause run-time errors until someone tries to create a new transacted operation by composing existing "explicit transaction" operations.

Unit-tests serve many purposes, and if for no other reason, you should write unit-tests to assess the testability of your code, and thus design quality, reusability and agility of the services.

PS! I have updated all my postings with the new tag/label mechanism of Blogger, so my RSS feed will be a little crazy due to the updates.

No comments: