Tuesday, January 27, 2009

100% Code Coverage is hard on Legacy Code

I have some legacy code that I am working on that has a simplified contract that looks like this:

virtual void ProcessBatch(OutputBatch batch)



The class that implements this contract is loaded by reflection and this method is called based on the execution context.I had a requirement to send a message to a queue once this method completed successfully so I created the following overload:
void ProcessBatch(OutputBatch batch, DataSourceQueue queue)
What I had to do to make this work is extract the execution code from the first method and place that into my overload and then call my overload from the first method. This all works fine; however I have no way to test the fact that the first method calls the second method.To demonstrate I wrote the following test:
[Test]
public void should_call_the_overload_method_taking_the_queue()
{
device.ProcessBatch(batch);
device.AssertWasCalled(x => x.ProcessBatch (Arg<OutputBatch>.Is.TypeOf, Arg<ProviderMailboxQueue>.Is.TypeOf));
}
I must be doing something wrong here because my brain tells me this should work but instead I get the message that this should be called once but was not.

Wednesday, January 21, 2009

So what reads better

I went to the TDD Firestarter this past weekend in Tampa, FL and I definately walked away with some new techniques for the tool belt.  Today I leveraged some of those techniques and I must say I believe the test code reads better.  There is no better way to express this then with an example.
I had written some test code approximately a week ago that made sure that file names complied with a file naming standard.  I was using the RowTest feature in MBUnit such that I had only a single test to validate the file naming convention for different scenarios.  My test looked like this:


[TestFixtureSetUp]
public override void Setup()
{
_mock = new MockRepository();
_testDataDir = TestHelper.GetTestDataDir(this.GetType());
_pathToMessageXml = Path.Combine(_testDataDir.FullName, "message.xml");
_pathToArBlob = Path.Combine(_testDataDir.FullName, "ARBlob.obj");
_pathToWriteCompressedFiles = Path.Combine(_testDataDir.FullName, CompressedFilesPath);

if (!Directory.Exists(_pathToWriteCompressedFiles)) Directory.CreateDirectory(_pathToWriteCompressedFiles);
}

[TestFixtureTearDown]
public override void TearDown()
{
Directory.Delete(_pathToWriteCompressedFiles, true);
}

[SetUp]
public override void TestSetup()
{

_settingsLookupService = _mock.StrictMock<TransmissionSettingsLookupService>();
_jobSystemDb = _mock.StrictMock<IJobSystemDatabase>();
_config = _mock.StrictMock<IConfig>();
_settings = new TestTransmissionSettings(_jobId, PackagingMethod.PGP,
File.ReadAllText(Path.Combine(_testDataDir.FullName, "publickeyusedtoencryptfile.txt")),
File.ReadAllText(Path.Combine(_testDataDir.FullName, "privatekeyusedtosignfile.txt")),
PassPhraseUsedToSignFile,
DataReturnType.FullDetailReport);

using (_mock.Record())
{

SetExpectationsOnConfig(_config, _testDataDir);
SetupResult.For(_jobSystemDb.IsValidJob(_jobId)).Return(true);
SetupResult.For(_settingsLookupService.GetTransmissionSettingsByKey(_jobId.ToString(),
_config)).IgnoreArguments().Return(_settings);
}
}

[RowTest]
[Row(new object[] { ExecutionContext.ProviderMailboxing, PackagingMethod.Zip,
@"PDF_\d+_\d+_\d{4}_\d{2}_\d{2}T\d{2}_\d{2}_\d{2}.zip", DataReturnType.FullDetailReport })]

[Row(new object[] { ExecutionContext.ProviderMailboxing, PackagingMethod.None,
@"835_\d+_\d+_\d{4}_\d{2}_\d{2}T\d{2}_\d{2}_\d{2}.edi", DataReturnType.Edi })]
public void verify_file_naming_convention_based_on_execution_context_packaging_method_and_data_return_type(string
executionContext, string packagingMethod, string filePattern, string dataReturnType)

{
_settings.SetPackagingMethod(packagingMethod);
_settings.SetDataReturnType(dataReturnType);
var filePatternToExpect = new Regex(filePattern);

using (Stream stream = CreateMessageFromXml(_pathToMessageXml, executionContext))
{

using (_mock.Playback())
{
var factory = new ServiceContextDataReturnMessageFactory();
var parsedMessage = PDFRegenHelper.ParseMessage(stream);
DataReturnMessage message = factory.CreateMessage(parsedMessage, _settingsLookupService, _config,
_jobSystemDb);

IJobPackager packager = GetPackager(packagingMethod, message);
var packagedFile = packager.Package(GetFilesToPackage(dataReturnType),
message.FileNameAndPathOfCompressedFile);

Assert.IsTrue(filePatternToExpect.Match(packagedFile.Name).Success,
string.Format("Expected a file in the format {0}, but received a file with name {1}", filePattern,
packagedFile.Name));
}
}
}


Believe it or not I received a requirement change for this on the Monday immediately following the TDD Firestarter event so I figured there is no time like the present to put what I learned into action.  The result of my Test method now looks like this:


[TestFixture]
public class when_generating_the_pdf_file_name: using_the_pdf_regen_helper
{

[SetUp]
public override void Observe()
{
fileNamePattern = @"PDF_\d+_\d+_\d{4}_\d{2}_\d{2}T\d{2}_\d{2}_\d{2}.zip";
settings.DataReturnType = DataReturnType.FullDetailReport;

compressedFileName = PDFRegenHelper.GetCompressedFileName(ExecutionContext.ProviderMailboxing, settings, config);
filesInDirectory.AddRange(testDataDir.GetFiles("*.obj"));

packager = new JobZipPackager();
packagedFile = packager.Package(filesInDirectory, Path.Combine(testDataDir.FullName, @"PackagedFiles\" + compressedFileName));

}

[Test]
public void the_file_follows_the_standard_file_naming_convention()
{

Assert.IsTrue(Regex.IsMatch(packagedFile.Name, fileNamePattern), string.Format("The file {0} does not match the format of {1}", packagedFile.Name, fileNamePattern));

}
}


So tell me what you think... I am leaning more toward the new method. The one drawback to this approach is the differences are defined in the observe method which means that the use of the RowTest in this context goes away or maybe there is a clean way to use RowTests with this approach but at first glance I do not see it.