If you are a programmer (or something worse than an architect), can you answer this simple question: how to write NOT test code? Are you thinking? If you can hardly name at least 3 ways to achieve non-testable code, then the article is for you.
Many will say: why do I need to know how to write non-testable code, do you want to teach me bad? The answer is: if you know the typical patterns of non-testable code, then, if they exist, you can easily see them in your project. And, as you know, the recognition of the problem is half the way to treatment. The article also provides an answer to how this treatment is actually carried out. I ask under the cat.
The article will not focus on a specific programming language, the following is relevant for all procedural languages. But, as it seems to me, the article will be especially useful for those who program in dynamic interpreted languages ​​and have not had serious experience in developing typed compiled languages. It is in the code of this category of developers that I most often noticed the patterns described below. Examples will be in pseudocode, similar to Java, C #, and TypeScript.
')
At once I will make a reservation that the article will not consider the question of how to write tests. On this subject, and so there are many articles. The question of how to write code so that it can be simply and beautifully tested, and the resulting tests are simple and supported, will be considered. Further, the notion of a test means a beautiful and clean unit test, which is written without various hacks, without additional “magic” libraries for replacing dependencies on the fly, and other fun that makes reading and support of tests difficult. That is the test in the finest sense of the word.
A bit of philosophy. Is it worth writing tests at all? My opinion: if you create a project that will develop, then you just need tests. In addition to the fact that the tests perform their direct function (they allow to check the compliance of the code with the requirements), they as a side effect “straighten out” the design of the classes. This is because tests cannot be written on a class with a “crooked” design, therefore, in order to achieve testability, one has to refactor a class. Or, if tests are written before the code, the class design is immediately born the right one. The test code is reusable, since we can reuse it in tests. And reusability is an important criterion for proper class design. The same tests, perhaps, but not necessarily, will improve the architecture of the application as a whole. As someone well noticed: the class design is only as good as this class can be tested using unit tests.
The list of the main killers of the code under test:
Mining knowledge
Knowledge extraction occurs when a method requires one set of arguments, but does not use them directly, but begins to pick these arguments in search of other objects. Typical scenarios:
- the method walks on the object through more than one point (.)
- the method does not directly use its argument (uses it to get other information)
A method or constructor must require what it really needs to work. He doesn’t have to worry about how to get / calculate it all, doing any work. It is necessary to require the minimum necessary set of arguments for their work.
Life example: when in a store you are asked to pay for a purchase, what do you do: give a wallet so that the cashier can take the money from there, or do you give money? I think the analogy is clear - the cashier class should require the money, rather than the purse class, to enter the calculation method.
Consider a couple of examples.
before: untested codeclass DiscountCard { DiscountCard(UserContext userContext) { this.user = userContext.getUser(); this.level = userContext.getLevel(); this.order = userContext.getOrder(); }
DiscountCard does not directly use userContext. The one who will write the test will need to examine the device of the DiscountCard class in order to understand what objects are really required there, because the userContext can contain dozens of objects for initialization. And this time and the risk of doing something wrong, and as a result, the test may be wrong. Let's make it so that DiscountCard needs what it really needs:
after: test code class DiscountCard { DiscountCard(User user, PlanLevel level, Order order) { this.user = user; this.level = level; this.order = order; }
Also, this example intentionally illustrates the famous
ServiceLocator pattern, which stores all the dependencies of the application. That is why such a pattern is considered an anti-pattern. Try to avoid it.
Consider another example of mining knowledge:
before: untested code class SalesTaxCalculator { TaxTable taxTable; SalesTaxCalculator(TaxTable taxTable) { this.taxTable = taxTable; } float computeSalesTax(User user, Invoice invoice) {
In the test, you must create a class User, although it only requires an address. The same can be said about the class Invoice. Again, from the one who writes the test, it is required to “go through” the code of SalesTaxCalculator in order to figure out what is really needed there. Bagoic place.
Also, SalesTaxCalculator cannot be reused in another project that does not have the User and Invoice classes.
after: test code class SalesTaxCalculator { TaxTable taxTable; SalesTaxCalculator(TaxTable taxTable) { this.taxTable = taxTable; } float computeSalesTax(Address address, float amount) { return amount * taxTable.getTaxRate(address); } }
Now the class requires only what is needed, and the tests become transparent.
New operator in business code
Perhaps this pattern can be given a gold medal for creating non-testable code.
Let's start parsing with a simple example:
before: untested code class House { Kitchen kitchen; Bedroom bedroom = new Bedroom(); House() { this.kitchen = new Kitchen(new Refrigerator()); }
In this code, everything is bad. It is impossible to test it, because calling any method of House will lead to a call to the kitchen and / or bedroom, and we do not control them. If they communicate with the database or send requests to the network, then the tests are doomed. With the help of polymorphism, it is impossible to replace the kitchen or bedroom, our house is rigidly tied to certain classes. How to make the code testable?
after: test code class House { Kitchen kitchen; Bedroom bedroom; House(Kitchen kitchen, Bedroom bedroom) { this.kitchen = kitchen; this.bedroom = bedroom; }
Now our class House requires the designer everything that it needs to work. And he doesn't care where it comes from. Any business class should require ready-made instances of its dependencies in its constructor; this is the main principle.
But some readers will say: “Just a minute. If now all the dependencies need to be demanded in the constructor, then in order to create a “deep” child class (a class located in the depth of the application dependency graph), I will have to push all dependencies of this class through the parent classes. And this will lead to the fact that the constructors of the “upper” classes (classes that are closer to the beginning of the dependency graph) will turn into a collection of everything. As for the example, this means that the class that makes the
new House will now have to demand the most in the designer kitchen, and the bedroom. But why should he know about them? Well this is nonsense. "
Only in these arguments there is one mistake. Following the above principle, the class that creates House should NOT create it at all. After all, according to the principle, he himself must demand House to himself in dependence. But then where, in the end, will take the class House? It will be created at the start of the application (if the lifetime of the House coincides with the lifetime of the application) or a factory creates it (if the lifetime of the House is less than the lifetime of the application):
House class creation class HouseFactory { House build() { Kitchen kitchen = new Kitchen(new Refrigerator()); Bedroom bedroom = new Bedroom(); return new House (kitchen, bedroom); } }
But some will again object: “So now, create a factory for each class, increasing the amount of code by 2 times? Well this is nonsense. ” In fact, one factory creates and links classes with the same lifetime. But in real applications there is not such a large amount of code with different lifetime. For example, consider what types of objects, divided by different lifetimes, exist in a web application:
- long living objects (created at the start of the application)
- session objects
- request objects
- rarely objects with a lifetime shorter than the query
So it’s enough for four factories for a typical web application. Therefore, do not be afraid of them (and below, in conclusion, it will be shown how factories can not be written at all).
Thus, we can distinguish two groups of code:
- business code group
- binding code group
Business code - a code that reflects business logic, a code that will change when customer requirements change. This is the main category of code, which is why we are writing code.
Binding code is a code that allows a business code to interact, it links it together. Binding code does not contain any business logic. These include factories, the starting point of the application and other connecting sites. In the binding code, there is a lot of operator
new .
This approach with the division of code into 2 groups allows you to avoid mixing the logic of the application with the creation of the object graph of the application. Therefore, always when using the new operator, think about whether you use it in that group of code.
An exception:
The
new operator can be used in business code to create storage objects that do not contain behavior (for example, HashMap, Array).
Consider another common case in the following example:
before: untested code class DocumentActions { Network network; DocumentModel documentModel; DocumentActions(Network network, DocumentModel documentModel) {
The DocumentActions class in each of its methods creates a Revision class, making it impossible for tests to substitute the implementation of this class for mocks. Worse, DocumentActions requires classes in the constructor that it does not use. But what to do if every time you need to create a Revision you need to give the third argument textOffset, which is not known in advance? Output: create a class that will take the knowledge of how to create Revision. And this is nothing like a factory:
after: test code class DocumentActions { RevisionFactory revisionFactory; DocumentActions(RevisionFactory revisionFactory) { this.revisionFactory = revisionFactory; } changeTextStyle(int textOffset, TextStyle style) { Revision revision = this.revisionFactory.build(textOffset); revision.updateTextStyle(style); revision.apply(); } insertParagraph(int textOffset, ParagraphProps props, ParagraphStyle style) { Revision revision = this.revisionFactory.build(textOffset); revision.addParagraph(props); revision.updateParagraphStyle(style); revision.apply(); }
The factory knows that Network and DocumentModel are definitely needed to create Revision. Therefore, she herself requires these classes for herself. And all the dynamic parameters (textOffset), for creating Revision, will be required as arguments of the factory build method.
Now, if in the future the Revision class will require another constant argument to the constructor, then it is not difficult to correct the RevisionFactory a bit, and the DocumentActions class will remain completely unchanged.
Global variables and singletons
I think everyone will agree that global variables are evil. But many do not see anything wrong with singletons, although in essence these are all the same global variables. So what are the bad singltons? Here are 2 reasons (although any of them are enough):
1) The problem with using singletons is that they introduce tight connectivity to the code. By creating a singleton, you say that your classes can only work with a single implementation that singleton provides. You do not pledge to replace it. This makes it difficult to test the class in isolation from the singleton, because the very nature of the test requires the ability to replace implementations with alternative ones. Until you change this design, you force yourself to rely on the correct operation of the singleton to test any of its clients.
2) The presence of singletons makes your classes lie about their dependencies, as it introduces "invisible" dependencies. To understand the real dependencies of a class, you need to read its code completely, instead of just looking at the list of constructor / method dependencies. And sooner or later it will lead to the fact that the tests will begin to influence each other, through a hidden global state.
A practical example illustrating both problems at once: in a web application, it became necessary to log user actions. For this we created a singleton, which was used in a number of classes. I’ll mention that Singleton used jQuery to get additional information from the DOM. Everything went well until it was necessary to reuse part of the classes in the node version of the application. This version periodically began to fall during testing. It turned out that classes that used the logger singleton fell into this version, and the node does not have a DOM. These classes used the “invisible” dependency (reason 2), with the result that this situation turned out to be possible. It was not possible to replace the logger with another one (reason 1), due to which some classes had to be redone.
Some will say: "I deliberately use singleton to have only a single instance of the class for the entire application." But when creating a singleton, you make a class instance unique to the entire code execution space (jvm in java, rhino or V8 in javascript), and not in the whole application. It's just that in most cases there is only one application inside the code execution space, and it turns out that these spaces are the same. But this is not the case for tests. Each test is a part of an application running in the same execution space with other tests.
It turns out that many of your mini applications start to live in one execution space, in which there are unique and irreplaceable singltons. This is where the side effects of singletons appear, when tests begin to influence each other.
In addition, you may someday need to have more than one copy of the application or its part in the same code execution space. Then the application space will not be equal to the code execution space. And there will be nothing else left but to do the job of getting rid of singletons.
Consider this example: you are assigned to test the PhoneAccount class, which is responsible for operations with a telephone account. Without thinking, you write:
attempt 1 PhoneAccount phoneAccount = new PhoneAccount('79008001020'); phoneAccount.addMoney(100); expect(phoneAccount.getBalance()).toBe(100);
You start your test in isolation and in runtime you get an access error to null. Something went wrong? You ask a colleague who wrote this class. He thinks a long time that he needs to initialize the singleton PhoneAccountTransactionProcessor class. You think: "how should I have guessed this?". Then add:
attempt 2 PhoneAccountTransactionProcessor.init(...); PhoneAccount phoneAccount = new PhoneAccount('79008001020'); phoneAccount.addMoney(100); expect(phoneAccount.getBalance()).toBe(100);
But again, when you start, you get an access error to null. In perplexity, you again ask your colleague: “What am I doing wrong?”. What do you get the answer: “Did you initialize the transaction queue - AccountTransactionQueue?”. You get a little annoyed with the code, but something tells you that this will not stop there, and ask not to leave the “experienced” colleague.
attempt 3 AccountTransactionQueue.start(...); PhoneAccountTransactionProcessor.init(...); PhoneAccount phoneAccount = new PhoneAccount('79008001020'); phoneAccount.addMoney(100); expect(phoneAccount.getBalance()).toBe(100);
And again a mistake. But you don’t have time to ask a question how a colleague gives out: “You didn’t connect the transaction database!”. Finishing, taking a deep breath:
attempt 4 TransactionsDataBase.connect(...) AccountTransactionQueue.start(...); PhoneAccountTransactionProcessor.init(...); PhoneAccount phoneAccount = new PhoneAccount('79008001020'); phoneAccount.addMoney(100); expect(phoneAccount.getBalance()).toBe(100);
Finally, the test passes.
I think you already understand what is wrong with this code - some kind of black magic. Three classes lie about their dependencies, the order of initializations should be exactly this, although the code does not dictate to you exactly this order.
And what if there are no singltons, and each class will require everything it needs to work in a constructor?
correct code TransactionsDataBase db = new TransactionsDataBase(...); AccountTransactionQueue transactionQueue; transactionQueue = new AccountTransactionQueue(db); PhoneAccountTransactionProcessor transactionProcessor; transactionProcessor = PhoneAccountTransactionProcessor(transactionQueue); PhoneAccount phoneAccount = new PhoneAccount('79008001020', transactionProcessor); phoneAccount.addMoney(100); expect(phoneAccount.getBalance()).toBe(100);
Now there is no magic, no hidden channels of communication between classes. Any new team member will be able to write such a test on his own, because he will immediately see all the dependencies. The code itself dictates the order of initialization; it cannot be done otherwise. And this is a huge plus, because in real applications there may be dozens of lines with the initialization of classes. And one more important conclusion: in such code, it is possible to pass null instead of a class (where possible) or replace the class with a mock. In the singleton code it was impossible.
Another example:
before: untested code class LoginService { private static LoginService instance; private LoginService() {}; static LoginService getInstance() { if (instance == null) { instance = new RealLoginService(); } return instance; }
Removing singleton:
Now you can easily and easily replace the LoginService, as well as the “test-only” methods are not needed.
Exceptions, in which cases the singletons can still be used:
- Singleton can be used for immutable objects, that is, for those that do not change during the life of the application. For example: constant settings
- Singleton can be used when an object with information is not used in an application. For example, logger or google analyst. The application only writes information, but cannot read it. Such singletons are safe for the application, since they seem to have no global status, it’s just a pipe to nowhere. But if you need to test that writing to the logs really happens, then you will have to change the dependencies and lower the logger into the constructor.
Also, some system objects have hidden global variables, such as
math.random and
new Date () . If you want to test classes that use such objects, then you need to create your own wrappers with them in order to be able to replace them.
Experienced designer
The hardened constructor is a constructor that does something other than initializing the fields of its class. Strictly speaking, the constructor should be responsible only for initializing the class fields, and any other work violates the Single Responsibility Principle. Such “extra” work in the constructor makes it difficult to test it, since the test cannot transfer its dependency stubs to the class under test. Here are typical patterns of experienced designers:
The last 3 patterns are characteristic not only for designers, therefore they were considered for the general case above.
Initialization of constructor arguments
before: untested code class Metro { TicketPrices ticketPrices; Metro(TicketPrices ticketPrices) { this.ticketPrices = ticketPrices; ticketPrices.setCostCalculator(new MoscowCostCalculatorWithVerySlowConstructor()); } }
The constructor of the Metro class does not fulfill its duty - initializes the argument. This code is bad for many reasons, but we are interested in testability. If initialization is slow, all tests on Metro will take a long time.
after: test code class Metro { TicketPrices ticketPrices; Metro(TicketPrices ticketPrices) { this.ticketPrices = ticketPrices; } } class TicketPricesFactory { TicketPrices build() { TicketPrices ticketPrices = new TicketPrices(); ticketPrices.setCostCalculator(new VerySlowMoscowCostCalculator()); return ticketPrices; } }
Now in the test we can not create classes that are not necessary for the test, replacing them with dummies. The logic of creating and initializing TicketPrices has gone to the factory.
Conditions and cycles
before: untested code class Car { IEngine engine; Car() { if (FLAG_ENGINE.get()){ this.engine = new V8Engine(); } else { this.engine = new V12Engine(); } } }
The test is tied to a certain flag. 2 .
: class Car { IEngine engine; Car(IEngine engine) { this.engine = engine; } } class EngineFactory { IEngine build(boolean isV8) { if (isV8){ return new V8Engine(); } else { return new V12Engine(); } } }
before: untested code class Voicemail { User user; private List<Call> calls; Voicemail(User user) { this.user = user; } init(Server server) { this.calls = server.getCallsFor(this.user); }
init, , . init , calls init. , , . . — . . init. , init, initialize, setup , . Voicemail (server.getCallsFor). Voicemail:
: class Voicemail { List<Call> calls; Voicemail(List<Call> calls) { this.calls = calls; }
Conclusion
, ( new , ), ,
Dependency Injection .
IoC , — (), . IoC . IoC — .
, IoC ( ), , Dependency Injection. , .
, . ( ), ( «» angular
Miško Hevery ), .