Unit testing a legacy ASP.NET application. A personal journey – part 1
These series of posts will cover my journey into getting a legacy (i.e. no unit tests) ASP.NET web application under automated unit test coverage, the goals, decisions, techniques and pitfalls we got into as we rolled out this initiative.
The context
One of the products of the company I work for is a SaaS application that was developed in ASP.NET in the early days of v2.0. As a SaaS provider we are always making improvements to the application and adding new functionality as our clients and the market requires.
The application is not that big, it has about 500k lines of code but it’s data intensive and makes extensive use of strongly typed data sets and table adapters as the data layer and extensive use of ASP.NET server controls in the presentation layer.
Why did we wanted to do this?
As our application is constantly changing and being developed, we constantly ran into the all too familiar problems and growing pains:
- Bugs were being introduced into one part of the system when we changed other parts of the system
- We always broke a couple of features (always the same ones) whenever we added a new feature
- Developers duplicating a bunch of code because they didn’t want to change/break existing code
- Growing list of manual test cases that sometimes (ok most of the time) where not all executed resulting in publishing the application with trivial bugs
- Manual testing is painful!
- We wanted to refactor a bunch of ugly large methods, but we were afraid to break something
How did we start doing it
When we decided to start adding unit tests, the first thing we did was to compile everything under .net 3.5, since some of the tools we wanted to use depended on 3.5. Also, we decided on the tools that we would be using:
Tools and Infrastructure
We decided to go with the following set of tools
- MSTests as the test framework, since it was already in the IDE and cover our initial test requirements (IDE integration, CruiseControl integration, existing experience in our team)
- StructureMap as a DI container. Simple, effective, no cost
- Moq as our mocking framework. Simple, effective, no cost
- Include unit testing in our continuous integration process. Obviously there’s no point in having tests if your not going to use them!
- TestDriven.net and nConver to track our progress
Processes
- We would start with our business layer and extend to the presentation and custom DAL code as possible.
- Any new feature would have to include unit tests (and promote the use of TDD for those features)
- Any bug, would have to be first reproduced by a unit test and then fixed making the test pass
- Code review any new test not just to catch errors but to spread the knowledge on what and how to write tests
Armed with these tools and processes we set off into writing our first test. To figure out what king of problems we would have in breaking dependencies, we picked one of the simple business classes that manage Categories in our application and set off to write a test for the Delete method.
The original method looked like this:
1: public bool Delete(int categoryId)
2: {
3: CategoriesDataTable dt = Adapter.GetCategoryById(categoryId,
4: CompanyBLL.GetCurrentCompanyId());
5:
6: if (dt.Count != 1)
7: return false;
8:
9: CategoriesRow r = dt[0];
10: r.Delete();
11: return Adapter.Update(dt) == 1;
12: }
Simple enough. We ask the instance of the TableAdapter for the category to make sure it exists in the database, we mark it as deleted and update the DataTable into the database. The problem comes with the dependencies on the Adapter (a property that holds the instance of the TableAdapter) and the call to the static method CompanyBLL.GetCurrentCompanyId()
To break these dependencies we had to do a few things:
1. First STOP CALLING STATIC METHODS! We knew this was going to bite us every time, because it was used this way all over the application, so it was a good time to have a better solution, we created a RequestContext class that implemented an IRequestContext interface to abstract all the per request data that every class in the business layer needed and it started like this:
1: public interface IRequestContext
2: {
3: int CurrentCompanyId { get; }
4: }
Afterwards we added some other methods to abstract the parts of the HttpContext that we were using like IsUserInRole, etc. but this was enough for our first case.
2. Then we created a new constructor for the CategoriesBLL class to inject the dependencies and modified the default constructor to create the concrete classes that we were using in production:
1: public CategoriesBLL(IRequestContext context, CategoriesTableAdapter adapter)
2: {
3: m_context = context;
4: m_adapter = adapter;
5: }
6:
7: public CategoriesBLL()
8: : this(new WebRequestContext(), new CategoriesTableAdapter())
9: {
10: }
I know, poor man’s dependency injection, but we wanted to change as little as possible to be able to get our test running. Later on we would use StructureMap to resolve the dependencies and get rid of the extra constructor.
3. Now we could change the Delete method to use the injected instances instead of the static method
1: public bool Delete(int CategoryId)
2: {
3: CategoriesDataTable dt = m_adapter.GetCategoryById(CategoryId,
4: m_context.CurrentCompanyId);
5:
6: if (dt.Count != 1)
7: return false;
8:
9: CategoriesRow r = dt[0];
10: r.Delete();
11: return Adapter.Update(dt) == 1;
12: }
And at last we had something we could write a test against. Our first test!
1: [TestMethod]
2: public void DeleteCategory_WithExistingCategory_ShouldReturnTrue()
3: {
4: // create a valid datatable with one category with id = 1
5: var dt = CreateAValidCategory(1);
6:
7: // set up the mock/stub objects
8: m_adapter.Setup(a=>a.GetCategoryById(1)).Returns(dt).Verifiable();
9: m_context.SetupGet(a => a.CurrentCompanyId).Returns(1).Verifiable();
10:
11: bool res = bll.Delete(1);
12:
13: Assert.IsTrue(res);
14: m_context.VerifyAll();
15: m_adapter.Verify(a=>a.Update(It.IsAny(CategoriesDataTable)), Times.Once());
16: }
And it passed! Well nice but no cigar, this was the first stab at it and we already started to learn a lot about how we were going to have to change our code base in order to make things testable. After a few more refactors / tests we got to grips with how, in general, we would have to refactor our business layer code base to make it testable, and I can say that it was a fairly simple thing to do once we got going, and the team was really excited about the whole thing, they finally were able to refactor code with a good level of confidence.
Probably the best reference on testing legacy code is Michael Feathers’ book Working Effectively with Legacy Code also he’s article Working Effectively With Legacy Code
In the next post I’ll write on how we incorporated StructureMap into the application and injected the dependencies on the aspx and ascx classes to call the business logic and start removing the poor man’s DI.