1

I have a file parser and my manager told me that I need to create unit tests for it. Here is my code:

public class ParsedDetails
{
    public int Id { get; set; }
    public Guid Guid { get; set; }
    public bool IsValid { get; set; }
}

public class CsvParser : IParser
{
    private const int CsvColumns = 2;
    private const char CsvSeparator = ';';
    private const int IdColumnIndex = 0;
    private const int GuidColumnIndex = 1;


    private ParsedDetails ParseLine(string line)
    {
        ParsedCardDetails result = new ParsedCardDetails();
        //validating and parsing data

        return result;
    }

    public IEnumerable<ParsedCardDetails> Parse(string path)
    {
        if (string.IsNullOrWhiteSpace(path))
        {
            throw new ArgumentException("The path must not be empty");
        }

        if (!File.Exists(path))
        {
            throw new FileNotFoundException();
        }

        List<ParsedCardDetails> result = new List<ParsedCardDetails>();

        var data = File.ReadAllLines(path);

        foreach (var item in data)
        {
            var validationResult = ParseLine(item);
            result.Add(validationResult);
        }

        return result;
    }
}

So, the only thing that crossed my mind was to write unit tests to the ParseLine method, but this is private and I have no other reason to make it public and there is no need for me to mock the parser class. Do you have any idea on how to proceed?

This is my first time writing unit tests.

gnat
  • 21,442
  • 29
  • 112
  • 288
Buda Gavril
  • 129
  • 1
  • 5

2 Answers2

6

In order to test code, you have to write testable code. As it stands, your class isn't easy to test, so it needs a rethink.

The key thing to address in this case is that you are breaking the single responsibility principle, as your class is responsible for validating a file path, loading the contents of that file and then parsing it into a new form. So break the thing into two:

public interface IParseableDataProvider
{
    IEnumerable<string> GetDataToParse();
}

internal class FileContentsDataProvider : IParseableDataProvider
{
    private readonly string _filePath;
    internal FileContentsDataProvider(string filePath)
    {
        _filePath = filePath;
    }

    public IEnumerable<string> GetDataToParse()
    {
        if (string.IsNullOrWhiteSpace(path))
        {
            throw new ArgumentException("The path must not be empty");
        }

        if (!File.Exists(path))
        {
            throw new FileNotFoundException();
        }

        return File.ReadAllLines(path);
    }
}


public class CsvParser : IParser
{
    private const int CsvColumns = 2;
    private const char CsvSeparator = ';';
    private const int IdColumnIndex = 0;
    private const int GuidColumnIndex = 1;


    private ParsedDetails ParseLine(string line)
    {
        ParsedCardDetails result = new ParsedCardDetails();
        //validating and parsing data

        return result;
    }

    public IEnumerable<ParsedDetails> ParseData(IParseableDataProvider dataProvider) =>
        dataProvider.Select(line => ParseLine(line));
}

Now you can unit test CsvParser using a IParseableDataProvider implementation that doesn't touch the file system and you can perform an integration test by using FileContentsDataProvider with CsvParser.

David Arno
  • 38,972
  • 9
  • 88
  • 121
5

The Parse method is doing 2 things, which is generally considered to be not good

  1. Loading the file from disk
  2. Parsing the loaded file.

The CsvParser class, and any other parsers that come after it, should be responsible for 1 thing: parsing text. Parsers should not care about where that text came from. It might have come from a file, or database, or web service, or fairy, or wherever. Parsers do not care where the source text comes from.

So to make testing easier, eliminate the file loading from the Parse function, so the method becomes

    public IEnumerable<ParsedCardDetails> Parse(String[] data)
    {
        List<ParsedCardDetails> result = new List<ParsedCardDetails>();

        foreach (var item in data)
        {
            var validationResult = ParseLine(item);
            result.Add(validationResult);
        }

        return result;
    }

Now all of a sudden, it becomes child's play to write tests for the parser. For example, it's really easy to test the behavior when an empty string array gets passed.

[TestMethod]
public void ParsingEmptyDataReturnsAnEmptyList()
{
   CsvParser parser = new CsvParser();
   var result = parser.Parse(new String[]);
   Assert.AreEqual(0, result.Count);
}

I'm not sure if that will actually compile, but the point should be fairly clear.

Separate the file loading from the actual parsing, and the tests practically write themselves.

If the ParseLine does not need to be public, then don't make it public. You can validate the parsing logic by passing known input into the Parse method, and then examine the list returned to validate the parsing logic worked correctly.

CHendrix
  • 514
  • 4
  • 10