Private method to validate Excel header

Question

I am new to unit testing and I am unsure how to design classes that use third party libraries, so that I can easily test them. The example I'll use is with EPPlus-OfficeOpenXml (an Excel document manipulator). I have a class to import excel documents and validate them.

Here is a sample of my code:

Imports OfficeOpenXml
Public Class TestImporter

    Private errorList As New List(Of String)

    Public Function import(fileName As String) As List(Of String)
        Dim openFile As New FileInfo(fileName)
        Dim document As New ExcelPackage(openFile)
        Dim sheet As ExcelWorksheet = document.Workbook.Worksheets(1)

        validateAll(sheet)

        Return errorList
    End Function

    Private Sub validateAll(sheet As ExcelWorksheet)
        validateHeaders(sheet)
    End Sub

    Private headers As String() = {"Header 1", "Header 2", "Header 3", "Header 4", "Header 5"}

    Private Sub validateHeaders(sheet As ExcelWorksheet)
        For i As Integer = 1 To headers.Length
            If Not (sheet.Cells(1, i).Value = headers(i - 1)) Then
                errorList.Add(String.Format("The header '{0}' does not match '{1}'", sheet.Cells(1, i).Value, headers(i - 1)))
            End If
        Next
    End Sub
End Class

The validation subroutines are private, because the only action I want to expose for this class is the import functionality.

How should I design this class in order to test my validation of Excel inputs? My first thought is have a test file for each test. Then pass in the name of the test file into the import function in each test. This doesn't seem to be the correct solution, and seems kind of hackish.

Answer 1

It's basically like RubberDuck said.

[...] the only action I want to expose for this class is the import functionality.

This means the validation logic is an implementation detail of the TestImporter class. When you test a unit, you only care about the public interface, and treat private members (which are called by the public ones) as implementation details; not testing them directly is fine, unless you do want to test that logic separately...

How should I design this class in order to test my validation of Excel inputs?

So you want to test the validation logic? Great! Treat it as a unit then (as opposed to an implementation detail), and extract it into its own class.

You will want to decouple the validation logic from the TestImporter class though, and it's easier to do that if you code against an abstraction. Make a TestImporter constructor inject the dependency:

Private ReadOnly HeaderValidator As IHeaderValidationLogic

Public Sub New(validator As IHeaderValidationLogic)
    HeaderValidator = validator
End Sub

Now, instead of calling validateAll, you call into the IHeaderValidationLogic interface:

validationErrors.AddRange(HeaderValidator.Validate(sheet))

But this creates another problem: does the validation logic really needs to be coupled with an ExcelWorksheet object? What if you switched to the VSTO interop assemblies and worked off a Microsoft.Office.Interop.Excel.Worksheet object instead, would the logic be any different?

Best would be to abstract away that "worksheet", and make the validation logic work off what it actually validates: the contents of the worksheet.

Extract the values of range $A$1:$E$1 into an array, and pass that array (of strings?) to your header validation logic. Assuming we're returning a list of error messages, the IHeaderValidationLogic interface could look like this:

Public Interface IHeaderValidationLogic
    Function Validate(headerContents As IEnumerable(Of String)) As IReadOnlyList(Of String)
End Interface

Now, returning a list of error messages isn't very .NET-like. Why not do this instead?

Public Interface IHeaderValidationLogic
    Sub Validate(headerContents As IEnumerable(Of String))
End Interface

And then the implementation can throw some ValidationException, with whatever string content you want it to have.

And that exception would bubble up the stack to Import(fileName As String), which could now be a Sub - because again, returning errors isn't very .NET-like.

---

Note that your method names should be PascalCase, to follow the established VB.NET naming conventions.

---

If you're going to want to write a unit test for the actual Import procedure, you'll need to reduce coupling - otherwise your tests will incur I/O, which slows them down and makes them dependent on the file system, which isn't ideal.

Extract another interface.

Public Interface IExcelPackageProvider
    Function Open(fileName As String) As ExcelPackage
End Interface

Now you can mock this interface and make the Open method return a stub ExcelPackage with a fake ExcelWorksheet object that you've setup for your tests, and as a side-effect of doing this, you've extracted a highly specialised and reusable component from your Import procedure, making the class more cohesive and a little less coupled.

Private ReadOnly WorkbookProvider As IExcelPackageProvider
Private ReadOnly HeaderValidator As IHeaderValidationLogic

Public Sub New(provider As IExcelPackageProvider, validator As IHeaderValidationLogic)
    WorkbookProvider = provider
    HeaderValidator = validator
End Sub

It's now the job of WorkbookProvider to deal with file I/O and return a workbook object for the Import method to work with.