Testing code that executes in a loop

Question

I thought I'd attempt a code kata that simulates an ATM machine on the command line. I decided to drive the design using TDD. I've run into an interesting scenario and I'm curious about what others have done in this scenario.

If you look (down at the bottom) at my AtmMachine class, you'll notice that I'm exiting my while loop on purpose so that my tests don't time out. This feels like a code smell to me and I'd like to know if others do things like this.

My current feelings are split betweeen:

1. "I'm doing it wrong" by attempting to unit test ongoing execution
2. The inability to write a unit test for it implies that "while" is the wrong construct

Here are the unit tests that I have for the atm machine so far:

[TestClass]
public class when_atm_starts
{
    private static readonly string WELCOME_MSG = "Welcome to Al Banco de Ruiz!";

    private AtmMachine _atm;
    private Mock<IAtmInput> _inputMock;
    private Mock<IAtmOutput> _outputMock;
    private Mock<ILogger> _loggerMock;
    private Mock<ICommandFactory> _cmdFactoryMock;


    [TestInitialize]
    public void BeforeEachTest()
    {
        _inputMock = new Mock<IAtmInput>();
        _outputMock = new Mock<IAtmOutput>();
        _loggerMock = new Mock<ILogger>();
        _cmdFactoryMock = new Mock<ICommandFactory>();

        _atm = new AtmMachine(_inputMock.Object, _outputMock.Object, _loggerMock.Object, _cmdFactoryMock.Object);
    }


    [TestMethod]
    public void no_one_should_be_logged_in()
    {
        this.SetupForCancelledUser();

        _atm.Start();

        Assert.IsNull(_atm.CurrentUser);
    }

    [TestMethod]
    public void should_print_welcome_to_output()
    {
        this.SetupForCancelledUser();

        _atm.Start();

        _outputMock.Verify(o => o.Write(WELCOME_MSG));
    }

    [TestMethod]
    public void should_execute_login_command()
    {
        Mock<ILoginCommand> loginCmdMock = new Mock<ILoginCommand>();

        _cmdFactoryMock.Setup(cf => cf.GetLoginCommand(_inputMock.Object, _outputMock.Object))
            .Returns(loginCmdMock.Object);

        loginCmdMock.Setup(lc => lc.LogonUser())
            .Returns(AtmUser.CancelledUser);

        _atm.Start();

        loginCmdMock.Verify(lc => lc.LogonUser());
    }


    private void SetupForCancelledUser()
    {
        Mock<ILoginCommand> loginCmdMock = new Mock<ILoginCommand>();

        _cmdFactoryMock.Setup(cf => cf.GetLoginCommand(_inputMock.Object, _outputMock.Object))
            .Returns(loginCmdMock.Object);

        loginCmdMock.Setup(lc => lc.LogonUser())
            .Returns(AtmUser.CancelledUser);
    }
}

And here is the corresponding AtmMachine class.

public class AtmMachine
{
    public static readonly string WELCOME_MSG = "Welcome to Al Banco de Ruiz!";


    private bool _shouldContinue;
    private ILogger _log;
    private ICommandFactory _cmdFactory;
    private IAtmInput _input;
    private IAtmOutput _output;


    public object CurrentUser { get; set; }


    public AtmMachine(
        IAtmInput input,
        IAtmOutput output,
        ILogger logger,
        ICommandFactory cmdFactory)
    {
        this._input = input;
        this._output = output;
        this._log = logger;
        this._cmdFactory = cmdFactory;
    }


    public void Start()
    {
        _shouldContinue = true;

        while (_shouldContinue)
        {
            _output.Clear();
            _output.Write(WELCOME_MSG);
            AtmUser user = this.GetNextUser();

            if (user == AtmUser.CancelledUser) { _shouldContinue = false; }

            _shouldContinue = false;
        }
    }


    private AtmUser GetNextUser()
    {
        ILoginCommand loginCmd = _cmdFactory.GetLoginCommand(_input, _output);
        return loginCmd.LogonUser();
    }
}

Answer 1

You're correct about testing the loop in that way. I don't have much context around the functionality your trying to test drive other than "no one logged in", so I'll take some liberties on my suggestions.

Firstly, for the loop testing, you can do it a couple of ways, depending on your preference. the first way is to extract the loop condition into a method that can be overridden in a subclass used for testing.

// In your AtmMachine
public void Start()
{
    _shouldContinue = true;

    while (stillRunning())
    {
        // Do some ATM type stuff
    }
}

protected virtual bool stillRunning() {
    return _shouldContinue;
}

Inside your test you can create a special test class that overrides stillRunning().

// Inside your test
[TestMethod]
public void no_one_should_be_logged_in()
{
    _atm = new AtmThatImmediatelyShutsDown();

    this.SetupForCancelledUser();

    _atm.Start();

    Assert.IsNull(_atm.CurrentUser);
}

class AtmThatImmediatelyShutsDown : AtmMachine {
    protected override bool stillRunning() {
        return false;
    }
}

Another option is to inject the condition as an class/interface that can be mocked. The choice is yours.

Secondly, for simplicity sake, I would extract the guts of that loop into a method with increased visibility to allow for testing of the code in the loop.

// In your AtmMachine
public void Start()
{
    _shouldContinue = true;

    while (stillRunning())
    {
        processCommands();
    }
}

public void processCommands() {
    ...
}

Now, you can directly call the processCommands() method and skip all of the looping.

Hope that helps!

Brandon

Answer 2

Seems like a place to apply SingleResponsibilityPrinciple.

1. A Daemon or Service that wait/loops for a trigger
2. SomeClass that services the AtmUser / handles a transaction.

If you split these unrelated responsibilities into 2 roles, it becomes much easier to test both of them.

public void Start()
    {
        while (_shouldContinue)
        {
            _output.Clear();
            _output.Write(WELCOME_MSG);
            if (HasUserLoggedIn)
               SomeOtherType.ProcessTransaction();
        }
}