If you have been in software development for a long time, then you can easily relate to the importance of unit testing. Experts say that most bugs can be captured in the unit testing phase itself, which eventually gets passed to quality teams if we follow these best practices for writing unit tests.
Though, bad unit tests are a reality and everyone who does code reviews, faces them occasionally (may be regularly). So what constitutes a bad test case? How to identify poorly written unit tests?
In this post, we are trying to identify 8 such signs, which will provide us with subtle hints, that a particular test case should be improved.
“It’s overwhelmingly easy to write bad unit tests that add very little value to a project while inflating the cost of code changes astronomically.”
Believe me, I have seen such tests in my previous projects which seem to do lots of stuff in code, but in reality, they were doing nothing. They were sending requests to the server and no matter what server respond, they were passing. Horror !!
Don’t let these unit tests sneak into your code repository, through strict code reviews. Once they make their way into your code base, they will become a liability to carry them, build them and run them every time but without adding any value.
2. Testing irrelevant things
This one is another sign of a bad test case. I have seen developers checking multiple irrelevant things so that code passes with doing SOMETHING, well not necessarily the correct thing.
The best approach is to follow the single responsibility principle, which says, one unit test case should test only one thing and should fail for a single reason. The idea is to identify the root cause of failure just by seeing the test name. If we have to debug for the failure point, we probably need more clearly written tests.
This one seems similar to the above sign, but it is not. Where in the previous sign, the test was testing irrelevant things, here in this sign, unit-test test the correct things but many of such things in one test case. This is again a violation of the single responsibility principle.
Well, please note that I am not encouraging the use of a single assertion per test. To test a single entity, you might need to use multiple assertions and do it as needed.
For example, one API takes some parameters in the post body and creates an Employee record and returns it in response. This Employee object can have multiple fields like first name, last name, address etc. Writing a test case to validate only first-name, then another for the last name and another for the address is code duplication, without any value. Don’t do it. It proves nothing except you can copy and paste.
Instead, write a single positive test case to create employee and validate all fields in that unit test. Negative test cases should be written separately doing only one thing and one assertion in this case. e.g. One test case for a blank first name, one test case for an invalid first name and so on. All such negative test cases can be validated using a single assertion that expects exception.
Testee is the object that we need to test against the assumptions. We get it from an API response or after processing the code under test.
Trying to change the testee to suit one’s needs is one of the worst things to notice. What happens when code refactoring on a testee happens? The test case will blow up.
Do not use this or allow it to be used either using immutable objects as much as possible.
I got my fair share of such poorly written tests. They silently swallow the exception in a little catch block at the end of test case. And worse is that they do not raise a failed alarm also.
Exceptions are signals that your application throws to communicate that something bad has happened and you must investigate it. You should not allow the test cases to ignore these signals.
Whenever you see an unexpected exception, fail the test case without a miss.
Personally, I also do not like test cases that require a number of things to happen before they start testing the actual thing. Such a scenario can be like an application that facilitates online meetings. Now to test whether a user of a particular type, can join the meeting, below is the steps the test is performing:
- Create the User
- Set user permissions
- Create the meeting
- Set meeting properties
- Publish meeting joining information
- [Test] User joins the meeting
Now in the above scenario, there are 5 complex steps that must be set up before actual logic is verified. This is not a good test case.
- A good unit test will take advantage of an in-memory database that is populated with initial data for tests.
- Another way is to use mock objects. They are there for this very purpose. Isn’t it??
This is not widely seen but is sometimes visible when written by freshers. They use system-dependent file paths, environment variables or properties instead of using common properties/paths, or things like this.
Watch out for these tests using hardcoded system paths.
These log generator tests do not seem to create problems on happy days. But when rainy days come, they make life hell by putting unnecessary text without any information in log files and make life hell for the debugger who is trying to find something hidden in those log files.
Tests are not for debugging the application, so do not put debug logs kind of statements. A single log statement for Pass/ Fail is enough. If something fails, it should be evident by just looking into test name.
These are my thoughts based on my learning in the last few years. I do not expect you to agree with me on all the above points. But might have some other opinion which is perfectly cool. But I will surely like to discuss, what you feel about the topic.
Happy Learning !!