Should unit tests be declared inline?

Question

I have long been a fan of Python's doctest library for the simple reason that comments can not only be useful but also usable in asserting correct behavior. I recently stumbled across the (seemingly) little known System.Diagnostics.ConditionalAttribute for .NET. This could easily be used to allow you to define tests for your class methods within the class itself. Here's a simple example:

using System.Diagnostics;
using NUnit.Framework;

namespace ClassLibrary1
{
    public class Class1
    {
        public static int AddTwoNumbers(int x, int y)
        {
            return x + y;
        }

        [Conditional("DEBUG")]
        [TestCase(1, 1, 2)]
        [TestCase(1, 2, 3)]
        [TestCase(2, 1, 3)]
        [TestCase(11, 7, 18)]
        public static void TestAddTwoNumbers(int x, int y, int sum)
        {
            int actual = AddTwoNumbers(x, y);
            Assert.AreEqual(sum, actual);
        }
    }
}

Doing this, you could create a debug assembly that will run tests and a production assembly with all of it stripped out, similar to the way FAKE can build projects. The question is, should you? Is this a good practice? Why or why not?

You'll further find that this example doesn't actually work as I'm expecting it to. I'm not sure why the attribute is allowing the test method to be compiled. Any ideas as to why?

Answer 1

ConditionalAttribute doesn't change whether or not the method itself is compiled. It changes whether calls to the method are generated or not.

For example, Debug.WriteLine has Conditional("DEBUG") applied to it - but the code is still present. The point is that client code that contains calls to Debug.WriteLine will ignore those calls when built without the DEBUG preprocessor symbol defined.

To conditionally compile a whole method, you'd use:

#if DEBUG
...
#endif

Even leaving this aside, I wouldn't do it myself. I like to keep the production code separate from the test code. I find it clearer that way - although it does mean I can't test private methods. (There are those who say you should never test implementation details anyway, but that's an entirely different issue.)

There's also the matter of testing against the real code. If you're going to build one version of the code with the tests built in and one without, and you use the non-test assembly in production, that means you're running code you haven't tested. Sure, it may well work the same way as with DEBUG defined... but what if it doesn't? I like being able to run my unit tests against the exact same binary I then use in production.

Answer 2

(IMO) Absolutely not.

While using the Debug attribute will keep the method from being exposed in your release projects, the fact of the matter is that while developing (which will be in DEBUG mode, more than likely), this will pollute the class quite a bit, depending on how many test cases you develop (and you can develop many, even for a class with a small footprint).

Additionally, I'd argue that it's poor encapsulation. You are looking to write tests for the class, those tests actually don't help to serve or enhance the actual function of the class, and therefore, should not be a part of it.

Finally, one of the great advantages of separate test harnesses is that you can set up complex interdependencies between multiple classes, allowing you to test the interaction between those classes.

In your case, the scope of the vision is one single class. If you need to pull in other classes (mock implementations, etc) then were would you put them?