22

I want to create a unit test for a member function of a class called ScoreBoard which is storing the top five players in a game.

The problem is that the method I created a test for (SignInScoreBoard) is calling Console.ReadLine() so the user can type their name:

public void SignInScoreBoard(int steps)
{
    if (topScored.Count < 5)
    {
        Console.Write(ASK_FOR_NAME_MESSAGE);
        string name = Console.ReadLine();
        KeyValuePair<string, int> pair = new KeyValuePair<string, int>(name, steps);
        topScored.Insert(topScored.Count, pair);
    }
    else
    {
        if (steps < topScored[4].Value)
        {
            topScored.RemoveAt(4);
            Console.Write(ASK_FOR_NAME_MESSAGE);
            string name = Console.ReadLine();
            topScored.Insert(4, new KeyValuePair<string, int>(name, steps));
        }
    }
}

Is there a way to insert like ten users so I can check if the five with less moves (steps) are being stored?

Artur INTECH
  • 6,024
  • 2
  • 37
  • 34
Petur Subev
  • 19,983
  • 3
  • 52
  • 68

10 Answers10

31

You'll need to refactor the lines of code that call Console.ReadLine into a separate object, so you can stub it out with your own implementation in your tests.

As a quick example, you could just make a class like so:

public class ConsoleNameRetriever {
     public virtual string GetNextName()
     {
         return Console.ReadLine();
     }
}

Then, in your method, refactor it to take an instance of this class instead. However, at test time, you could override this with a test implementation:

public class TestNameRetriever : ConsoleNameRetriever {
     // This should give you the idea...
     private string[] names = new string[] { "Foo", "Foo2", ... };
     private int index = 0;
     public override string GetNextName()
     {
         return names[index++];
     }
}

When you test, swap out the implementation with a test implementation.

Granted, I'd personally use a framework to make this easier, and use a clean interface instead of these implementations, but hopefully the above is enough to give you the right idea...

Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
  • 2
    I completely disagree. Refactoring just for the sake of testing is a bad idea. Check out Microsoft Moles for a much better solution. – Stephen Cleary Jul 01 '10 at 20:28
  • 3
    @Stephen Cleary There's no possible way for me to disagree with you more. Not only should you strive for testable code, but separation of concerns isn't just a testing concern. This refactoring will leave him with a more modular, reusable code base. It also enables him to write clean, simple unit tests. – Andy_Vulhop Jul 01 '10 at 20:40
  • 5
    @Stephen: I wouldn't refactor this "just for the sake of a test" - I'd refacotr this because it's got multiple responsibilities, and testability would be a simple side effect of the refactoring. This method is too complex to test effectively. – Reed Copsey Jul 01 '10 at 20:41
  • 1
    Separation of concerns is an excellent design goal, but too much just creates a huge mess. The unit tests aren't any cleaner or simpler than using Moles. The refactoring will *not* leave him with a more modular, reusable code base - simply because *code must be understood before it can be reused*, and the completely unnecessary abstraction of the console complicates the code considerably. – Stephen Cleary Jul 01 '10 at 20:53
  • 2
    @Stephen: I think we'll have to agree to disagree here. I feel that input of user data (and output, for that matter) should ALWAYS be abstracted out of methods. Having Console.ReadLine inside of a method that does anything but process user input smells really bad to me... – Reed Copsey Jul 01 '10 at 23:07
  • @Reed: I see what you're saying. I do agree that user interaction should be abstracted out, so in this situation I'd agree with you. But the suggested abstractions aren't clean at all; the only WPF implementation of `ConsoleNameRetriever` would force a modal dialog UI. I'm all for MVVM because of its UI semi-abstraction, but in this case, the only decent implementations would be `Console` or a test stub... That is why I see this as refactoring just for the sake of the test; the code really needs to be completely redesigned. – Stephen Cleary Jul 01 '10 at 23:13
20

You should refactor your code to remove the dependency on the console from this code.

For instance, you could do this:

public interface IConsole
{
    void Write(string message);
    void WriteLine(string message);
    string ReadLine();
}

and then change your code like this:

public void SignInScoreBoard(int steps, IConsole console)
{
    ... just replace all references to Console with console
}

To run it in production, pass it an instance of this class:

public class ConsoleWrapper : IConsole
{
    public void Write(string message)
    {
        Console.Write(message);
    }

    public void WriteLine(string message)
    {
        Console.WriteLine(message);
    }

    public string ReadLine()
    {
        return Console.ReadLine();
    }
}

However, at test-time, use this:

public class ConsoleWrapper : IConsole
{
    public List<String> LinesToRead = new List<String>();

    public void Write(string message)
    {
    }

    public void WriteLine(string message)
    {
    }

    public string ReadLine()
    {
        string result = LinesToRead[0];
        LinesToRead.RemoveAt(0);
        return result;
    }
}

This makes your code easier to test.

Of course, if you want to check that the correct output is written as well, you need to add code to the write methods to gather the output, so that you can assert on it in your test code.

Lasse V. Karlsen
  • 380,855
  • 102
  • 628
  • 825
  • 2
    +1, IMO this is the structure you have to follow. However, I would think more abstractly, and instead of thinking about a Console and wrap it, I would use an INameProvider interface, with a GetName() method. Then, you can have implementations such as ConsoleNameProvider, HardCodedNameProvider, WhateverNameProvider, etc. – bloparod Jul 02 '10 at 04:04
5

You should not mock something that comes from the framework, .NET already provides abstractions for its components. For the console, they are the methods Console.SetIn() and Console.SetOut().

For instance, with regards to Console.Readline(), you would do it this way:

[TestMethod]
MyTestMethod()
{
    Console.SetIn(new StringReader("fakeInput"));
    var result = MyTestedMethod();
    StringAssert.Equals("fakeInput", result);
}

Considering the tested method returns the input read by Console.Readline(). The method would consume the string we set as input for the console and not wait for interactive input.

adamency
  • 682
  • 6
  • 13
4

You can use Moles to replace Console.ReadLine with your own method without having to change your code at all (designing and implementing an abstract console, with support for dependency injection is all completely unnecessary).

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
2

Why not create a new stream (file/memory) for both stdin and stdout, then redirect input/ouput to your new streams before calling the method? You could then check the content of the streams after the method has finished.

OJ.
  • 28,944
  • 5
  • 56
  • 71
  • 1
    By the way, I don't like this method at all :) I'd much prefer to abstract the console like others have suggested. – OJ. Jul 01 '10 at 20:24
2
public void SignInScoreBoard(int steps, Func<String> nameProvider)
{
    ...
        string name = nameProvider();
    ...
}  

In your test case, you can call it as

SignInScoreBoard(val, () => "TestName");

In your normal implementation, call it as

SignInScoreBoard(val, Console.ReadLine);

If you're using C# 4.0, you can make Console.ReadLine a default value by saying

public void SignInScoreBoard(int steps, Func<String> nameProvider=null)
{
    nameProvider = nameProvider ?? Console.ReadLine;
    ...
Mark H
  • 13,797
  • 4
  • 31
  • 45
1

Rather than abstracting Console, I would rather create a component to encapsulate this logic, and test this component, and use it in the console application.

Stéphane
  • 11,755
  • 7
  • 49
  • 63
0

I can't believe how many people have answered without looking at the question properly. The problem is the method in question does more than one thing i.e. asks for a name and inserts the top-score. Any reference to console can be taken out of this method and the name should be passed in instead:

public void SignInScoreBoard(int steps, string nameOfTopScorer)

For other tests you will probably want to abstract out the reading of the console output as suggested in the other answers.

Dan Ryan
  • 624
  • 7
  • 24
  • Actually it only needs the name if they've outscored the top scorers, so always passing a name wouldn't be appropriate, which is why all the other answers suggest some variation of a _NameProvider_ to ask when appropriate. – drzaus Aug 13 '18 at 15:35
0

I had a similar issue few days ago. Encapsulation Console class seemed like overkill for me. Based on KISS principle and IoC/DI principle I putted dependencies for writer (output) and reader (input) to constructor. Let me show the example.

We can assume simple confirmation provider defined by interface IConfirmationProvider

public interface IConfirmationProvider
{
    bool Confirm(string operation);
}

and his implementation is

public class ConfirmationProvider : IConfirmationProvider
{
    private readonly TextReader input;
    private readonly TextWriter output;

    public ConfirmationProvider() : this(Console.In, Console.Out)
    {

    }

    public ConfirmationProvider(TextReader input, TextWriter output)
    {
        this.input = input;
        this.output = output;
    }

    public bool Confirm(string operation)
    {
        output.WriteLine($"Confirmed operation {operation}...");
        if (input.ReadLine().Trim().ToLower() != "y")
        {
            output.WriteLine("Aborted!");
            return false;
        }

        output.WriteLine("Confirmated!");
        return true;
    }
}

Now you can easily test your implementation when you inject dependency to your TextWriter and TextReader (in this example StreamReader as TextReader)

[Test()]
public void Confirm_Yes_Test()
{
    var cp = new ConfirmationProvider(new StringReader("y"), Console.Out);
    Assert.IsTrue(cp.Confirm("operation"));
}

[Test()]
public void Confirm_No_Test()
{
    var cp = new ConfirmationProvider(new StringReader("n"), Console.Out);
    Assert.IsFalse(cp.Confirm("operation"));
}

And use your implementation from apllication standard way with defaults (Console.In as TextReader and Console.Out as TextWriter)

IConfirmationProvider cp = new ConfirmationProvider();

That's all - one additional ctor with fields initialization.

honzakuzel1989
  • 2,400
  • 2
  • 29
  • 32
0

ConsoleTests.cs (passing test)

using NUnit.Framework;

class ConsoleTests
{
    [Test]
    public void TestsStdIn()
    {
        var playerName = "John Doe";

        var capturedStdOut = CapturedStdOut(() =>
        {
            SubstituteStdIn(playerName, () =>
            {
                RunApp();
            });
        });

        Assert.AreEqual($"Hello, {playerName}", capturedStdOut);
    }

    void RunApp(string[]? arguments = default)
    {
        var entryPoint = typeof(Program).Assembly.EntryPoint!;
        entryPoint.Invoke(null, new object[] { arguments ?? Array.Empty<string>() });
    }

    string CapturedStdOut(Action callback)
    {
        TextWriter originalStdOut = Console.Out;

        using var newStdOut = new StringWriter();
        Console.SetOut(newStdOut);

        callback.Invoke();
        var capturedOutput = newStdOut.ToString();

        Console.SetOut(originalStdOut);

        return capturedOutput;
    }

    void SubstituteStdIn(string content, Action callback)
    {
        TextReader originalStdIn = Console.In;

        using var newStdIn = new StringReader(content);
        Console.SetIn(newStdIn);

        callback.Invoke();

        Console.SetIn(originalStdIn);
    }
}

Program.cs (implementation)

Console.Write($"Hello, {Console.ReadLine()}");

P.S. Provider, retriever and wrapper class solutions suggested above do not solve the problem the author described since you anyway need to test that you indeed asking for user input or printing something to standard output.

Artur INTECH
  • 6,024
  • 2
  • 37
  • 34