Common mistakes when writing unit tests. Yandex lecture
If you master a small list of typical errors that occur when writing unit tests, you can even fall in love with writing them. Today the head of the Yandex. Browser development team for Android Konstantin kzaikin Zaikin will share his experience with Habr's readers.
- I have a practical report. I hope he will benefit you all - both those who already write unit tests, those who only think to write, and those who try and who failed.
We have a pretty big project. One of the largest mobile projects in Russia. We have a lot of code, a lot of tests. Tests chase each pool rekvest, they do not fall at the same time.
Who knows what the test coverage in his project? Zero, okay. Who has unit tests in the project? And who believes that unit tests are not needed? I do not see anything wrong with this, there are people who are sincerely convinced of this, and my story should help them to dissuade it.
To this happiness, the green tests - thousands of them - we did not come immediately. No silver bullet, and the main idea of my report on the screen:
The Chinese dictum says in hieroglyphs that the journey of a thousand li begins with one step. It seems that there is such an analogue of this saying.
We made a decision a long time ago that we need to improve our product, our code, and we are purposefully moving towards this. On this way, we met many cones, an underwater rake, and gathered together with it some convictions.
Why do we need tests?
In order not to drop the old features when we introduce new ones. That was a badge on GitHub. To refactor existing features - a deep thought, it needs to be revealed to those who do not write tests. To prevent existing features from falling during refactoring, we will protect ourselves with tests. That the head zaapruvil pool rekvest, it is yes.
My opinion - please do not associate it with the opinion of my team - that tests help us. They allow you to run your code without putting it into production, without installing it on devices, you launch it very quickly and run it. You can get rid of all the corner cases that you don’t get in your life on the device and in production, and your tester doesn’t invent them. But you as an developer with an inquisitive mind come up with, check and fix bugs at the earliest possible stage.
Very important: tests tell how, in the opinion of the developer, the code should work and what, according to the developer, your methods should do. These are not comments that turn away and after a while from useful ones become harmful. It happens that one thing is written in the comments, but something completely different in the code. Unit tests in this sense can not lie. If the test is green, it documents what is happening there. The test broke - you violated the developer’s initial intention.
To fix contracts. These are not contracts with a signature and a stamp, but program contracts of behavior of classes. If you refactor, in this case the contracts will be broken and the tests will fall if you break them. If the contracts continue, the tests remain green, you will have more confidence that your refactoring is correct.
This is the general idea of my entire report. You can show the first line and go.
Many people think that test code is so-so code, it is not for production, so you can write it so-so. I categorically disagree with this and I think that the tests should be approached first and foremost responsibly, as well as the production code. If you approach them in the same way, then the tests will be useful. Otherwise it will be one smut.
Specifically, the two lines below apply to any code, it seems.
KISS - keep it simple, stupid. No need to complicate. Tests should be simple. And the production code should be simple, but tests especially. If you have tests that are easy to read, then these will be tests that are most likely well written, they well expressed the idea that they will be easy to verify. Even during a pull request, a person who looks at your new tests will understand what you want to say. And if something breaks, you can easily understand what happened.
DRY - don't repeat yourself. In tests, the developer is often inclined to use the forbidden technique that nobody seems to use in production - copy paste. In a production developer who will actively copy and paste, they simply won’t understand. In tests, this is a normal practice, unfortunately. No need to do so, because - the first line. If you write tests in an honest way, like real good code, the tests will be useful to you.
While we were developing our hundreds of thousands of lines of code, wrote thousands of tests, collected rakes, I had typical remarks to the tests. I'm lazy enough, and when I came to pool requests and watched the same mistakes, on the basis of the DRY principle, I decided to write down these typical problems, and made them on the internal wiki first, and then put some practical test smells on GitHub that you can use. when writing tests.
I will list the points. Increment the counter in your mind if you remember such a test smell. If you count to five, then you can raise your hand and shout “Bingo!” And in the end, I wonder who counted up to how many. My counter will be equal to the number of points, I collected them all myself.
You know the most difficult thing about programming. And in tests it is really important. If you do not call the test well, then most likely you will not be able to formulate what the test checks.
Humans are fairly simple creatures, they are easily trapped by names. Therefore, I ask you to call the tests well. Formulate what the test checks and follow the simple rules.
If there is no description in the test name of what the test checks, for example, you have the Controller class, and you write the test testController, what do you check? What should this test do? Most likely, either nothing or too many things to check. Neither one nor the other does not suit us. Therefore, in the name of the test you need to write what we check.
You can not go to the other extreme. The name of the test should be short enough so that the person can easily parse it. In this sense, Kotlin is beautiful because it allows you to write test names in quotes with spaces in normal English. It is easier to read them. But still long names - it is smell.
If your test name is too long, most likely, you put too many test methods in one test class, and you need to clarify what you are testing. In this case, you need to spread your test class to several. Do not be afraid of this. You will have the name of the test class that checks the name of your production code, and there will be short test names.
This is an atavism. Previously, in Java, everything was tested using JUnit, where before the fourth version there was an agreement that test methods should begin with the word test. It so happened, so far everyone is called that. But there is a problem, in English, the word test is the verb "check." People are easily caught in this trap, and no longer write any other verbs. Write testController. It is easy to verify yourself: if you have not written a verb that your test class should do, most likely you didn’t check something, didn’t write well enough in the title. Therefore, I always ask to remove the word test from the names of test methods.
I tell very simple things, but oddly enough, they help. If the tests are called well, most likely under the hood they will look good. It is very simple.
I actually read the test smells as gitub. Link below, you can walk and enjoy.
There are many asserts in the test method. So maybe or not? May be. Is it good or bad? I think this is very bad. If you have written several assertions in the test method, then you check several assertions. If you test your test and the first assert falls, will the test reach the second assertion? Will not come. After the fall of your assembly somewhere on CI, you will get the test dropped, you will go to fix something, flood it again, it will fall on the next assertion. It may well be.
At the same time, it would be much cooler if you sawed this test method into several, and all the methods with several asserts fell at the same time, because they would be launched independently of each other.
A few asserts can mask the different actions that are performed with the test class. I recommend writing one test - one assert. Moreover, assertions can be quite complex. My colleague in the very first report showed a piece of code, where he used the magnificent construction assertThat and the matcher. I really love JUnit's gamers, so you can use them too. For the reader of tests, it is obtained simply by one short operator. GitHub has examples of all these smells and how to fix them. There is an example of bad code and a number of good ones. This is all done in the form of a project that you can download, open, compile and run all the tests.
The next smell is closely related to the previous one. You do something with the system - do an assert. You do something else with the system, some long operations - you do an assert - you do something else. In fact, you just cut into several methods, and you get solid good test methods.
This refers to verbosity. If you have a test class, and in each test method, the same methods are executed at the beginning.
A test class in which you run the same methods at the beginning. It seems a bit, but in every test method this garbage is present. And if it is common to all test methods, then why not carry it to the constructor or to the Before block or the Before Each block in JUnit 5. If you do this, the readability of each method will improve, plus you will get rid of the DRY sin. Such tests are easier to maintain and easier to read.
The reliability of tests is very important. There are signs by which it can be determined that the test will flaccate, to be either green or red. When the developer writes it, he is sure that he is green, and then for some reason the tests become green, then red, give us pain and insecurity in general, that the tests are useful. We are not sure about the tests, which means that we are not sure that they are useful.
I myself once wrote tests that had Math.random () inside, did random numbers, did something with them. Don't do that. We expect that the test system enters the test in the same configuration, and the output must also be the same. Therefore, in unit tests, for example, you never need to do any operations with the network. Because the server may not respond, there may be different timings, something else.
If you need a test that works with the network, make a proxy, local, whatever, but in no case do not go to the real network. This is the same random. And of course, you cannot use random data. If you have to do something, make a few examples with boundary conditions, with bad conditions, but they must be hardcoded.
The classic problem faced by developers when trying to test some kind of asynchronous code. This is what I did in the test, and then I need to wait until it is executed. How to do? Thread.sleep (), of course.
There is a problem. When you developed your test, for example, you did it on some of your typewriter, it works at some speed. Tests you run on another machine. And what will happen if your system during Thread.sleep () does not have time to work? The test will turn red. This was unexpected. Therefore, the recommendation here is that if you perform asynchronous operations, do not test them at all. Almost any asynchronous operation can be deployed so that you will have some kind of conditional mechanism providing asynchronous and synchronously running block of code. For example, AsyncTask inside has a synchronously running block of code. You can easily test it synchronously, without any asynchronous. There is no need to test AsyncTask itself, this is a framework class, why test it? Take it out of your brackets and your life will become easier.
Thread.sleep () delivers a lot of pain. Besides the fact that it worsens the reliability of tests, since it allows them to fly because of different timings on devices, it also slows down the execution of your tests. Who would like his unit tests, which must run for milliseconds, run for five seconds, because I set tread sleep?
Typical smell, that we changed some kind of global static variable at the beginning of the test, to check that our system is working correctly, and did not return at the end. Then we get a cool situation: on the machine, the developer performed the tests in the same sequence, first checked the global variable with the default value, then changed it in the test with another, then did something else. Both dough are green. And on the CI, as it happened, the tests started in the reverse order. And either one or both tests will be red, although they were all green.
Need to tidy up after themselves. The rules of boy scouts in this sense: changed the global variable - return to the original state. Better yet, do not use global states. But this is a deeper thought. It is about the fact that tests sometimes highlight defects in architecture. If we have to change global states and return them to the initial ones in order to write tests, do we really do everything well in our architecture? Do we really need global variables, for example? As a rule, you can do without them by injecting some classes of contexts or something so that you can reinitialize, inject, and cleanly execute them in the test each time.
Test smell for advanced. The need to use such a thing does not arise on the first day, as a rule. You have already tested something, and here you needed to translate the class into some specific state. And you do backdoor yourself. You have a production class, and you do a specific method that will never be called in production, and through it you inject something into a class or change its state. Thereby maliciously breaking encapsulation. In production, your class works somehow, but in tests, in fact, this is a different class, you communicate with it through other inputs and outputs. And here you can get a situation where you change the production, and the tests will not notice. Tests continue to go through the backdoor and did not notice that, for example, exceptions started to shoot in the constructor, as they walk through another constructor.
In general, you should test your classes through the same inputs and outputs as in production. There should be no access to any methods only for tests.
How many 15,000 tests are performed? About 20 minutes, at each pull-request, on Team City, developers are forced to wait. Just because 15 thousand is a lot of tests. And in this section, I collected smells that slow down the tests. Although thread_sleep was already.
In Android, there are instrumentation tests, they are beautiful, they run on a device or an emulator. This will raise your project completely, for real, but they are very slow. And for them you need to even raise the whole emulator. Even if you imagine that you have a raised emulator on the CI - so it coincided that you have it - then the test on the emulator will take much longer than on the host machine, for example, using Robolectric. Although there are other methods. This is a framework that allows you on a host machine, on pure Java, to work with classes from the Android framework. We use it quite actively. Google used to treat it somewhat coolly, but now Googleers themselves are talking about it on various reports, it is recommended for use.
Android framework in Robolectric is emulated. It is not complete there, although the implementation the further, the more complete. It's almost real Android, only running on your desktop, laptop or CI. But it also does not need to be used everywhere. Robolectric is not free. If you have a test that you heroically transferred from Android instrumentation to Robolectric, you need to think - maybe go even further, get rid of the Robolectric, turn it into the simplest JUnit test? Robolectric tests take time to initialize, try to load resources, initialize your activity, application and everything else. It takes some time. This is not a second, it is milliseconds, sometimes tens and hundreds. But when there are a lot of tests, even this matters.
There are techniques that allow you to get rid of Robolectric. You can isolate your code through interfaces by wrapping the entire platform part with interfaces. Then it will be just a JUnit-host test. JUnit on the host machine is very fast, there is a minimum amount of overhead, such tests can be run by thousands and tens of thousands, they will be executed a minute, a few minutes. Unfortunately, our tests are performed for a long time, because we have a lot of Android instrumentation tests, because we have a native part in the browser and we have to perform them on a real emulator or device. So for so long.
I will not tire you anymore. How many smells do you have? While seven maximum. Subscribe to the channel , put the stars.
- I have a practical report. I hope he will benefit you all - both those who already write unit tests, those who only think to write, and those who try and who failed.
We have a pretty big project. One of the largest mobile projects in Russia. We have a lot of code, a lot of tests. Tests chase each pool rekvest, they do not fall at the same time.
Who knows what the test coverage in his project? Zero, okay. Who has unit tests in the project? And who believes that unit tests are not needed? I do not see anything wrong with this, there are people who are sincerely convinced of this, and my story should help them to dissuade it.
To this happiness, the green tests - thousands of them - we did not come immediately. No silver bullet, and the main idea of my report on the screen:
The Chinese dictum says in hieroglyphs that the journey of a thousand li begins with one step. It seems that there is such an analogue of this saying.
We made a decision a long time ago that we need to improve our product, our code, and we are purposefully moving towards this. On this way, we met many cones, an underwater rake, and gathered together with it some convictions.
Why do we need tests?
In order not to drop the old features when we introduce new ones. That was a badge on GitHub. To refactor existing features - a deep thought, it needs to be revealed to those who do not write tests. To prevent existing features from falling during refactoring, we will protect ourselves with tests. That the head zaapruvil pool rekvest, it is yes.
My opinion - please do not associate it with the opinion of my team - that tests help us. They allow you to run your code without putting it into production, without installing it on devices, you launch it very quickly and run it. You can get rid of all the corner cases that you don’t get in your life on the device and in production, and your tester doesn’t invent them. But you as an developer with an inquisitive mind come up with, check and fix bugs at the earliest possible stage.
Very important: tests tell how, in the opinion of the developer, the code should work and what, according to the developer, your methods should do. These are not comments that turn away and after a while from useful ones become harmful. It happens that one thing is written in the comments, but something completely different in the code. Unit tests in this sense can not lie. If the test is green, it documents what is happening there. The test broke - you violated the developer’s initial intention.
To fix contracts. These are not contracts with a signature and a stamp, but program contracts of behavior of classes. If you refactor, in this case the contracts will be broken and the tests will fall if you break them. If the contracts continue, the tests remain green, you will have more confidence that your refactoring is correct.
This is the general idea of my entire report. You can show the first line and go.
Many people think that test code is so-so code, it is not for production, so you can write it so-so. I categorically disagree with this and I think that the tests should be approached first and foremost responsibly, as well as the production code. If you approach them in the same way, then the tests will be useful. Otherwise it will be one smut.
Specifically, the two lines below apply to any code, it seems.
KISS - keep it simple, stupid. No need to complicate. Tests should be simple. And the production code should be simple, but tests especially. If you have tests that are easy to read, then these will be tests that are most likely well written, they well expressed the idea that they will be easy to verify. Even during a pull request, a person who looks at your new tests will understand what you want to say. And if something breaks, you can easily understand what happened.
DRY - don't repeat yourself. In tests, the developer is often inclined to use the forbidden technique that nobody seems to use in production - copy paste. In a production developer who will actively copy and paste, they simply won’t understand. In tests, this is a normal practice, unfortunately. No need to do so, because - the first line. If you write tests in an honest way, like real good code, the tests will be useful to you.
While we were developing our hundreds of thousands of lines of code, wrote thousands of tests, collected rakes, I had typical remarks to the tests. I'm lazy enough, and when I came to pool requests and watched the same mistakes, on the basis of the DRY principle, I decided to write down these typical problems, and made them on the internal wiki first, and then put some practical test smells on GitHub that you can use. when writing tests.
I will list the points. Increment the counter in your mind if you remember such a test smell. If you count to five, then you can raise your hand and shout “Bingo!” And in the end, I wonder who counted up to how many. My counter will be equal to the number of points, I collected them all myself.
GitHub link
You know the most difficult thing about programming. And in tests it is really important. If you do not call the test well, then most likely you will not be able to formulate what the test checks.
Humans are fairly simple creatures, they are easily trapped by names. Therefore, I ask you to call the tests well. Formulate what the test checks and follow the simple rules.
no_action_or_assertion
If there is no description in the test name of what the test checks, for example, you have the Controller class, and you write the test testController, what do you check? What should this test do? Most likely, either nothing or too many things to check. Neither one nor the other does not suit us. Therefore, in the name of the test you need to write what we check.
long_name
You can not go to the other extreme. The name of the test should be short enough so that the person can easily parse it. In this sense, Kotlin is beautiful because it allows you to write test names in quotes with spaces in normal English. It is easier to read them. But still long names - it is smell.
If your test name is too long, most likely, you put too many test methods in one test class, and you need to clarify what you are testing. In this case, you need to spread your test class to several. Do not be afraid of this. You will have the name of the test class that checks the name of your production code, and there will be short test names.
older_prefix
This is an atavism. Previously, in Java, everything was tested using JUnit, where before the fourth version there was an agreement that test methods should begin with the word test. It so happened, so far everyone is called that. But there is a problem, in English, the word test is the verb "check." People are easily caught in this trap, and no longer write any other verbs. Write testController. It is easy to verify yourself: if you have not written a verb that your test class should do, most likely you didn’t check something, didn’t write well enough in the title. Therefore, I always ask to remove the word test from the names of test methods.
I tell very simple things, but oddly enough, they help. If the tests are called well, most likely under the hood they will look good. It is very simple.
I actually read the test smells as gitub. Link below, you can walk and enjoy.
multiple_asserts
There are many asserts in the test method. So maybe or not? May be. Is it good or bad? I think this is very bad. If you have written several assertions in the test method, then you check several assertions. If you test your test and the first assert falls, will the test reach the second assertion? Will not come. After the fall of your assembly somewhere on CI, you will get the test dropped, you will go to fix something, flood it again, it will fall on the next assertion. It may well be.
At the same time, it would be much cooler if you sawed this test method into several, and all the methods with several asserts fell at the same time, because they would be launched independently of each other.
A few asserts can mask the different actions that are performed with the test class. I recommend writing one test - one assert. Moreover, assertions can be quite complex. My colleague in the very first report showed a piece of code, where he used the magnificent construction assertThat and the matcher. I really love JUnit's gamers, so you can use them too. For the reader of tests, it is obtained simply by one short operator. GitHub has examples of all these smells and how to fix them. There is an example of bad code and a number of good ones. This is all done in the form of a project that you can download, open, compile and run all the tests.
many_tests_in_one
The next smell is closely related to the previous one. You do something with the system - do an assert. You do something else with the system, some long operations - you do an assert - you do something else. In fact, you just cut into several methods, and you get solid good test methods.
repeating_setup
This refers to verbosity. If you have a test class, and in each test method, the same methods are executed at the beginning.
A test class in which you run the same methods at the beginning. It seems a bit, but in every test method this garbage is present. And if it is common to all test methods, then why not carry it to the constructor or to the Before block or the Before Each block in JUnit 5. If you do this, the readability of each method will improve, plus you will get rid of the DRY sin. Such tests are easier to maintain and easier to read.
The reliability of tests is very important. There are signs by which it can be determined that the test will flaccate, to be either green or red. When the developer writes it, he is sure that he is green, and then for some reason the tests become green, then red, give us pain and insecurity in general, that the tests are useful. We are not sure about the tests, which means that we are not sure that they are useful.
random
I myself once wrote tests that had Math.random () inside, did random numbers, did something with them. Don't do that. We expect that the test system enters the test in the same configuration, and the output must also be the same. Therefore, in unit tests, for example, you never need to do any operations with the network. Because the server may not respond, there may be different timings, something else.
If you need a test that works with the network, make a proxy, local, whatever, but in no case do not go to the real network. This is the same random. And of course, you cannot use random data. If you have to do something, make a few examples with boundary conditions, with bad conditions, but they must be hardcoded.
tread_sleep
The classic problem faced by developers when trying to test some kind of asynchronous code. This is what I did in the test, and then I need to wait until it is executed. How to do? Thread.sleep (), of course.
There is a problem. When you developed your test, for example, you did it on some of your typewriter, it works at some speed. Tests you run on another machine. And what will happen if your system during Thread.sleep () does not have time to work? The test will turn red. This was unexpected. Therefore, the recommendation here is that if you perform asynchronous operations, do not test them at all. Almost any asynchronous operation can be deployed so that you will have some kind of conditional mechanism providing asynchronous and synchronously running block of code. For example, AsyncTask inside has a synchronously running block of code. You can easily test it synchronously, without any asynchronous. There is no need to test AsyncTask itself, this is a framework class, why test it? Take it out of your brackets and your life will become easier.
Thread.sleep () delivers a lot of pain. Besides the fact that it worsens the reliability of tests, since it allows them to fly because of different timings on devices, it also slows down the execution of your tests. Who would like his unit tests, which must run for milliseconds, run for five seconds, because I set tread sleep?
modify_global
Typical smell, that we changed some kind of global static variable at the beginning of the test, to check that our system is working correctly, and did not return at the end. Then we get a cool situation: on the machine, the developer performed the tests in the same sequence, first checked the global variable with the default value, then changed it in the test with another, then did something else. Both dough are green. And on the CI, as it happened, the tests started in the reverse order. And either one or both tests will be red, although they were all green.
Need to tidy up after themselves. The rules of boy scouts in this sense: changed the global variable - return to the original state. Better yet, do not use global states. But this is a deeper thought. It is about the fact that tests sometimes highlight defects in architecture. If we have to change global states and return them to the initial ones in order to write tests, do we really do everything well in our architecture? Do we really need global variables, for example? As a rule, you can do without them by injecting some classes of contexts or something so that you can reinitialize, inject, and cleanly execute them in the test each time.
@VisibleForTesting
Test smell for advanced. The need to use such a thing does not arise on the first day, as a rule. You have already tested something, and here you needed to translate the class into some specific state. And you do backdoor yourself. You have a production class, and you do a specific method that will never be called in production, and through it you inject something into a class or change its state. Thereby maliciously breaking encapsulation. In production, your class works somehow, but in tests, in fact, this is a different class, you communicate with it through other inputs and outputs. And here you can get a situation where you change the production, and the tests will not notice. Tests continue to go through the backdoor and did not notice that, for example, exceptions started to shoot in the constructor, as they walk through another constructor.
In general, you should test your classes through the same inputs and outputs as in production. There should be no access to any methods only for tests.
How many 15,000 tests are performed? About 20 minutes, at each pull-request, on Team City, developers are forced to wait. Just because 15 thousand is a lot of tests. And in this section, I collected smells that slow down the tests. Although thread_sleep was already.
unnecessary_android_test
In Android, there are instrumentation tests, they are beautiful, they run on a device or an emulator. This will raise your project completely, for real, but they are very slow. And for them you need to even raise the whole emulator. Even if you imagine that you have a raised emulator on the CI - so it coincided that you have it - then the test on the emulator will take much longer than on the host machine, for example, using Robolectric. Although there are other methods. This is a framework that allows you on a host machine, on pure Java, to work with classes from the Android framework. We use it quite actively. Google used to treat it somewhat coolly, but now Googleers themselves are talking about it on various reports, it is recommended for use.
unnecessary_robolectric
Android framework in Robolectric is emulated. It is not complete there, although the implementation the further, the more complete. It's almost real Android, only running on your desktop, laptop or CI. But it also does not need to be used everywhere. Robolectric is not free. If you have a test that you heroically transferred from Android instrumentation to Robolectric, you need to think - maybe go even further, get rid of the Robolectric, turn it into the simplest JUnit test? Robolectric tests take time to initialize, try to load resources, initialize your activity, application and everything else. It takes some time. This is not a second, it is milliseconds, sometimes tens and hundreds. But when there are a lot of tests, even this matters.
There are techniques that allow you to get rid of Robolectric. You can isolate your code through interfaces by wrapping the entire platform part with interfaces. Then it will be just a JUnit-host test. JUnit on the host machine is very fast, there is a minimum amount of overhead, such tests can be run by thousands and tens of thousands, they will be executed a minute, a few minutes. Unfortunately, our tests are performed for a long time, because we have a lot of Android instrumentation tests, because we have a native part in the browser and we have to perform them on a real emulator or device. So for so long.
I will not tire you anymore. How many smells do you have? While seven maximum. Subscribe to the channel , put the stars.
Source: https://habr.com/ru/post/436850/