views:

237

answers:

6

For example, I'm writing tests against a CsvReader. It's a simple class that enumerates and splits rows of text. Its only raison d'être is ignoring commas within quotes. It's less than a page.

By "black box" testing the class, I've checked things like

  • What if the file doesn't exist?
  • What if I don't have permission on the file?
  • What if the file has non-Windows line-breaks?

But in fact, all of these things are the StreamReader's business. My class works without doing anything about these cases. So in essence, my tests are catching errors thrown by StreamReader, and testing behavior handled by the framework. It feels like a lot of work for nothing.

I've seen the related questions

My question is, am I missing the point of "glass box" testing if I use what I know to avoid this kind of work?

A: 

You should always be managing errors that your framework throws; that way your application is robust & doesn't crash on catastrophic errors...

Paul Nathan
But, to be clear, my class is not catching any errors thrown by the framework. I consider it the caller's job to do so, since it's more likely to know what can be done about it. So all my tests do is verify that the errors are thrown.
harpo
A: 

Yes, but that would strictly be for the purposes of unit testing:

You could abstract your CSV reader implementation from any particular StreamReader by defining an abstract stream reader interface and testing your own implementation with a mock stream reader that implements that interface. Your mock reader would obviously be immune to errors like non-existent files, permission problems, OS differences etc. You would therefore entirely be testing your own code and can achieve 100% code coverage.

Ates Goral
Hmmm... that sounds way complicated. As it is, my problem is too much coverage. If I understand you correctly, I could achieve the same thing in effect by limiting the test data only to cases that I know to be covering my actual code.
harpo
+2  A: 

I don't think you should waste time testing things that are not your code. It's a design choice, not a testing choice, whether to handle the errors of the underlying framework or let them propagate up to the caller. FWIW, I think you're right to let them propagate up. Once you've made the design decision, though, your unit testing should cover your code (and cover it well) without testing the underlying framework. Using dependency injection and a mock Stream is probably a good idea, too.

[EDIT] Example of dependency injection (see link above for more info)

Not using dependency injection we have:

public class CvsReader {
   private string filename;
   public CvsReader(string filename)
   {
      this.filename = filename;
   }

   public string Read()
   {
      StreamReader reader = new StreamReader( this.filename );
      string contents = reader.ReadToEnd();
      .... do some stuff with contents...
      return contents;
   }
}

With dependency injection (constructor injection) we do:

public class CvsReader {
   private IStream stream;
   public CvsReader( IStream stream )
   {
      this.stream = stream;
   }

   public string Read()
   {
       StreamReader reader = new StreamReader( this.stream );
       string contents = reader.ReadToEnd();
       ...  do some stuff with contents ...
       return contents;
   }
}

This allows the CvsReader to be more easily testable. We pass an instance implementing the interface we depend on in the constructor, in this case IStream. Because of this we can create another class (perhaps a mock class) that implements IStream, but doesn't necessarily do file I/O. We can use this class to feed to our reader whatever data we want without involving any of the underlying framework. In this case, I'd use a MemoryStream since we just reading from it. In we wanted to, though, we could use a mock class and give it a richer interface that lets our tests configure the responses that it gives. This way we can test the code that we write and not involve the underlying framework code at all. Alternatively we could also pass a TextReader, but the normal dependency injection pattern uses interfaces and I wanted to show the pattern with interfaces. Arguably passing in a TextReader would be better since the code above still is dependent on the StreamReader implementation.

tvanfosson
Yes, as long as I know certain cases are much more unlikely than others, the time could be better spent elsewhere. As for "dependency injection," I'm still new to the concept.
harpo
+2  A: 

This really depends on the interface of your CsvReader, you need to consider what the user of the class is expecting.

For example, if one of the parameters is a file name and the file does not exist what should happen? This should not be dependent upon whether you use a stream reader or not. The unit tests should test for the observable external behaviour of your class and in some cases, to dig slightly deeper and additionally ensure certain implementation details are covered, e.g. the file is closed when the reader has finished.

However you don't want the Unit Tests to be dependent upon all of the details or to assume that because of an implementation detail something will happen.

All of the examples you mention in your question involve observable behaviour (in this case exceptional circumstances) of your class and therefore should have unit tests related to them.

marcj
I agree with tvan that it's necessary to draw the line, but ultimately, you're right, the line has to be drawn from the client's point of view. For example, I didn't list the "close after using" test because there was no question that would be expected.
harpo
+1  A: 

I tend to agree with tvanfosson: If you inherit from a StreamReader and extend it in some way, your unit tests should only exercise the functionality you've added or altered. Otherwise, you're going to waste a lot of time and cognitive energy writing, reading and maintaining tests that don't add any value.

Although markj is correct that tests should cover the "observable external behaviour" of a class, I think it's appropriate to consider where that behaviour comes from. If it's behaviour via inheritance from another (presumably unit tested) class, then I see no benefit in adding your own unit tests. OTOH, if it's behaviour via composition then it might justify some tests to ensure the pass-throughs are working properly.

My preference would be to unit test the specific functionality you alter, and then write integration tests that check for error conditions, but in the context of the business need you're ultimately supporting.

Seth Petry-Johnson
A: 

Just an FYI, if this is .NET, you should consider not reinventing the wheel.

For C#

Add a reference to Microsoft.VisualBasic Use the fantastic class Microsoft.VisualBasic.FileIO.TextFieldParser() to handle your CSV parsing needs.

Microsoft already tested it, so you won't have to.

Enjoy.

AUSTX_RJL