Wednesday, December 22, 2010

Duplicated Setup Code in Your Tests? Refactor to a Factory!

Your test code is production code. Treat it the same way. Don’t let cruft build up, and follow good software engineering principles – like Don’t Repeat Yourself (DRY).

I was looking through some test code the other day and found a great opportunity to refactor out some common behavior to a factory. I do this quite often in the test frameworks I build – shove off responsibility for data and object creation to a factory so I can keep things much clearer in my tests.

In this case, we’re testing a parser class that creates reports based on an XML file. A number of tests check the parser’s handling of malformed XML or missing elements. The basic pattern below is repeated across eight or nine tests: hardwire an XML string in the test, feed that to the parser, then check if appropriate errors and messages are generated by the parser.

   1: var reportXml =
   2:     @"<report version='4.0'>
   3:             <key>FiscalYearByBrowser</key>
   4:             <description>Shows cool stuff.</description>
   5:             <connectionString>TelligentAnalyticsAnalysisServer</connectionString>
   6:             <query>
   7:                select some SQL-fu from a magic place and return unicorns
   8:             </query>
   9:       </report>";
  10:  
  11: _report = ReportParser.Parse(reportXml);

This is repetitious, duplicated, copied cruft. Small joke there. Moreover, it’s not readily apparent what the XML snippet’s intent is. I have to read the entire XML, then mentally unwind what I know of the structure to figure out that, oh, this snippet is actually missing the title element.

Compare the above with this section of refactored code:

   1: _report = ReportParser.Parse(TestReportFactory.GetWithoutTitle());

The implementation of the TestReportFactory isn’t overly important [1]; what’s important here is that duplicated behavior should be pushed out to a common area, regardless of whether it’s in the system under test or your test code itself.

Your tests are production code. Treat them as such!

(NOTE: Some folks, particularly the Really Smart Guy Jeremy D. Miller, make the absolutely solid case that you should never sacrifice readability in your tests for the sake of cutting duplication. It’s good to push duplicated behavior out if you can keep your tests completely clear. It’s not ok to push behavior up to base classes or elsewhere if your test turns into a vague skeleton. Readability trumps everything else.)

[1] The TestReportFactory simply builds up a string of XML, creates an XElement from that, then uses XElement.remove() or XAttribute.remove() to drop elements or attributes as needed. The entire class is pasted below. Yes, there are a number of other ways to skin this cat. I could make things a bit more dynamic, but this worked for what I needed right now. And yes, I drove design and implementation of this factory with a separate set of tests I wrote first…

   1: internal class TestReportFactory
   2: {
   3:     private static string _reportOpen = @"<report version='4.0'>";
   4:     private static string _key = @"<key>FiscalYearByBrowser</key>";
   5:     private static string _title = @"<title>Fiscal Year by Browser</title>";
   6:  
   7:     private static string _description =
   8:         @"<description>Shows page views per browser (IE, FireFox, etc..)
   9:          for each fiscal year.</description>";
  10:  
  11:     private static string _connectionString =
  12:         @"<connectionString>TelligentAnalyticsAnalysisServer</connectionString>";
  13:  
  14:     private static string _query =
  15:         @"<query>
  16:             select non empty [Time].[Fiscal Year].Members on columns,
  17:             non empty [Dim Browser].[Browser Name].Members on rows
  18:             from [Evolution Reporting]
  19:          </query>";
  20:  
  21:     private static string _chartTypes =
  22:         @"<chartTypes>
  23:                                 <add type=""fancy 3d chart"" />
  24:                                 <add type=""plain 2d chart"" />
  25:                               </chartTypes>";
  26:  
  27:     private static string _reportClose = @"</report>";
  28:  
  29:  
  30:     public static string GetValidXml()
  31:     {
  32:         XElement doc = CreateValidReportXElement();
  33:         return doc.ToString();
  34:     }
  35:  
  36:     private static XElement CreateValidReportXElement()
  37:     {
  38:         var _outgoing = new StringBuilder();
  39:         _outgoing.Append(_reportOpen);
  40:         _outgoing.Append(_key);
  41:         _outgoing.Append(_title);
  42:         _outgoing.Append(_description);
  43:         _outgoing.Append(_connectionString);
  44:         _outgoing.Append(_query);
  45:         _outgoing.Append(_chartTypes);
  46:         _outgoing.Append(_reportClose);
  47:         XElement doc = XElement.Parse(_outgoing.ToString());
  48:         return doc;
  49:     }
  50:  
  51:  
  52:     internal static string GetWithoutQuery()
  53:     {
  54:         return CreateXmlStringWithoutElement("query");
  55:     }
  56:  
  57:     private static string CreateXmlStringWithoutElement(string elementName)
  58:     {
  59:         XElement doc = CreateValidReportXElement();
  60:         XElement element = doc.Element(elementName);
  61:         element.Remove();
  62:         return doc.ToString();
  63:     }
  64:  
  65:  
  66:     internal static string GetWithoutKey()
  67:     {
  68:         return CreateXmlStringWithoutElement("key");
  69:     }
  70:  
  71:     internal static string GetWithoutDescription()
  72:     {
  73:         return CreateXmlStringWithoutElement("description");
  74:     }
  75:  
  76:     internal static string GetWithoutConnectionString()
  77:     {
  78:         return CreateXmlStringWithoutElement("connectionString");
  79:     }
  80:  
  81:     internal static string GetWithoutChartTypes()
  82:     {
  83:         return CreateXmlStringWithoutElement("chartTypes");
  84:     }
  85:  
  86:     internal static string GetWithoutTitle()
  87:     {
  88:         return CreateXmlStringWithoutElement("title");
  89:     }
  90:  
  91:     internal static string GetWithoutVersion()
  92:     {
  93:         XElement doc = CreateValidReportXElement();
  94:         XAttribute ver = doc.Attribute("version");
  95:         ver.Remove();
  96:         return doc.ToString();
  97:     }
  98:  
  99:     internal static string GetInvalidXml()
 100:     {
 101:         return "<report>";
 102:     }
 103: }
 104:  

No comments:

Subscribe (RSS)

The Leadership Journey