An ORM is NO excuse to violate the Single Responsability Principal
time to read 14 min | 2770 words

This post is directed at yours truly. Take a look at this class:

(Image from clipboard).png

The ISessionManager interface is provided by Windsor NHibernate Integration, it gives me an access to NHibernate's ISession, which in turn gives me an access to the database.

It is really easy to write code for this system, you pull some stuff from the database, minor processing, and it is done. It is testable, because you are using only interfaces. The problem is with the ease of testing. I couldn't quite put my hand on why my tests were cumbersome to write.

Take a look at this snippet:

[Test]

public void LogErrorIfFileHasNotArrived()

{

    MockRepository mocks = new MockRepository();

    ISessionManager sessionManager = GetSessionManagerAndSetupMocks(mocks, new ArrayList());

Where GetSessionManagerAndSetupMocks is defined:

 

private static ISessionManager GetSessionManagerAndSetupMocks(MockRepository mocks, IList list)

{

    ISessionManager sessionManager = mocks.CreateMock<ISessionManager>();

    ISession session = mocks.CreateMock<ISession>();

    ICriteria criteria = mocks.CreateMock<ICriteria>();

    Expect.Call(sessionManager.OpenSession()).Return(session);

    Expect.Call(session.CreateCriteria(typeof (FileState))).Return(criteria);

    Expect.Call(criteria.Add(null)).IgnoreArguments().Repeat.Twice().Return(criteria);

    Expect.Call(criteria.List()).Return(list);

 

    session.Flush();

    session.Dispose();

    return sessionManager;

}

And that is for a fairly simple test. This creates an environment where it is pretty hard to write the tests, because I need to setup quite a bit of code, and it has to be intimately familiar with the structure of the class under test. I am dealing more with infrastructure than with testing something that adds a real value.

I am currently refactoring all the usages of ISessionManager (and NHibernate) to satallite interfaces/classes, which allows me to transform the above test to this:

[ Test]

public void LogErrorIfFileHasNotArrived()

{

       MockRepository mocks = new MockRepository();

       ICheckFileArrivedDataHelper dataHelper = mocks.CreateMock<ICheckFileArrivedDataHelper>();

 

       MemoryAppender appender = new MemoryAppender();

       BasicConfigurator.Configure(appender);

 

       Expect.Call(dataHelper.GetFilesNotMarkedByWatcher(3)).Return(new FileState[0]);

 

       mocks.ReplayAll();

 

       CheckFileArrivedTask fileNotArrivedTask = new CheckFileArrivedTask(dataHelper, 3);

 

       fileNotArrivedTask.Execute();

 

       LoggingEvent loggingEvent = appender.GetEvents()[0];

 

       Assert.AreEqual(Level.Error, loggingEvent.Level);

 

       string msg = @"File type: 3 did not arrived in its period!";

 

       Assert.AreEqual(msg, loggingEvent.MessageObject);

 

       mocks.VerifyAll();

}

The part in large font replaced all the code in GetSessionManagerAndSetupMocks. Now the intent of the test is much clearer, and it is far easier to test this stuff. I no longer have to match every little thing that is going on there, instead, I can focus on the bigger picture.

I still need to test the implementation of the data helper, but that can be done in the integration tests.

Tweet Share Share 0 comments
Tags: