views:

253

answers:

3

I am writing a wrapper for the ConfigParser in Python to provide an easy interface for storing and retrieving application settings.

The wrapper has two methods, read and write, and a set of properties for the different application settings.

The write method is just a wrapper for the ConfigParser's write method with the addition of also creating the file object needed by the ConfigParser. It looks like this:

def write(self):
    f = open(self.path, "w")
    try:
        self.config_parser.write(f)
    finally:
        f.close()

I would like to write a unit test that asserts that this method raises an IOError if the file could not be written to and in the other case that the write method of the config parser was called.

The second test is quite easy to handle with a mock object. But the open call makes things a little tricky. Eventually I have to create a file object to pass to the config parser. The fact that a file will actually be created when running this code doesn't make it very useful for a unit test. Is there some strategy for mocking file creation? Can this piece of code be tested in some way? Or is it just too simple to be tested?

+3  A: 

Actually, only open could throw an exception in your code. The docs for write() doesn't say anything about exceptions. Possibly only a ValueError or something for a bad file pointer (as a result of open failing, which can't be the case here).

Making an IOError for open is easy. Just create the file elsewhere and open it for writing there. Or you could change the permissions for it so you don't have access.

You'd might wanna use the with statement here though, and it'll handle the closing itself.

In python 2.5 you need the first line. In later versions you don't need it.

from __future__ import with_statement # python 2.5 only

def write(self):
    with open(self.path, 'w') as f:
        self.config_parser.write(f)

The write method is guaranteed to be called if open succeeds, and won't be called if open raises an IOError. I don't know why you'd need a test to see if write was called. The code says that it does. Don't overdo your testing. ;)

Tor Valamo
1) I don't want to involve the file system at all for the reasons that Travis Bradshaw mentioned in his answer. 2) Does the with statement work in Python 2.4? 3) Maybe you are right in that this code does so little so it is pointless in testing.
Rickard Lindberg
It's not that it does so little. It's that it does nothing that can possibly fail. In a worst case it returns an IOError, for reasons that aren't the worry of this function (self.path should know what it holds). ;) You should instead test config_parser.write() separately, and make sure it works.
Tor Valamo
+2  A: 
Roger Pate
+4  A: 

First, you don't actually need to unit test open(), since it's pretty reasonable to assume that the standard library is correct.

Next, you don't want to do file system manipulations to get open() to generate the error you want, because then you're not unit testing, you're doing a functional/integration test by including the file system.

So you could perhaps replace open() in the global namespace with a surrogate that just raises an IOError. Though, probably need to make sure you put things back if execution continues.

But in the end, what value does the test have? There's so little in that code snippet that's your own system. Even replacing open() really just ends up being a test that says "does the try and finally statement in Python work?"

My suggestion? Just add a statement to the docstring that records your expectation. "Raises an IOError if the file can't be written." Then move on. You can add a unit test later if this method gains some complexity (and merit for testing).

Travis Bradshaw
Actually it shouldn't be in the docstring, because it doesn't say anything about the function. *From PEP257: "It prescribes the function or method's effect as a command ("Do this", "Return that"), not as a description; e.g. don't write "Returns the pathname ..."."* In a best case, it should be "Raise IOError if..." but that's wrong, because it's not your function that does that.
Tor Valamo
That's not wrong. Allowing an error to propagate without catching it is a "reraise". This method raises an `IOError` if `open()` raises an `IOError`. And while I agree with PEP257, there's more to docstrings than *just* the introductory phrase. It's definitely a normal convention to put any sort of documentation you want into a docstring in combination with the introductory statement. With that in mind, you can put it there as a comment if you consider it only internal, or properly put it in the docstring if you consider it a contract with the user of the method. (a *documented* behavior)
Travis Bradshaw
Perhaps, as you say, the function contains so little of "my own system" that it is not useful to test. Thanks for your input.
Rickard Lindberg
No problem. Good unit testing is a challenging thing! I feel like I'm still just learning as well. Best of luck. (Also, don't forget to mark an answer as your selected answer if you've become satisfied from the resulting discussion.)
Travis Bradshaw