Make class visible to only test assemblies

Yesterday, i was spending my time at work trying to increase the test coverage for our solutions. It's such a hard work because everone wants to write unit tests, everyone want to have better coverage but all people seem to have a lot of stuff to finish before deadline. We talked about it every sprint retrospective and the coverage started to increase very slowly from last week. Hmmm, I always want to be in a TDD team but never had that chance. However, we accept the fact that we could implement the user story first write unit test for what we did and .... wait for bugs :D. It's given that we'd better have unit tests rather than have nothing. Now, the effort i spent was 3 hours that produced 27 unit tests which was my ambition to cover 100% a class which has 1 single method of about 50 lines of code. The reason for that big number of unit tests is that my method had so many cases. Well, it actually cover about 9x%, not really 100% as I said but the test does visit every single line of my class. After checking code in, I was quite happy with what the result and decided to update my twitter status. Right after 10 seconds, my colleague said: "WTF is that method?" Hmm, now look at the original method:

public override void OnActionExecuted(ActionExecutedContext filterContext)
{
    var model = filterContext.Controller.ViewData.Model as BusinessViewModel;
    if (model != null && !filterContext.Controller.ViewData.ModelState.IsValid && !GetModelUpdatedState(filterContext))
    {
        if ((model.Categories == null || model.Categories.Count == 0) &&
            (model.CategoryIds != null && model.CategoryIds.Count > 0))
        {
            var cats = ServiceFactory.BusinessService.GetBusinessCategoriesByIds(model.CategoryIds).ToList();
            model.Categories = Mapper.Map<List<BusinessCategory>, List<BusinessCategoryViewModel>>(cats);
        }

        if ((model.Features == null || model.Features.Count == 0) &&
            (model.FeatureTitles != null && model.FeatureTitles.Count > 0))
        {
            var features = ServiceFactory.BusinessService.GetBusinessFeaturesByTitles(model.FeatureTitles).ToList();
            model.Features = Mapper.Map<List<BusinessFeature>, List<BusinessFeatureViewModel>>(features);
        }

        if ((model.Tags == null || model.Tags.Count == 0) &&
            (model.TagNames != null && model.TagNames.Count > 0))
        {
            var tags = ServiceFactory.BusinessService.GetBusinessTags(model.TagNames).ToList();
            model.Tags = Mapper.Map<List<BusinessTag>, List<TagViewModel>>(tags);
        }

        if ((model.Owners == null || model.Owners.Count == 0) &&
            (model.OwnersIds != null && model.OwnersIds.Count > 0))
        {
            var owner = ServiceFactory.IdentityService.GetById(model.OwnersIds[0]);
            if (owner != null) model.Owners = new List<BusinessOwnerViewModel> { new BusinessOwnerViewModel { Id = owner.Id, Email = owner.Email } };
        }

        SetModelUpdatedState(filterContext, true);
    }

    base.OnActionExecuted(filterContext);
}

There are quite alot of cases, isn't it. I think if I want to cover 100% of this one, I should have written 6 + 8 + 8 + 8 + 10 unit tests. My colleague raised a concern about this method is very hard to maintain. If someone changes some small thing, it could break my tests. Indeed, there is a way to break this method into smaller methods that would be very easier to test and maintain. I was suggested to split this method into smaller internal methods and use [InternalsVisibleTo] to make those internal methods visible to my test assembly. So my new methods are:

public override void OnActionExecuted(ActionExecutedContext filterContext)
{
    var model = filterContext.Controller.ViewData.Model as BusinessViewModel;
    if (model != null && !filterContext.Controller.ViewData.ModelState.IsValid && !GetModelUpdatedState(filterContext))
    {
        UpdateCategories(model);

        UpdateFeatures(model);

        UpdateTags(model);

        UpdateOwners(model);

        SetModelUpdatedState(filterContext, true);
    }

    base.OnActionExecuted(filterContext);
}

internal void UpdateOwners(BusinessViewModel model)
{
    if ((model.Owners == null || model.Owners.Count == 0) &&
        (model.OwnersIds != null && model.OwnersIds.Count > 0))
    {
        var owner = ServiceFactory.IdentityService.GetById(model.OwnersIds[0]);
        if (owner != null) model.Owners = new List<BusinessOwnerViewModel> { new BusinessOwnerViewModel { Id = owner.Id, Email = owner.Email } };
    }
}

internal void UpdateTags(BusinessViewModel model)
{
    if ((model.Tags == null || model.Tags.Count == 0) &&
        (model.TagNames != null && model.TagNames.Count > 0))
    {
        var tags = ServiceFactory.BusinessService.GetBusinessTags(model.TagNames).ToList();
        model.Tags = Mapper.Map<List<BusinessTag>, List<TagViewModel>>(tags);
    }
}

internal void UpdateFeatures(BusinessViewModel model)
{
    if ((model.Features == null || model.Features.Count == 0) &&
        (model.FeatureTitles != null && model.FeatureTitles.Count > 0))
    {
        var features = ServiceFactory.BusinessService.GetBusinessFeaturesByTitles(model.FeatureTitles).ToList();
        model.Features = Mapper.Map<List<BusinessFeature>, List<BusinessFeatureViewModel>>(features);
    }
}

internal void UpdateCategories(BusinessViewModel model)
{
    if ((model.Categories == null || model.Categories.Count == 0) &&
        (model.CategoryIds != null && model.CategoryIds.Count > 0))
    {
        var cats = ServiceFactory.BusinessService.GetBusinessCategoriesByIds(model.CategoryIds).ToList();
        model.Categories = Mapper.Map<List<BusinessCategory>, List<BusinessCategoryViewModel>>(cats);
    }
}

And i put the follwowing line to AssemblyInfo.cs

 
[assembly: InternalsVisibleTo("MyProject.Web.Mvc.Test")]

So, those internal methods are more simple than then it's easier to setup and test those things without paying any care about ViewModel, ModelState. etc because they would be tested in different tests. The complexitity is still there, but now people can changes and are not afraid to break the tests easily like before. The number of lines of my test class reduced from 600 to 500, not much actually but I like this way.