How to mock UrlHelper?

Sometime, you need to write unit test for an action method that needs a UrlHelper. But I'm sure you will never can mock that stuff since it's a concrete class and it's methods are not virtual. Let's say we have following action method and want to test it:

[Authorize]
public ActionResult LogOnAs(Guid userId, string returnUrl)
{
    // Logic to login user by Id ...
    
    if (Url.IsLocalUrl(returnUrl))
    {
        return Redirect(returnUrl);
    }

    return RedirectToAction("Index", "User");
}

I did some research, experimented with some results and find that extracting the UrlHelper methods to an interface then make a wrapper class to implement that interface is the best way. We need to define a new property to the BaseController like below.

 
public new IUrlHelper Url {get; set;}
 

Then in the unit test project, after initializing the controller, you can mock the interface easily. You probably need only some methods from the UrlHelper so I would recommend you extract only those required methods to IUrlHelper. For example:

public interface IUrlHelper
{
    string Action(string actionName, string controllerName);

    string Action(string actionName, string controllerName, object routeValues);

    string Action(string actionName, string controllerName, RouteValueDictionary routeValues);
    
    bool IsLocalUrl(string url);
}

Finally, we can create an adaptor class like below to delegate all method calls to the real UrlHelper object:

public class UrlHelperAdaptor : UrlHelper, IUrlHelper
{
    internal UrlHelperAdaptor(RequestContext requestContext)
        : base(requestContext)
    {
    }

    internal UrlHelperAdaptor(RequestContext requestContext, RouteCollection routeCollection)
        : base(requestContext, routeCollection)
    {
    }

    public UrlHelperAdaptor(UrlHelper helper) 
        : base(helper.RequestContext, helper.RouteCollection)
    {
    }
}

Apparently, we need to initialize the new Url property in the BaseController to make the real code work normally:

protected override void Initialize(System.Web.Routing.RequestContext requestContext)
{
    base.Initialize(requestContext);
    Url = new UrlHelperAdaptor(base.Url);
}

Now, the controller method is fully testable.

[Test]
public void LogonAs_should_return_RedirectToRouteResult()
{
    // Arrange
    var controller = new AccountController();
    /* Create a mock of IUrlHelper */
    controller.Url = Moq.Mock.Of<IUrlHelper>(x => x.IsLocalUrl(It.IsAny<string>()) == false);     

    // Action
    var result = controller.LogOnAs(Guid.NewGuid(), "any-return-url") as RedirectToRouteResult;

    // Assert
    result.Should().Not.Be.Null();
}

Using this approach can help you test any class that depends on IUrlHelper such as custom UrlHelper classes. Cheers

Read more »

Never give a chance to make something wrong

Recectly, I've penetrated the truth that if you don't want others do something (bad), never ever give them a chance to do that. It's true in both my real life and in programming. Since my blog is just about software programming, I would like to write about some funny mistakes that people often make. Well, I did make some of them in the past :D

1/ You may forget about the ability of the constructor.

Let's look at following code:

public class UserViewModel
{    
    // These properties must not be null
    public string FirstName { get; set;}
    public string LastName { get; set;}
    public string Email { get; set;}    
}

We can see from the comment in the class above, the author want 3 propreties must have value. It oould be his intention but someone else may need a instance of this class, so he instantiates a UserViewModel object without setting the values for any of those 3 properties. And that could cause problems at runtime since it's not what the author of UserViewModel want. So instead of doing this, we can make the language ensuring that requirement for us:

using System.Diagnostics.Contracts;
public class UserViewModel
{    
    public UserViewModel(string firstName, string lastName, string email)
    {
        Contract.Assert(!string.IsNullOrEmpty(firstName), "firstName must have value");
        Contract.Assert(!string.IsNullOrEmpty(lastName) , "lastName must have value");
        Contract.Assert(!string.IsNullOrEmpty(email)    , "email must have value");
    
        FirstName = firstName;
        LastName = lastName;
        Email = email;
    }
    
    // These properties must not be null
    public string FirstName { get; private set;}
    public string LastName { get; private set;}
    public string Email { get; private set;}    
}

2/ You won't need a private field for a public property.

My team is applying BDD and using PageObject pattern. A guy in a team created a class that have a something like:

private RegistrationStatus _registrationStatus;

public RegistrationStatus Status
{
    get { return _registrationStatus; }
}

What an unnecessary 4 lines of code. You know what, just a few weeks later, the solution is full of these mistakes because alot of people are writing BDD tests and they like copy & paste. So, just change it to 1 line of code version:

 
public RegistrationStatus Status {get; private set;}
 

Well, if you don't want to see these stuff in the code, don't make any thing like this because we're all doing the "COPY & PASTE" job.

3/ If you have not finished something, use NotImplementedException

Apparently working in a team, someone could create a service interface with some methods then the other could use the service for presentation layer. Here is an example:

public interface IUserService
{
    User Get(Guid id);
    int Count();
}

public UserService : IUserService
{
    public User Get(Guid id)
    {
        return null;
    }
    
    public int Count()
    {
        return -1;
    }
}

Everything is fine when compile but why return something like those when they're useless value. Well, if we need an implementation of IUserService, why not create a Mock or a fake object then everyone knows it's a fake and it will never been used in the production. However, it could be acceptable if we are creating a NullObject, but I mostly throw NotImplementedException instead of doing nothing for a void method or return meaningless value for functions.

4/ it's not really a singleton

Everyone knows about singleton. It supposes to be the easiest most simple design pattern. Let's see this code:

public class Helper
{
    private static Helper _instance;
    public static Helper Instance
    {
        get
        {
            if (_instance == null)
            {
                _instance = new Helper();
            }
            return _instance;            
        }
    }
}

Okey, it's just a very simple implementation. But the idea is that it allows only 1 instance of Helper in the system and whenever you want that instance, you must access it from Helper.Instance. Cool but it's not really singleton. Anyone can create an object of Helper easily because by default, the constructor is public so make it private if you don't want people use it.

5/ We should follow Dependency Inversion Principle

This principle is the one that affects my way of thinking. If you read about the Onion architecture, you'll see that architecture is not complicated. It just applies the DIP and make a flashy name. If we apply DIP strickly, we'll never put implementation and interface together in a class library. Instead, put them in separated project. Classes depend on the interface will never know about the concreate implementation of that interface and they will be connected by some sort of IOC library. However, doing that way can make you create alot of projects in one solution. Honestly I have never done anything like this but, but I think Onion architecture is something to consider when you design new application. I prefer isolating the business implementation from the technology we use so any changes of technology will make a little affect to our code.

Read more »

Basic Unit Of Work Implementation

    I bet you must have heard about Unit Of Work. If you have not, you can find the discription here : http://martinfowler.com/eaaCatalog/unitOfWork.html.    You can change the database with each change to your object model, but this can lead to lots of very small database calls, which ends up being very slow. Furthermore it requires you to have a transaction open for the whole interaction, which is impractical if you have a business transaction that spans multiple requests. The situation is even worse if you need to keep track of the objects you've read so you can avoid inconsistent reads.     Nowadays, DDD is one of the most famous design methodologies in the developer community. Even though 95% of all software applications fall into the “not so good for using DDD” categories, some practices and ideas of DDD can be used in many .NET projects, especially Repository and Domain Service. I have seen many implementations of Repository pattern and UnitOfWork, but i am not so happy with any because of the following reason. We tend to use some kinds of ORM like NHibernate or EF as the technology behind Repository implementation. These frameworks mostly have the Unit Of Work built in, so I think the Domain Service layer should not call IUnitOfWork.CommitChange() when it needs to persist something to the repository. In some complex services, we call a service method which may execute different service methods; and because a service method (that requires changing the database) should call IUnitOfWork.CommitChanges. Therefore, as a result, IUnitOfWork.CommitChanges will be called many times in 1 business transaction.     Another example could be the controller which can use several domain services to execute an action method. Let's say a website user wants to register at your website. When he submits the registration form, the RegisterController will call the UserService to create a user to the repository and another service create a WelcomeEmail in the Email repository. That means there are 2 records are got involed in this transaction (assume that you use SQL as the data source). And a proper UnitOfWork implementation should be called only 1 time to finish the saving task. However, it's easy to see the implementation like this:

[HttpPost]
public ActionResult Register(RegisterUserViewModel viewModel)
{
    if (!ModelState.IsValid)
    {
        return View(viewModel)
    }
    
    UserService.Create(user);
    EmailService.Create(welcomeEmail);
    
    return RedirectToAction("Index", "Home");
}

In the user service implementation :

public void Create(User contact)
{    
    _userRepository.Add(contact);
    _unitOfWork.CommitChange();
}

And in the email service implementation :

public void Create(Email email)
{    
    _emailRepository.Add(email);
    _unitOfWork.CommitChange();
}

    It's not difficult to see that the UnitOfWork will call method CommitChange() two times. If you use EF behind the Repository implementation, I'm sure the code above will make your breakpoint stop at method ObjectContext.SaveChanges() 2 times. For me it’s not “Unit Of Work” if we do like that. It would be nice if we could have a way to call the CommitChange() only when we actually need, it could be the end of every http request. Therefore, the domain services should never call CommitChange() on your UnitOfWork.     Base on this idea which is a bit similar to "Open Session In View" pattern, I created a basic implementation of UnitOfWork below:

public interface IUnitOfWork
{
    void CommitChange();
    bool IsDirty { get; set; }
}

The domain service will use a UnitOfWorkScope to ensure the change will be committed in the end or immediately based on the UnitOfWorkScopeOption:

public enum UnitOfWorkScopeOption
{
    Shared,
    CommitImmediately
}

public class UnitOfWorkScope : Disposable
{
    private readonly UnitOfWorkScopeOption _option;
    private readonly IUnitOfWork _unitOfWork;

    public UnitOfWorkScope() : this(UnitOfWorkScopeOption.Shared)
    {
    }

    public UnitOfWorkScope(UnitOfWorkScopeOption option)
    {
        _option = option;
        _unitOfWork = ObjectFactory.GetInstance<IUnitOfWork>(); // I'm using StructureMap
    }

    protected override void DisposeCore()
    {
        switch(_option)
        {
            case UnitOfWorkScopeOption.CommitImmediately:
                {
                    _unitOfWork.CommitChange();
                    _unitOfWork.IsDirty = false;
                    break;
                }
            default:
                _unitOfWork.IsDirty = true;
                break;
        }
    }
}

I will change the service methods like below:

public void Create(User contact)
{    
    using (new UnitOfWorkScope())
    {
        _userRepository.Add(contact);    
    }
}

public void Create(Email email)
{    
    using (new UnitOfWorkScope())
    {
        _emailRepository.Add(email);
    }
}

The default constructor of UnitOfWorkScope would set the scope option to Shared. That means it would do nothing but mark the UnitOfWork.IsDirty to true. In the end, we can base on that value to determine whether we should call commit change. I put this logic in the base controller, so after the ActionExecuted, the UnitOfWork will be invoked to do the saving job.

protected override void OnActionExecuted(ActionExecutedContext filterContext)
{
    base.OnActionExecuted(filterContext);

    if (filterContext.Exception == null || filterContext.ExceptionHandled)
    {
        var unitOfWork = ObjectFactory.GetInstance<IUnitOfWork>();
        if (unitOfWork != null && unitOfWork.IsDirty)
        {
            unitOfWork.CommitChange();
        }
    }    
}

I made a test project and looks like it run at least 2 times faster than the the original way when I use UnitOfWorkScope. There is a very cool implementation of UnitOfWork at NCommon. You should check it out. Finally, here is the test project. Cheers

Read more »

Some issues with BDD Specflow & WatIn

    Last few weeks, our team is doing alot of experiments on BDD. We read several books trying to find out the proper way to apply BDD in .NET project using Specflow and WatIn. There are some good articles out there indeed but for me, they're quite basic. These articles do the good works to help me start the first steps but after walking several steps, some issues comes up: - What is the good way to initialize some test data before a scenario? - What is the good way to write feature files? - When would we run these UI tests? - What is the good way to implement step definitions that faciliate reusable and easy maintenance? - What to do if the feature requires some interaction to external sources?     Honestly, we've not had the best answers in 3 weeks. Some other issues would come up in the future and we might have better answers for what we want. However, I'm quite happy with following solutions:

1/ What is the good way to initialize some test data before a scenario?

This question came up when we met some feature that requite existing data. Let's look at following scenario:

Scenario: Log-In
Given I enter a previously registered email address / password combination
When I click log-in
Then the site will authenticate me and remember me for the remainder of my session

    In order to execute this test, it assumes that we have a registered user in the system. I tried to google a lot to find the answer for this question. Fortunately, I found it while reading the cuke4ninja document. Basicaly, there are 3 methods to do this: - Direct access to the database - Call to the business layer API - Creating records using the UI     Our team had a big argument about it for 2 weeks. The reason is we devided developers into Presentation and Service teams. These 2 teams will work parallel together, one to build the services include backend code like repositories, services, etc; one to implement the web and UI test that utilises the service made by the other team. The thing is presentation team will not know and should not know anything about what the other team would do. For example, the presentation team should not know anything about database schema. We should have agrement on the a service the presentation team needs. So the presentation team will mock the service to be able to run the web while the required service is being implemented by the other team. And thus, the presentation team could call this service to make some initialisation for the test. Personally, I don't like this idea because of following reasons: - Ultimately, the UI tests will talk to the database, not the mocked service because we want the end to end task, so writing some codes that access to the database to clean up/ initalise is not a big deal. - For me, The UI test code should only be methods that are called to interact with user interface rather than depending on some Service API - The UI test would be run multiple times against the real database, we definitely need some way to clean up the records made be previous test run. It could be difficult to delete a record since it could require cascade delete. If I still keen to use this way, I have to modify the service and add method Delete for some related services. And I think it is not a best practice because we should not change the code design just for the test run.     Therefore, a small script to empty tables, re-insert data to tables is quite good for me. This script will be made by the service team and they need to ensure that the script is up to date to make the whole test pass. The presentation team should use UI to create records they need for the test. Everything will be much simpler if the developer can get involve in implementing everything from service code to presentation code.

2/ What is the good way to write feature files?

    The solution above leads to this question. For some complex scenarios that require complex exisiting data, writing UI code for it is not a proper way. Even though the script to initialise data could do this perfectly, but let me say again, the presentation team will not know what the service team would do include that scripts. So if i don't want to call the service api to do the job, I must spend effort to write the UI code for initialisation stuff.     So, why don't we re-write the scenario in a way that reduces the need for data initialisation. Let's say we have this scenario:

Given I am a new user
And I am on the Registration page
When I enter the following basic details
 | Forename | Surname | Email                | Password |
 | Van      | Nguyen  | van.nguyen@email.com | 123456   |
And I submit the form
Then the site will save my details
And show a message saying 'Registration successful'

Should we write:

Given I am a new user
And I am on the Registration page
When I enter the my basic details with an email that's never been used
And I submit the form
Then the site will save my details
And show a message saying 'Registration successful'

    Then we could implement the "Never been used" by using an email that contains a GUID or DateTime.Now.Ticks. So that test won't need any database clean up or initialisation. It's easier to create something that is unique rather than making sure something that does not exist in the system. Positive way is always better than negative way, isn't it :D. In the sample solution, I utilised the @tag feature of Specflow, so any scenarios with the tag @requireCleanDb would have the step hook to clean and initialise the database to the original state.

3/ When would we run these UI tests?

    I must say that I'm a bad developer. I hate to fix bugs, expecially bugs that caused by others. In our project, I implemented a feature and later on, any bugs on that feature were assigned to me. That was fine but the thing is when fixing these bugs I found that the root reason was some changes in javascript or some guys just removed the property of a View Model without fixing the View. Unit test does the very good work that can make sure any checkin will not break the existing code, but it cannot prevents bug caused by a small change in javascript. UI automation test could do this, but it has a problem with running time so It could not be executed for every checkin. Then my pain will still be there as my team agrees to run the tests every day at night. And in the morning, developers will receive the test run report. However, every developer could run the tests before checkin to make sure he would not ruin something.

4/ What is the good way to implement step definitions that facilitate reusable and easy maintenance?

    This is my most interesting question. I couldn't resist the temptation to use WatIn classes to implement the test when I read the how to document. But when I read the other book cuke4ninja and I watched this mvcConf: BDD in ASP.NET MVC using SpecFlow, WatiN and WatiN Test Helpers, I knew I need some kind of abstraction. I need wrappers for WatIn classes. I implemented some "technical" classes like Page, Form and "Workflow" classes for the flow of the UI tests. I decided to split these things into separated class libraries, most of the classes were internal except the "workflow" classes which will be used in the step definition implementation. So in the end, my integration test project just know about workflow classes without knowing anything about Page, Form or WatIn. Actually, at first, I let the UI test methods utilise Page, Form objects along with WorkFlow classes but then I thought it would be better to make Page and Form internal. So instead of doing like this:

[Then(@"the site will authenticate me and remember me for the remainder of my session")]
public void ThenTheSiteWillAuthenticateMeAndRememberMeForTheRemainderOfMySession()
{
      Assert.IsTrue(Page.Current.ContainsText("Log Off"));
      Assert.IsTrue(Page.Current.Url == "/Home/Index" || Page.Current.Url == "/");
}

I prefer doing like this:

[Then(@"the site will authenticate me and remember me for the remainder of my session")]
public void ThenTheSiteWillAuthenticateMeAndRememberMeForTheRemainderOfMySession()
{
      Assert.IsTrue(LoggedInUser.Current.IsLoggin());
      Assert.IsTrue(LoggedInUser.Current.IsAtHomePage());
}

    It makes the test method easy to read and certainly, the method is reusable. Please checkout the example project in the end of this post for more detail. Please note that I implement it based on my perspective and I don't say It the best way. So please correct me.

5/ What to do if the feature requires some interaction to external sources?

Or should we test if the feature requires interation to external resources? Let's say we have following scenarios:

Scenario: Forgot Password - Password Retrieval
Given I enter an email address that was previously registered with the site
When I click 'send email'
Then I receive a message telling 'An email was sent to you to reset your password!'
And I receive an email containing a link to reset my password

    If we want to write the test for this, we might need a mail box such as Gmail. We'll simulate user interaction on the mail box to click on the forget password link, etc. So what happens if the Gmail change their UI which could make our test fail. Personally, I still like to test this scenario as a normal user instead of creating some kind of mocks to have the reset link. Because I think the "end to end test" is testing the appliation just like the normal user use it. And these kind of scenarios would just be a few so It will not be a big deal. Okey, what's if they are alot? Well, I think abstraction of the external interaction activities could be an option. Talking to external resources is the common thing of a software. We use ORM to talk to database, they could change the library anytime, so what we do? We update the library. We call to 3rd API like Facebook API, Twitter API, Credit card processing service, etc. They could change their API anytime, so what we do? We make the wrapper to these API then we could switch to better service anytime. That's it. So I believe abstraction would be a good solution for these kinds of thing. However, it's still a debate in my team whether it could add much value to write UI test for these kinds of scenario. Perhaps the time will find the answer for this question.     Hmmm, so far so good. Please checkout the demo project. It contains my basic idea above. Again, I don't say It's the best way of using WatIn or implementation of BDD. There must be other issues because these features are very basic. Everything here is based on my personal perspective and I'm very gratefull if any readers of this post would give me your idea to make it better, or just tell me I was wrong at some points. Thanks for reading. Code: Download

Read more »

Defining Custom Functions in Entity Framework

Well, I's been 1 month since my last post. Today, I'm gonna write something about the well-known not supported issue of Entity Framework. I intended to write a LinQ to Entity query that would require a comparison and conversion between uniqueidentifier and string value in SQL database. The query looks like this:

var items = from x in XRepository.GetAll()
            join y in YRepository.GetAll() on x.UniqueId.ToString() equals y.StringValue into xy
            from z in xy.DefaultIfEmpty()
            select x;

I wrote the unit test and the test run through the code without any problem. But when I run the application against the real SQL database, there would be a following exception: System.NotSupportedException: LINQ to Entities does not recognize the method 'System.String ToString()' method, and this method cannot be translated into a store expression. After a while googling, I realized that there are not any built-in support to achive what I want. However, in EF4, there is a class named: SqlFunctions.StringConvert that can convert from double to string. Moreover, it can be used in LinQ to Entity queries. That class led me to the EdmFunctionAttribute and then I decided to create something similar to convert from uniqueidentifier to nvarchar for my need. I definitely need a custom SQL Function for conversion:

CREATE FUNCTION [dbo].[GuidToString]
(
    @guid as uniqueidentifier
)
RETURNS varchar(50)
AS
BEGIN
    return CONVERT(varchar(50), @guid)
END");

What I do next is to modify the EDMX file and add a small xml code:

<Function Name="GuidToString" 
          Aggregate="false" 
          BuiltIn="false" 
          NiladicFunction="false" 
          IsComposable="true" 
          ParameterTypeSemantics="AllowImplicitConversion" 
          Schema="dbo" ReturnType="varchar">
    <Parameter Name="guid" Type="uniqueidentifier" Mode="In" />
</Function>

I just find </Schema></edmx:StorageModels> and add the xml code before it. Because the function would return something, so the IsComposable must be set to true and we must provide the ReturnType value for the function. After that, I create a static class like below. It can be put anywhere in your solution.

[EdmFunction("My.Namespace.Store", "GuidToString")]
public static string GuidToString(Guid guid)
{
    // This code will never been run against real SQL database
    // This will help any test requires this method pass
    return guid.ToString().ToUpper();
}

The namespace I provide to EdmFunctionAttribute is really important. It must match the namespace in the EDMX file. You can easily find the namespace as an attribute of the xml node "Schema" when viewing the EDMX file using any XML editor. Finally, I can change the code to use the method like below:

var items = from x in XRepository.GetAll()
            join y in YRepository.GetAll() on GuidToString(x.UniqueId) equals y.StringValue into xy
            from z in xy.DefaultIfEmpty()
            select x;

Cheers

Read more »