Proxy test class approach to allow covering legacy code with unit tests

Question

There is huge legacy class MyService in which several new business cases need to be implemented. Unfortunately we are not allowed to touch existing legacy code within _legacyMethod and need to build a new condition on top of it. However I'd like to have a new code covered with unit tests (using NUnit)

public class MyService
{
    public bool Execute()
    {
        return _newMethod() && _legacyMethod();
    }

    protected virtual bool _newMethod()
    {
        // some business logics

        return true;
    }

    protected virtual bool _legacyMethod()
    {
        // hundreds of lines of code

        return false;
    }
}

So primarily my goals were:

1. Leave legacy method protected
2. Cover new methods with unit tests
3. Cover integration of new methods with legacy code

To achieve this I built a proxy within test class:

using NUnit.Framework;

[TestFixture]
public class MyServiceTests
{
    private MyServiceProxy _sut;

    [SetUp]
    public void SetUp()
    {
        _sut = new MyServiceProxy();
    }

    [TearDown]
    public void TearDown()
    {
        _sut = null;
    }

    [Test]
    [TestCase(true, true, true)]
    [TestCase(false, true, false)]
    [TestCase(true, false, false)]
    [TestCase(false, false, false)]
    public void Execute_ReturnsExpected(bool newMethodStub, bool legacyMethodStub, bool expectedResult)
    {
        _sut.LegacyMethodStub = legacyMethodStub;
        _sut.NewMethodStub = newMethodStub;

        var result = _sut.Execute();

        Assert.AreEqual(expectedResult, result);
    }

    [Test]
    public void NewMethod_ReturnsTrue()
    {
        var result = _sut.NewMethodProxy();

        Assert.That(result, Is.True);
    }

    public class MyServiceProxy : MyService
    {
        public virtual bool NewMethodProxy()
        {
            return base._newMethod();
        }

        public bool? NewMethodStub { get; set; }

        public bool? LegacyMethodStub { get; set; }

        protected override bool _newMethod()
        {
            if (NewMethodStub.HasValue)
            {
                return NewMethodStub.Value;
            }
            return base._newMethod();
        }

        protected override bool _legacyMethod()
        {
            if (LegacyMethodStub.HasValue)
            {
                return LegacyMethodStub.Value;
            }

            return base._legacyMethod();
        }
    }
}

In such a way I managed to have a separate set of tests for new method itself, and another one - for integration old-new logics within Execute method. Could you advice please if this approach is applicable or can be simplified/improved somehow?

Answer 1

public class MyService
{
    public bool Execute()
    {
        return _newMethod() && _legacyMethod();
    }

    protected virtual bool _newMethod()
    {
        // some business logics

        return true;
    }

    protected virtual bool _legacyMethod()
    {
        // hundreds of lines of code

        return false;
    }
}

You abuse inheritance for testing purposes instead of splitting it up in two classes - one for each service. The current solution also violates the Open/Closed Principle. If you wanted to add another service or logic later you couldn't do it without modifying this god-service.

Virtual/abstract methods should have some purpose and be overriden in a concrete implementation and not used as workaround for testing.

You could improve it by either implementing the decorator pattern or using inheritance correctly. Here are two examples.

---

Decorator

I find the decorator pattern would be a better choice here. With a decorator you would define an interface like

interface IService
{
    bool Execute();
}

and implement it in two classes. One for the legacy service:

public class MyLegacyService : IService
{
    public bool Execute()
    {
        // hundreds of lines of code
    }
}

and one for the new service:

public class MyNewService : IService
{
    private readonly IService _service;
    public MyNewService(IService service) 
    {
        _service = service;
    }

    public bool Execute()
    {
        return ExecuteInternal() && _service.Execute();
    }

    private bool ExecuteInternal()
    {
        ..
    }
}

This has the advantage that you can provide any IService to the decorator and test the new logic independently. You can easily create a test-service to fake the legacy one.

---

Inheritance

If you want to do it with inheritance then the new service should be derived from the legacy service that is derived from an abstract base service.

The overridable method should then be called ExecuteCore. The base service just calles the core method:

public abstract class MyService
{
    public bool Execute()
    {
        return ExecuteCore();
    }

    protected abstract bool ExecuteCore();
}

The legacy-service implements the core method:

public class MyLegacyService : MyService
{
    protected override bool ExecuteCore()
    {
        // hundreds of lines of code

        return false;
    }
}

and the new-service implements it in its own way and using its base class core method:

public class MyNewService : MyLegacyService
{
    protected override bool ExecuteCore()
    {
        // some new business logics

        return newResult && base.ExecuteCore();
    }
}

This will be a little bit harder to test as the new-service depends on the legacy one so you would need to stub it in tests. Nothing impossible but more work then with the decorator.