Should unit tests be declared inline?

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?

---

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.

---

(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?

---

There are going to be pros and cons to this approach.

On the one hand, you'll be able to test internals. Some will argue that that's a good thing. Others will argue that you should only test the public interface. Personally, I think there's a occasionally a need to test the internals (if for no other reason than isolating a specific behavior), but that's something you can likely achieve with InternalsVisibleTo.

On the other hand, you'll be able to test internals. Your test code will behave as though it belonged in the assembly. My personal opinion is that it isn't part of the application, and shouldn't be there. Consider it a form of "separation of concerns." Tests test, and applications do the things that the tests test. But the test should know as little about the implementation details as possible. If the tests are on the inside, it's far too easy for them to know all the gritty details. If the implementation details later change, the tests are invalidated and have to be rewritten.

Personally, I prefer an external assembly. Not only does it enforce the notion that my tests ONLY test software, but it makes me ask myself how to write the software so it can be tested by an external source. That leads to better software design overall. I haven't regretted it yet.

---

I agree with most other answers: it's better to keep your unit tests separate. Especially note Jon's point about how ConditionalAttribute works: in particular, the code is still there, it's just not called.

However, I think you would like Code Contracts. They actually use a rewriter that modifies your code after you've compiled it. This allows you to easily set up a Debug build that includes all kinds of runtime checks along with a Release build that doesn't have any of that code. You can also install a VS extension that displays pre- and post-conditions as popups in the code editor.

I have a brief getting started post on my blog, where I walk through setting up CC for a library. My style is to have a Debug build (with full checks) and a Release build (without checks, but including a separate dll containing the preconditions).

Note that Code Contracts is not a replacement for unit testing, but it is a nice complement to it.