Is a check like isInUnitTest() an antipattern?

Question

I'm working on a personal project (meaning clean source code, no legacy dependencies) and attempting to follow best practices regarding unit testing, dependency management, etc.

My company's codebase is littered with code like this:

public Response chargeCard(CreditCard card, Money amount) {
  if(Config.isInUnitTests()) {
      throw new IllegalStateException("Cannot make credit card API calls in unit tests!");
  }
  return CreditCardPOS.connect().charge(card, amount);
}

The goal here is to actively prevent dangerous code / code with external dependencies from being executed during testing. I like the notion of failing fast if unit tests do something bad, but I don't like this implementation for a few reasons:

• It leaves hidden dependencies to the static Config class scattered throughout our codebase.
• It changes the control flow between testing and live behavior, meaning we're not necessarily testing the same code.
• It adds an external dependency to a configuration file, or some other state-holding utility.
• It looks ugly :)

A fair number of the places where we use this in my company's codebase could be avoided by better dependency awareness, which I'm attempting to do, but there are still some places where I'm still struggling to see away around implementing an isInUnitTests() method.

Using the credit card example above, I can avoid the isInUnitTests() check on every charge by properly wrapping this up inside a mock-able CardCharger class, or something similar, but while cleaner I feel like I've only moved the problem up one level - how do I prevent a unit test from constructing a real instance of CardCharger without putting a check in the constructor / factory method that creates it?

• Is isInUnitTests() a code smell?
• If so, how can I still enforce that unit tests don't reach external dependencies?
• If not, what's the best way to implement such a method, and what are good practices for when to use / avoid it?

---

To clarify, I'm trying to prevent unit tests from accessing unacceptable resources, like the database or network. I'm all for test-friendly patterns like dependency injection, but good patterns are useless if they can be violated by a careless developer (i.e. me) - it seems important to me to fail-fast in cases where unit tests do things they aren't supposed to, but I'm not sure the best way to do that.

Answer 1

Is isInUnitTests() a code smell?

Yes, definitively, you have many ways to avoid coupling your code to unit tests. There is no valid reason to have something like that.

If so, how can I still enforce that unit tests don't reach external dependencies?

Your tests must only validate a unit of code and create mock or stubs for external dependencies.

Your code seems to be Java, which it's plenty of mature mock frameworks. Research a little about existing ones and pick the one that likes you more.

EDIT

how do I prevent a unit test from constructing a real instance of HTTPRequest without putting a check in the constructor / factory method that creates it

You're supposed to use a dependency injector to resolve your instance dependencies so you will never need to use a if to test if you're on a test or not since on your "real" code you inject full functional dependencies and on your test you inject mock or stubs.

Take a more serious example, like a credit card charger (a classic unit testing example) - it would seem like a very poor design if it were even possible for test code to access the real card charger without triggering a deluge of exceptions. I don't think it's enough in a case like that to trust the developer will always do the right thing

Again, you're supposed to inject external dependencies as a credit card charger so on your test you would inject a fake one. If some developer don't know this, I think the first thing your company needs is training for that developer and some pair programming to guide the process.

In any case, kind of I get your point and let me tell you a similar situation I experienced.

There was an application that sent a bunch of emails after some processing. These emails were not to be send on any other environment except live, otherwise it would be a big problem.

Before I started working on this application, it happened several times that developers "forgot" about this rule and emails were sent on test environments, causing a lot of problems. It's not a good strategy to depends on human memory to avoid this kind of problem.

What we did to avoid this to happen again was adding a config setting to indicate if send real emails or not. The problem was broader than executing things or not on unit tests as you can see.

Nothing replace communication though, a developer could set the incorrect value for this setting on his development environment. You're never 100% safe.

So in short:

• First line of defense is communication and training.
• You second line of defense should be dependency injection.
• If you feel that this is not enough, you could add a third line of defense: a configuration setting to avoid executing real logic on test/development environments. Nothing wrong about that. But please don't name it IsInUnitTest as the problem is broader than that (you want also o avoid this logic to be executed on developer machine)