Is validation as isInUnitTest () antipattern?

Question

I am working on a personal project (implying clean source code, without obsolete dependencies) and trying to follow the best methods of unit testing, dependency management, etc.

My code database contains this code:

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 is to actively prevent dangerous code / code with external dependencies on execution during testing. I like the idea of ​​a failed attempt if unit tests do something bad, but I don't like this implementation for several reasons:

• It leaves hidden dependencies on the Config static class scattered throughout our code base.
• It changes the control flow between testing and live behavior, that is, we do not necessarily test the same code.
• It adds an external dependency to a configuration file or some other stateful utility.
• It looks ugly :)

A fair number of places where we use this in my code base could be avoided if I understood what I was trying to do, but there are still some places where I still try to avoid implementing a isInUnitTests() .

Using the credit card example above, I can avoid checking isInUnitTests() on every charge by properly wrapping it in a cool CardCharger class or something similar, but while cleaner I feel like I only moved the problem one level - How can I prevent the unit test from creating a real instance of CardCharger without putting the check in the constructor / factory method that creates it?

• Is isInUnitTests() a code smell?
• If so, how can I still apply those unit tests that do not reach external dependencies?
• If not, what is the best way to implement such a method and what are some good practices to use / prevent?

---

To clarify, I'm trying to prevent unit tests from accessing inappropriate resources such as a database or network. I am all for test models, such as dependency injections, but good patterns are useless if they can be broken by a carefree developer (i.e. me). It seems important to me that I be unsuccessful in cases where unit tests do what they do. I'm not sure if this is the best way to do this.

Answer 1

Is isInUnitTests () a code smell?

Yes, in the long run, you have many ways to avoid combining code with unit tests. There is no good reason to have something like this.

If so, how can I still apply those unit tests that do not reach external dependencies?

Your tests should only check the unit of code and create a layout or stub for external dependencies.

Your code seems to be Java in which there are a lot of mature frames. Learn a little about existing ones and choose the one you like best.

EDIT

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

It is supposed to use the dependency injector to resolve dependencies between instances, so you will not need to use if for testing if you are testing or not, because you enter full functional dependencies on your "real" code, and you enter a layout or stubs on your test.

Take a more serious example, for example, a credit card charger (an example of classical testing) - it will be a very poor design, if even the test code could access the real charger without causing a flood of exceptions, I do not think that this is enough in in such a case, to trust the developer will always do the right thing

Again, you must introduce external dependencies as a credit card charger, so when you test, you enter a fake one. If some developer does not know this, I think that the first thing your company needs is training for this developer and several paired programs to guide the process.

In any case, I will get your thought and let you tell you about a similar situation that I experienced.

After some processing, several letters were sent. These letters were not sent to any environment other than the living, otherwise it would be a big problem.

Before I started working on this application, this happened several times when the developers “forgot” this rule and emails were sent to test environments, which caused a lot of problems. This is not a good strategy that depends on human memory to avoid such a problem.

What we did to avoid repeating this event by adding a configuration setting to indicate whether to send real emails or not. The problem was wider than performing any action or not on unit tests, as you can see.

Nothing will replace the link, although the developer may have set the wrong value for this parameter in his development environment. You are never 100% safe.

Shortly speaking:

• The first line of defense is communication and training.
• The second line of defense should be dependent injection.
• If you feel that this is not enough, you can add a third line of defense: configuration settings to avoid executing real logic in a test / development environment. There is nothing wrong with that. But please do not call it IsInUnitTest , because the problem is wider than this (you also want to avoid this logic, which must be executed on the developer's machine)

Answer 2

The UnitularJS Device Testing Documentation really does provide a really fantastic job of describing the different ways that developers handle unit testing.

Of the four different methods that they set out, the one that they recommend is one that involves using dependency injection, so that the flow of your production code matches the flow of testing. The only differences are that the objects you pass into your code will be different. Note that Claudio used the term “stub” to refer to the objects you pass into the method used as placeholders for those that you used in the production process:

 public Document getPage(String url, AbstractJsoup jsoup) { return jsoup.connect(url).get(); } 

My Java is a little rusty, so think that you might actually need to do some funky things to get the job done, such as checking the type of the jsoup object or applying it to your test Jsoup object or to your Jsoup object to create, which might just defeat the whole purpose of using dependency injection. Thus, if you are talking about a dynamic language like JavaScript or some other freely typed functional programming language, dependency injection will keep things really clean.

But in Java, if you do not manage dependencies and cannot use a design template, for example, a strategy template , passing in different specific implementations, doing what you do can be simpler, especially since you do not control Jsoup code. However, you can check to see if they have any stubs available that you can use as placeholders, as some library developers write records.

If you are not the owner of the code, another option would be to use the Factory class to get the desired objects, depending on the flag you set when you first installed it. This seems like a less desirable solution, as you still set the global flag on this Factory object, which may affect what you might not be trying to check. The advantage of implementing dependencies for unit testing is that you must explicitly pass the test stub during testing and explicitly pass in the production object when you want the method to execute what it wrote. As soon as the method completes execution, the test is completed, and any production process that calls it will automatically start it in production mode, because it will enter production objects.

Answer 3

Update

It was an idea that I published shortly after the publication of this question, but now I am convinced that this is not a very good plan. Leaving it here for posterity, but see my new answer for what I ended up doing.

---

I'm far from certain that this is correct, but I thought that at least it addresses my first and third objections, comes from Determines whether the code works as part of a unit test :

You can avoid saving the external state of whether you are in the unit test or not by directly examining the executable stack, for example:

 /** * Determines at runtime and caches per-thread if we're in unit tests. */ public static class IsInUnitTest extends ThreadLocal<Boolean> { private static final ImmutableSet<String> TEST_PACKAGES = ImmutableSet.of("org.testng"); @Override protected Boolean initialValue() { for(StackTraceElement ste : Thread.currentThread().getStackTrace()) { for(String pkg : TEST_PACKAGES) { if(ste.getClassName().startsWith(pkg)) { return true; } } } return false; } } 

The main advantage here is that we do not maintain state; we just check the stack trace - if the trace contains the test-framework package, we are in unit test, otherwise we will not. This is not ideal - in particular, it can potentially lead to false positives if you use the same testing framework for integration or other more permissive tests, but avoiding the external state, at least it seems to be a small gain.

Curious what others think of this idea.

Answer 4

I have never seen a system in which steps have been taken to actively prevent access to external resources of code running as part of unit tests. The problem just did not appear. You have my deepest sympathy for working somewhere where it is.

Is there a way to control the class path used for unit tests so that libraries do not need access to external resources that are not available? If there are no JSoup and JDBC drivers in the classpath, the tests for the code that tries to use them will fail. You cannot exclude JDK-level classes such as Socket and URLConnection in this way, but it can be useful.

If you were running tests using Gradle, that would be pretty simple. If you are using Maven, perhaps not. I don't think there is any way to have different classes for compiling and testing in Eclipse.

Answer 5

Well, you can achieve the same clean way using the Abstract Factory Pattern though (perhaps not suitable for invoking the Abstract Factory Pattern ).

Example in C #:

 public class CardChargerWrapper{ public CardChargerWrapper( NullCardCharger nullCharger , TestUnitConfig config){ // assign local value this.charger = new CardCharger(); } CardCharger charger; NullCardCharger nullCharger; TestUnitConfig config; public Response Charge(CreditCard card, Money amount){ if(!config.IsUnitTest()) { return charger.connect().charge(card, amount); } else { return NullCardCharger.charge(card, amount); } } } 

EDIT: Changed CardChargerWrapper to use a hard-coded instance of CardCharger instead of typing it.

Note. You can change NullCardCharger to something like MockCardCharger or OfflineCardCharger for logging purposes.

Note again: you can modify the CardChargerWrapper constructor to match. For example, instead of the constructor introducing NullCardCharger , you can make this property entered. Same thing with TestUnitConfig .

EDIT: Regarding calling IsUnitTest() good idea or not:

It really depends on your business perspective and how you conduct the testing. Like many people, code that has not yet been tested is not credible for their correctness. It cannot be redirected. On the side note, I prefer IsChargeRealCard() be more appropriate than IsUnitTest() .

Let's say that in our context we take out the unit test , at least you still need to conduct integration testing in a test environment. You probably want to check something like:

• I want to check the validation of a credit card (is it real or not, etc.).
• I want to check the payment method and see if the card is charging. As a process, but not as a real credit card payment.

In paragraph 2, I believe that it is best to create a credit card charger layout for registering a transaction. This means that the charge is correct. And it will be conducted both on the test and on the dev-server.

So how can CardChargerWrapper help in this situation?

Now with CardChargerWrapper you can:

• Switch NullCardCharger to NullCardCharger chargers to improve device testing.
• All classes using CardChargerWrapper can be sure that they check IsUnitTest first before charging a real card.
• The developer should use CardChargerWrapper instead of CardCharger , preventing a development error for unit tests.
• While viewing the code, you can determine if CardCharger in another class instead of CardChargerWrapper . This should ensure that there is no code leak.
• I'm not sure, but it looks like you can hide the links of your main project to your real CardCharger . This will further protect your code.

Answer 6

If [ isInUnitTest() is antipattern], how can I still apply those unit tests that do not reach external dependencies?

Now I have a solution that I'm happy with, which ensures that external dependencies cannot be used in test environments without explicit permission. All classes that depend on external resources (HTTP requests, databases, credit card processors, etc.) accept as one of their arguments a configuration class that contains the necessary parameters for initializing these objects. In a real environment, a real Config object is passed containing the data they need. In the test environment, the layout is transmitted and without explicit configuration of the layout, the object will not be able to build / connect.

For example, I have an Http connection utility:

 public class HttpOp { public HttpOp(Config conf) { if(!conf.isHttpAllowed()) { throw new UnsupportedOperationException("Cannot execute HTTP requests in "+ getClass()+" if HTTP is disabled. Did you mean to mock this class?"); } } .... } 

In the unit test, if you tried to run the code that creates the HttpOp object, you should throw an exception, because the mocked Config will not return true unless explicitly set for this. In a functional test, where necessary, you can do this explicitly:

 @Test public void connect() { State.Http httpState = mock(State.Http.class); when(httpState.isEnabled()).thenReturn(true); RemoteDataProcessor rdp = new RemoteDataProcessor(new HttpOp(httpState)); ... 

}

Of course, it still depends on the fact that Config properly ridiculed in test environments, but now we have exactly one danger for searching, and reviewers can quickly verify that the Config object is ridiculed and trusts that only utilities that are explicitly enabled will be available . Similarly, there is now only one piece of news that new team members need to say (“always the Config layout in tests”), and now they can be sure that they will not accidentally charge a credit card or send emails to customers.