Rhino Mocks and OpenAccess ORM
jdl
public static Guid AddUser(string name, int legacyId = -1) { try { using (var model = new VerifyModel()) { User userEntity = GetUserEntity(name); if (userEntity != null) { return userEntity.Guid; } var newUser = new User { Name = name, LegacyId = legacyId }; model.Add(newUser); model.SaveChanges(); UserContainer.Instance.Refresh(); return newUser.Guid; } } catch (Exception exception) { ErrorLogWriter.Write(MethodBase.GetCurrentMethod().Name, exception); } return Guid.Empty; }Update method handles optimistic verification exceptions but at the end of the day simply performs a sql update.public static bool UpdateUserDetails(Guid userGuid, IEnumerable<ElementAttributeValues> elementAttributeValues) { try { var historyEvents = new List<string>(); using (var model = new VerifyModel()) { var user = model.Users.SingleOrDefault(p => p.Guid == userGuid); if (user == null) { return false; } var enumeratedElementAttributeValues = elementAttributeValues asList<ElementAttributeValues> ?? elementAttributeValues.ToList(); try { UpdateUserEntity(enumeratedElementAttributeValues, ref user, refhistoryEvents); model.SaveChanges(); } catch (OptimisticVerificationException optimisticVerificationException) { ErrorLogWriter.Write(string.Format("An optimistic verification exception occured in {0} and the commit will be retried. The error message is: {1}.", MethodBase.GetCurrentMethod().Name, optimisticVerificationException.Message)); model.Refresh(RefreshMode.OverwriteChangesFromStore, user); // retry update UpdateUserEntity(enumeratedElementAttributeValues, ref user, refhistoryEvents); model.SaveChanges(); } foreach (string historyEvent in historyEvents) { HistoryWriter.Write(userGuid, historyEvent); } return true; } } catch (Exception exception) { ErrorLogWriter.Write(MethodBase.GetCurrentMethod().Name, exception); } return false; }
Patrick Steele
1. The methods you showed are static. At some point, you may want to
mock out these methods when testing other parts of your code. Static
methods are not mockable with Rhino.Mocks. You should make the
virtual instance methods.
2. What is the "VerifyModel" you create?
3. If model.Add() and model.SaveChanges() are virtual, you can mock
them, but you'd have to change the way the 'model' is created (inject
it or provide a factory to create it).
4. "UserContainer.Instance.Refresh()" --> looks like another static
method. Those can't be mocked/stubbed with Rhino.Mocks
---
Patrick Steele
http://weblogs.asp.net/psteele
> You received this message because you are subscribed to the Google Groups
> "Rhino.Mocks" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/rhinomocks/-/QqeKVDy-DTkJ.
> To post to this group, send email to rhino...@googlegroups.com.
> To unsubscribe from this group, send email to
> rhinomocks+...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/rhinomocks?hl=en.
jdl
bill richards
I'm not 100% clear about where you are considering adding ObjectScope in an overloaded constructor, but I would recommend that you do not do it, this technique/trick has come to be known as "bastard injection" and it is a code smell.
You are on the right track by injecting ObjectScope, but I would seriously recommend NOT overloading the constructor, but rather have a single constructor which is provided with all of the class' dependencies, there are various reasons why you should not use bastard injection if you can avoid it (which generally means if it's a class that you have written or rather, when the code used is not generated by an automaton), the most important of these reasons is that you will most likely be using the class differently under test than you will in released code; which then beggars the question of what your tests are actually testing ... turns out that usually they are testing code put in place specifically to enable testing ... which means that you will get false positives/negatives, you could also find that in your released code you might have some dependency issues since the code has been modified in some "hacky" manner in order to use the injected dependency under test, or to new it up somehow in live.
Using Rhino, you will not be able to mock your CRUD service unless it's members are instance and virtual. In my opinion, the CRUD service should not be static anyhow ... there are other objects that have dependencies upon this static class, make that dependency explicit in your design (i.e. inject an instance of the CRUD service). There are times and places for statics, but most of the time they are not where and when they are being used ... something which I find quite annoying about ReSharper is its constant insistence upon informing one that a member can be made static.
So the points that Patrick has raised are valid, however I'm not entirely sure of what the real issue is that needs attention ... sometimes I'm a bit dim see?
bill richards
I think that maybe the dependency graph is your issue. I think what you are trying to do is to test your POCO objects, which you are finding difficult to do because in the current design there are some hidden, hard coded volatile dependencies.
Once you have broken these dependencies your problem will disappear (at least I think that's the case, if I've interpreted the combination of your question and the Telerik link you posted).
jdl
bill richards
Before I start ... I know that this is a RhinoMocks thread, but please accept that I've not been using Rhino for the past seven months and am a little rusty on the syntax, so in my tests take them as pseudo code rather than the actual tests, because the syntax is most likely wrong.
.. and oh yeah, it's a long one.
We have a component upon which the application currently has a dependency. The component (OpenAccess) is not under our control, and we have to take it as written that it has been fully tested prior to release, and therefore it does what it needs to do.
public static class DataAccess
{
public static Guid AddUser(string name, int legacyId = -1)
{
try
{
using (var model = new VerifyModel())
{
User userEntity = GetUserEntity(name);
if (userEntity != null)
return userEntity.Guid;
var newUser = new User { Name = name, LegacyId = legacyId };
model.Add(newUser);
model.SaveChanges();
UserContainer.Instance.Refresh();
return newUser.Guid;
}
}
catch (Exception exception)
{
ErrorLogWriter.Write(MethodBase.GetCurrentMethod().Name, exception);
}
return Guid.Empty;
}
}
There are a couple of dependencies in this first method, which need to be made explicit, namely ErrorLogWriter, and UserContainer.
There are three approaches available to us once we have accepted that we need to address the issue by using Dependency Injection (DI): Method Injection; Property Injection; or Constructor Injection.
To use constructor injection would require us to make the class non-static, make the method non-static, create a constructor, and pass both ErrorLogWriter and UserContainer in as parameters.
public class DataAccess
{
private readonly LogWriter _errorLogWriter;
private readonly Container _userContainer
public DataAccess(LogWriter errorLogWriter, Container userContainer)
{
_errorLogWriter = errorLogWriter;
_userContainer = userContainer;
}
public Guid AddUser(string name, int legacyId = -1)
{
try
{
using (var model = new VerifyModel())
{
User userEntity = GetUserEntity(name);
if (userEntity != null)
return userEntity.Guid;
var newUser = new User { Name = name, LegacyId = legacyId };
model.Add(newUser);
model.SaveChanges();
_userContainer.Refresh();
return newUser.Guid;
}
}
catch (Exception exception)
{
_errorLogWriter.Write(MethodBase.GetCurrentMethod().Name, exception);
}
return Guid.Empty;
}
}
This involves a considerable amount of change. I won't go in to Property Injection because this is really only achievable when utilising a DI container, which we have not brought in to the solution; but we can look at Method Injection, which would result in the least amount of change for our class and for its callers.
public static class DataAccess
{
public static Guid AddUser(LogWriter errorLogWriter,
Container userContainer,
string name, int legacyId = -1)
{
try
{
using (var model = new VerifyModel())
{
User userEntity = GetUserEntity(name);
if (userEntity != null)
return userEntity.Guid;
var newUser = new User { Name = name, LegacyId = legacyId };
model.Add(newUser);
model.SaveChanges();
userContainer.Refresh();
return newUser.Guid;
}
}
catch (Exception exception)
{
errorLogWriter.Write(MethodBase.GetCurrentMethod().Name, exception);
}
return Guid.Empty;
}
}
now our DataAccess class does not know about the type of LogWriter being used, nor does it know about the type of Container being used, so now these things are not dependent upon some static objects, and it is clear to any caller that this member requires a LogWriter and a Container. As far as the DataAccess object is concerned however, so long as they simply ARE instances of LogWriter and Container, then that is all that matters.
We have quite easily broken the dependencies in such a way that we are now able to Mock both logging and Container interaction. For example
public class DataAccessTestFixture
{
[Test]
public void WhenNewUserIsSuccessfullyAdded_ThenTheContainer_ShouldBeRefreshed()
{
// Arrange
var mockLogWriter = MockRepository.GenerateMock<LogWriter>();
var stubContainer = MockRepository.GenerateStub<Container>();
// Act
DataAccess.AddUser(mockLogWriter, stubContainer, "test user");
// Assert
stubContainer.AssertWasCalled(c => c.Refresh()).Times(1);
}
[Test]
public void WhenAddingANewUserFails_ThenTheLogWriter_ShouldBeInvoked()
{
// Arrange
var exception = new Exception();
var stubLogWriter = MockRepository.GenerateStub<LogWriter>();
var stubContainer = MockRepository.GenerateStub<Container>();
/* enables us to mock a failure */
stubContainer.Stub(c=> c.Refresh()).Throws(exception);
// Act
DataAccess.AddUser(mockLogWriter, stubContainer, "test user");
// Assert
stubLogWriter.AssertWasCalled(w => w.Write("AddUser", exception)).Times(1);
}
}
This however will only be possible if LogWriter.Write() and Container.Refresh() are virtual members. If this is not the case then we can introduce our own interfaces for logging and container interaction, we can call them ILogger and IContainer, and we can expose all of the public members of LogWriter and Container respectively in those interfaces. The implementation of those interfaces will simply be invocations of those members on the actual LogWriter and Container instances. This will then allow us to mock logging and container interaction as per the above psuedo code.
This ultimately results in us having a data access layer which is decoupled from the actual implementation of the data storage layer. Now that we are using interfaces and DI, we can quite easily swap out the data storage mechanism, which is effectively what we have done by using a mocking framework.
We have, however, still not decoupled our application from the data access layer. Undoubtably there are numerous references to DataAccess throughout our codebase, it could be that they are all contained within a single class or assembly, but they are definitely there. So we really need to apply the same logic to our DataAccess as we have to the logger and container ineteraction classes; this means that we should really make DataAccess a non-static class, which means that "maybe we should have gone straight for Constructor Injection over Method Injection?". My personal view is yes we should, but it all depends really on how much is going to change in our codebase as a result; remember that refactoring is about making small changes and keeping our tests running green.
When we have finished our refactoring, we will most likely end up with something akin to the following, which looks remarkably similar to our initial discussion of Constructor Injection.
public interface IDataAccess
{
Guid AddUser(string name, int legacyId = -1);
}
public class DataAccess : IDataAccess
{
private readonly ILogger _logWriter;
private readonly IContainer _container
public DataAccess(ILogger errorLogWriter, IContainer userContainer)
{
_logWriter = errorLogWriter;
_container = userContainer;
}
public Guid AddUser(string name, int legacyId = -1)
{
try
{
using (var model = new VerifyModel())
{
User userEntity = GetUserEntity(name);
if (userEntity != null)
return userEntity.Guid;
var newUser = new User { Name = name, LegacyId = legacyId };
model.Add(newUser);
model.SaveChanges();
_container.Refresh();
return newUser.Guid;
}
}
catch (Exception exception)
{
_logWriter.Write(MethodBase.GetCurrentMethod().Name, exception);
}
return Guid.Empty;
}
}
Now, throughout the application, we should only speak in terms of IDataAccess, ILogger, and IContainer, which means that we have completely decoupled the callers from any specific implementations. We can test everything in isolation (erm, I know I didn't test drive this refactoring, but it was omitted for berevity -as if this post is brief!), and we can even change our data access layer and/or our data storage mechanism on a whim (or for genuine reasons) without the need to recompile the whole application.
I know I've rambled, and now that I've reached this point I'm not sure if I'm actually answering any questions! So, the submit button will be pressed and any responses are welcome.
bill richards
You are absolutely right to be breaking the SQL db dependency. Your UNIT TESTS should never go near a database, the database should only really come into the picture at the Integration Test level, I have been using Bob Martin's FitNesse for a while now, and in fact we are rolling it out as the Integration Test Framework across the entire development department here.
I don't particularly like the tool itself, but I really do like what the tool represents and enables and in the absence of anything better, it is fit for purpose (if you'll excuse the pun). It enables us to capture all domain information and our user stories within the Wiki, and then to hook those stories up to acceptance tests by using the FITRunner. This means that the Product Owners can use the same tool as the developers to write out their requirements in plain English, which ultimately results in our domain specific language, and ALL of it is captured within our tests. This of course has the added benefit that any developer can join any project and get up to speed in a matter of hours simply by reading and running the test suite :)