views:

386

answers:

2

It is common to have classes with methods with string parameters that must be validated agains null or empty, such as this example:

public class MyClass {

    public void MyMethod(string param){

        if(string.IsNullOrEmpty(param)){
            throw new ArgumentNullException(...);
        }
        //...
    }
}

It's clear that the behavior of the method is the same for both (invalid) values. This is a very common situation, and when it comes to testing these methods, I always doubt about how to do it. I always create two separate tests for these cases:

[TestClass]
public class Tests {

    [TestMethod]
    public void MyMethod_should_fail_if_param_is_null(){
        //...
        myclass.MyMethod(null);
        //...
    }

    [TestMethod]
    public void MyMethod_should_fail_if_param_is_empty(){
        //...
        myclass.MyMethod("");
        //...
    }

}

But I see too much redundancy. Those tests are exactly the same, with the only difference being the parameter passed to the method. That bothers me very much, since I have to create two tests for each string parameter. A method with 3 parameters would have 6 tests only to test the parameters.

I think this is the right way of testing those parameters, but if I know that 99% of string parameters will be validated the same way, wouldn't it be better just test them for null (or empty) and assume that the behavior in the other case will be the same?

I would like to know what you think about this. I know what I'm asking is more a technical opinion than a technical question, but I think the testing community may have something interesting to say about this situation.

Thank you!

+3  A: 

Personally I'd consider using a single test for all of the parameters. That doesn't follow the normal dogma of unit testing, but it increases the readability of the tests (by minimizing the amount of test code which is dedicated to a pretty repetitive case) and doesn't have much in the way of downsides. Yes, if the test fails you don't know whether all of the checks after the first failing one will also fail - but is that really a problem in practice?

The important point is to make sure that you've got a short cut for testing the case. For instance, you might write something like this (if your unit test framework doesn't have it already):

public static void ExpectException<T>(Action action) where T : Exception
{
    try
    {
        action();
        Assert.Fail("Expected exception " + typeof(T).Name);
    }
    catch (T exception)
    {
        // Expected
    }
}

Then you can write:

[Test]
public void MyMethodFailsWithInvalidArguments()
{
    ExpectException<ArgumentNullException>(() => myClass.MyMethod(null));
    ExpectException<ArgumentException>(() => myClass.MyMethod(""));
}

Much more concise than doing each one with an individual try/catch block, or even using an ExpectedException attribute and multiple tests.

You might want overloads for cases where you also want to verify that in each case, no mocked objects have been touched (to check that side-effects are avoided) or possibly overloads for common exceptions like ArgumentNullException.

For single-parameter methods you could even write a method to encapsulate exactly what you need:

public void ExpectExceptionForNullAndEmptyStrings(Action<string> action)
{
    ExpectException<ArgumentNullException>(() => action(null));
    ExpectException<ArgumentException>(() => action(""));
}

then call it with:

[Test]
public void MyMethodFailsWithInvalidArguments()
{
    // This *might* work without the 
    ExpectExceptionForNullAndEmptyStrings(myClass.MyMethod);
}

... and maybe another one for methods with a single parameter but a non-void return type.

That's possibly going a bit far though :)

Jon Skeet
Thank you for your quick response!I already created my own version of ExpectException (you may check it out here http://gerardocontijoch.wordpress.com/2008/12/02/uso-de-expectedexceptionattribute-para-testear-excepciones/), and use it in these cases. Can't live without it :PI like the solution you propose, I thought about it, but as you said, "that doesn't follow the normal dogma of unit testing" and I only wanted to follow it, you know, get used to good practices. I'll probably go for this solution after all. Let's see what other think...
Gerardo Contijoch
Thing is, I don't believe that following dogma and throwing pragmatism out of the window *is* a good practice :) It's worth *knowing* the dogma, but then consciously ignoring it when you've weighed up the pros and cons for a particular situation and found it not to work.
Jon Skeet
I totally agree with you and that was the reason for my question. I found the cons of following a certain "good" practice, but not many pros (kind of a contradiction, a good practice with more cons than pros), so I posted a question to see if there was something I was missing.
Gerardo Contijoch
+1  A: 

If you use Java and JUnit you can use this syntax

@Test(expected = IllegalArgumentException.class)
public void ArgumentTest() {
   myClass.MyMethod("");
   myClass.MyMethod(null);
}
Janusz
That method will pass if *at least one* of those method calls throws the right exception - rather than if *both* do. It's an easy mistake to make, unfortunately - that sort of test is only useful (IMO) when it's a single statement. Otherwise you're not testing exactly where the exception is thrown.
Jon Skeet
Yes you are right... should be two tests...
Janusz