.NET Unit Testing - Best Practices for hiding test seams in release code

Question

I'm looking for some advice for unit-testing in a manner that doesn't leave "test hooks" in place in production code.

Let's say I have a static class (VB.NET Module) named MethodLogger who has a "void WriteEntry(string Message)" method that is intended to write the calling method and a message to a log file on disk. The actual target of WriteEntry() can be diverted to a fake implementation of IMethodLogger for running unit tests, and for all other cases it should call the real implementation of IMethodLogger.

Here's a rough-cut of what I've whipped up so far, and I like the approach--but in some quick tests it raises some questions and doubts:

[InternalAttributesVisibleTo("MethodLogger.Tests")]
internal static class MethodLogger
{
 public void WriteEntry(string message)
 {
  LogWriter.WriteEntry(message);
 }

 private IMethodLogger LogWriter
 {
  get;
  set;
 }

#if DEBUG
 internal void SetLogWriter(IMethodLogger logger)
 {
  LogWriter = logger;
 }
#endif
}

Here's my specific questions:

• This tightly couples the unit tests to running against the DEBUG build; when I run unit tests though it appears that it doesn't specifically rebuild the assembly under test in DEBUG--is it possible to do so?

  • Would I ever want to run the unit tests against a non-debug build? Off the top of my head I see less value--but would there be?

• Would it be possible for me to use a custom pre-compiler flag like "TEST" in lieu of "DEBUG"? How would I go about telling Visual Studio to always rebuild the target with this flag for testing to ensure my hooks / seams are available?

Answer 1

We make sure to run tests against Release/Obfuscated builds because we have often uncovered problems this way, so, yes, I think there's value to making them work.

Obfuscation makes internals completely unusable, which is good enough for me, so we leave in testing methods (and make them internal).

On balance, I'd rather have the code be tested than worry about them being there -- as long as they don't impact performance.

Answer 2

I don't like the idea of not being able to run the tests against the real production binaries, personally.

Could you not just mark the setter as "obsolete" and disable the obsolete warning in your test code? Along with suitable documentation, that should stop production code from accidentally calling it, but still make it available to tests.

However, to address your actual questions:

• Yes, it makes sense to run unit tests against non-debug builds - it gives more confidence that changes between debug and non-debug builds don't break anything.
• Yes, you can use your own pre-processor symbol (such as TEST) if you want; you could either apply that in the existing configurations or create a new build configuration.

Answer 3

Having #IF DEBUG etc... is bad form.

Production code should be independent of test/debug code.

I'd advise looking into dependency injection to aid in unit testing.

Answer 4

It's an internal method. Just leave it as is without conditional compilation, and only call it from your unit tests.

There's nothing wrong with leaving the hook in -- in fact, you may even want to consider making the method public and allowing any code to change the logging hook.

Looking over your code again; you don't even need the SetLogWriter method; just use a private accessor for your static class and you can set it yourself in your unit tests.

I can see it being OK to leave the hook in place, removing it is just a preference, but I don't agree with making it public--it feels too much like elevating permissions just because it removes a little hassle. – Yoooder

Well, I was more commenting on general code organization principles. It's generally a good idea to create such hooks for your code; in your case you wouldn't be "elevating permissions to remove a hassle" you would be "designing a robust and flexible logging framework".

Answer 5

This seems like a prime candidate for an IoC container to me. I would make my logger not static, and set up different configurations for test, development, and production.

Answer 6

Your build process must include a production build with unit tests executed against it. If it does not, you cannot guarantee that someone hasn't slipped up with pragmas (release vs. debug for example).

What I always do in the case of logging is use Log4Net. During development and testing the log messages are set at the most verbose level, and in production I tend to leave them in error or info level, but set to more verbose for troubleshooting as necessary.

IMO the best way to perform what you are trying to accomplish with SetLogWriter is actually to use a inversion of control container, like StructureMap, to assign concretes at runtime based on various configurations. During unit test time you can use a library like Moq to manufacture concretes that behave in an expected manner (a SQL query which always returns the same exact result) though in your particular case you are performing just the opposite (desired logging during unit tests vs. no logging at production run time).

Answer 7

According to Michael Feathers "Working Effectively with Legacy Code" book it seems it would be better if you named the setter something specific for testing.

Example: SetTestingLogger(IMethodLogger logger)

I would also remove the preprocessor statements. The book also states and I quote

"You might be left a little queasy by the idea that you are removing access protection when you use static setter, but remember that the purpose of access protection is to prevent errors. We are putting in tests to prevent errors also."

Conclusion: It would seem that it would be best to place these hooks in because you are placing it for the reason of testing and ultimately to remove errors.

The "Introduce Static Setter" Section in the book explains this in more detail. I would recommend it as a book every developers should have.

Answer 8

In reality, you are on the road towards Dependency Injection - you just haven't realized it yet :)

There is nothing wrong with leaving the seams in place - you just have to model them so that instead of being test-specific, they are actually enhancements of your API. This requires a bit of practice, but first of all a change of mindset.

I wrote about this not long ago in Testability Is Really The Open/Closed Principle.

In your specific case, there are a couple of changes you should make to the MethodLogger class:

1. Make it an instance class instead of a static class. As all experienced TDD practioners will tell you, statics are evil.
2. Let the only constructor take an instance of IMethodLogger. This is called Constructor Injection and forces all clients to make an explicit decision about how to log.
3. Either set up the Dependency Graph manually, or use a DI container to wire the MethodLogger up with the desired IMethodLogger.

Answer 9

Would it be possible for me to use a custom pre-compiler flag like "TEST" in lieu of "DEBUG"? How would I go about telling Visual Studio to always rebuild the target with this flag for testing to ensure my hooks / seams are available?

Yes. What you want to do is add a conditional compilation symbol. You can do this in the project properties under the Build tab. You can create a different build configuration in the Configuration Manager and add only your TEST symbol to new configuration.

Answer 10

1. like others have said, move towards Dependency Injection and inversion-of-control (IoC)
2. isolate classes-under-test well, with the help of isolation frameworks (Moq, etc.)
3. move tests into a separate assembly - no need for #if DEBUG... #endif

Answer 11

You could use Typemock Isolator to circumvent any design issues that hurt testability. unlike other frameworks, it will also work with static methods, private methods and non virtual methods. as you learn to better design, your code will be more "testable" by default, but if you want to just get around these issues now, this will help

http://blog.typemock.com/2009/01/isolator-new-vbnet-friendly-api.html

(also, it's the only tool with a VB friendly API)