Adding a virtual keyword when testing legacy code

Question

I am adding tests to some gnarly legacy code to have enough confidence to seriously reorganize it. One of the problems is that the one who wrote the code obviously did not try to make the code testable (given that he never wrote a single unit test!)

A common problem is that there are currently no interfaces, but only an 11-level inheritance chain. I use Rhino Mocks to isolate the tested class from its dependencies, but since I am mocking the class, not the interface, I can only drown out the read-only property if it has the virtual .

My current thinking is that I just add the virtual to the property. It is not planned to add any additional objects to the existing dependency chain, and this will allow you to write tests.

Are there any arguments against adding the virtual , or is this an acceptable compromise to get the tests?

Sample Code ...

In the test class:

 var someClassStub = MockRepository.GenerateStub<SomeClass>(); someClassStub.Stub(s => s.SomeProperty).Return("Test"); 

In SomeClass:

 public virtual string SomeProperty { get { return someDependency.SomeMethod(); } } 

Answer 1

The main argument against adding virtual is that it distorts your intentions. The virtual keyword signals for the derived classes that you expect from this property can be overridden.

I would not use virtual , but make fun of the dependency as shown below:

 var mockedDependency = MockRepository.GenerateMock<IDependency>(); mockedDependency.Expect(x => x.SomeMethod()) .Returns("whatever your test dictates"); var target = new SomeClass(mockedDependency); mockedDependency.VerifyAllExpectations(); 

Then add this to the newly created overloaded constructor, for example:

 public SomeClass(IDependency dependency) : base() { this.someDependency = dependency; } 

Answer 2

Instead of adding virtual there are several safer ways to make your code initially verifiable. Personally, I highly recommend using the Extract Interface tool provided with Visual Studio and replacing specific class references with an interface where this can be done safely. Then, mock the interface instead of a specific class.

If you are using a version of Visual Studio (or some other IDE) that does not support Extract Interface, all you need to do is to track all the public members of the class and add them to the interface and make your specific class implement it.

The first priority should be that original test suite. That way, you can make more dangerous changes later with reasonable assurance that your code hasn't broken.

For those who are working on making the old obsolete code module testable, I highly recommend reading the book "Effectively work with obsolete code . " It costs money. So much so that my manager ended up buying me an office to consult.