Testing only through public methods is bad.
In programming and in TDD, in particular, there are good principles that are useful to follow: DRY and testing through public methods. They have repeatedly justified themselves in practice, but in projects with large legacy-code they can have a “dark side”. For example, you can write code, guided by these principles, and then find yourself dismantling tests, covering a bunch of 20+ abstractions with a configuration that is incommensurably larger than the logic being tested. This “dark side” scares people and inhibits the use of TDD in projects. Under the cat, I argue why testing through public methods is bad and how you can reduce the problems arising from this principle.
The idea underlying the principle sounds good: you need to test behavior, not implementation. So, you only need to test the class interface. In practice, this is not always the case. To present the essence of the problem, imagine that you have a method that calculates the cost of workers employed in shift work. This is not a trivial task when it comes to shift work, because they have tips, bonuses, weekends, holidays, corporate rules, etc., etc. This method inside performs a lot of operations and uses other services that give it information about holidays, tips and so on. when writing a unit test for it, it is necessary to create a configuration for all used services, if the code being tested is somewhere at the end of the method. At the same time, the code itself can only be partially used, or do not use configurable services at all. And there are already some unit tests written in this way.
Now you want to add a reaction to a new feature that has a non-trivial logic and is also used somewhere at the end of the method. The nature of the flag is such that it is part of the service logic and, at the same time, is not part of the service interface. In the above case, this code is relevant only for this public method, and can be, generally, inscribed inside the old method.
If the rule is to test everything in a project only through public methods, then the developer can, most likely, simply copy some existing unit test and tweak it a little. In the new test, there will still be a configuration of all services for running the method. On the one hand, we followed the principle, but, on the other hand, we got a unit test with overconfiguration. In the future, if something breaks, or requires a configuration change, you will have to do the monkey work on correcting the tests. It is tedious, long and does not bring either joy or any visible benefit to the client. It would seem that we are following the correct principle, but we find ourselves in the same situation that we wanted to leave, refusing to test private methods.
Further such human factor as laziness can interfere. For example, a private method with nontrivial flag logic might look like this example.
This method requires 10 checks to cover all cases, 8 of them significant.
When writing unit tests to test this logic, the developer will have to write at least 8 large unit tests. I came across cases when the unit test configuration took 50+ lines of code, with 4 lines of direct call. Those. only about 10% of the code carries the payload. In this case, the temptation is to reduce the amount of work by writing fewer unit tests. As a result, only 8 units of 8 remain, for example, for each clockIn value. This situation leads to the fact that either, again, it is tedious and long you need to write all the necessary tests, creating a configuration (Ctrl + C, V works, where without it), or the method remains only partially covered. Each option has its unpleasant consequences.
In addition to the principle of "testing behavior", there is still OCP (Open / closed principle). By applying it correctly, you can forget what “fragile tests” are by testing the internal behavior of a module. If you need a new module behavior, you will write new unit tests for the new heir class, in which the behavior you need will be changed. Then you will not need to spend time on re-checking and updating existing tests. In this case, this method can be declared as internal, or protected internal, and tested by adding InternalsVisibleTo to the assembly. In this case, your IClass interface will not suffer, and the tests will be the most local, not subject to frequent changes.
Another alternative would be to declare an additional helper class, into which our method can be pulled out, declaring it as public. Then the principle will be observed, and the test will be concise. In my opinion, this approach does not always justify itself. For example, some may decide to pull out even one method into one class, which leads to the creation of a heap of classes with one method. At the other extreme, dumping such methods into a single helper class, which turns into a GOD-helper-class. But this option with the helper may be the only one if the working assembly is signed with a strong name, and you cannot sign the test assembly, for some reason. InternalsVisibleTo will work when both assemblies are either signed or not at once.
And in the end, because of the combination of such problems, the idea of TDD and unit tests suffers, since Nobody has a desire to write volume tests and support them. I will be glad to any examples of how strict adherence to this principle led to problems and a decrease in the motivation to write tests for the development team.
Denial of responsibility
Just want to dispel a possible impression. Some may not even feel the drawbacks that will be discussed, by virtue of, for example, the size of their projects. Also, these shortcomings, in my opinion, are part of the technical debt and have the same characteristics: the problem will grow if it is not paid attention. Therefore, it is necessary to decide on the situation.
The idea underlying the principle sounds good: you need to test behavior, not implementation. So, you only need to test the class interface. In practice, this is not always the case. To present the essence of the problem, imagine that you have a method that calculates the cost of workers employed in shift work. This is not a trivial task when it comes to shift work, because they have tips, bonuses, weekends, holidays, corporate rules, etc., etc. This method inside performs a lot of operations and uses other services that give it information about holidays, tips and so on. when writing a unit test for it, it is necessary to create a configuration for all used services, if the code being tested is somewhere at the end of the method. At the same time, the code itself can only be partially used, or do not use configurable services at all. And there are already some unit tests written in this way.
Minus 1: Overconfiguration of unit tests
Now you want to add a reaction to a new feature that has a non-trivial logic and is also used somewhere at the end of the method. The nature of the flag is such that it is part of the service logic and, at the same time, is not part of the service interface. In the above case, this code is relevant only for this public method, and can be, generally, inscribed inside the old method.
If the rule is to test everything in a project only through public methods, then the developer can, most likely, simply copy some existing unit test and tweak it a little. In the new test, there will still be a configuration of all services for running the method. On the one hand, we followed the principle, but, on the other hand, we got a unit test with overconfiguration. In the future, if something breaks, or requires a configuration change, you will have to do the monkey work on correcting the tests. It is tedious, long and does not bring either joy or any visible benefit to the client. It would seem that we are following the correct principle, but we find ourselves in the same situation that we wanted to leave, refusing to test private methods.
Minus 2: Incomplete Coverage
Further such human factor as laziness can interfere. For example, a private method with nontrivial flag logic might look like this example.
privateboolHasShifts(DateTime date, int tolerance, bool clockIn, Shift[] shifts, int[] locationIds)
{
boolisInLimit(DateTime date1, DateTime date2, int limit)
=> Math.Abs(date2.Subtract(date1).TotalMinutes) <= limit;
var shiftsOfLocations = shifts.Where(x => locationIds.Contains(x.LocationId));
return clockIn
? shiftsOfLocations.Any(x => isInLimit(date, x.StartDate, tolerance))
: shiftsOfLocations.Any(x => isInLimit(date, x.EndDate, tolerance));
}
This method requires 10 checks to cover all cases, 8 of them significant.
Decoding 8 important case studies
Total: 2 x 2 x 2 = 8
- shiftsOfLocations - 2 values - whether or not
- clockIn - 2 values - true or false
- tolerance - 2 different values
Total: 2 x 2 x 2 = 8
When writing unit tests to test this logic, the developer will have to write at least 8 large unit tests. I came across cases when the unit test configuration took 50+ lines of code, with 4 lines of direct call. Those. only about 10% of the code carries the payload. In this case, the temptation is to reduce the amount of work by writing fewer unit tests. As a result, only 8 units of 8 remain, for example, for each clockIn value. This situation leads to the fact that either, again, it is tedious and long you need to write all the necessary tests, creating a configuration (Ctrl + C, V works, where without it), or the method remains only partially covered. Each option has its unpleasant consequences.
Possible solutions
In addition to the principle of "testing behavior", there is still OCP (Open / closed principle). By applying it correctly, you can forget what “fragile tests” are by testing the internal behavior of a module. If you need a new module behavior, you will write new unit tests for the new heir class, in which the behavior you need will be changed. Then you will not need to spend time on re-checking and updating existing tests. In this case, this method can be declared as internal, or protected internal, and tested by adding InternalsVisibleTo to the assembly. In this case, your IClass interface will not suffer, and the tests will be the most local, not subject to frequent changes.
Another alternative would be to declare an additional helper class, into which our method can be pulled out, declaring it as public. Then the principle will be observed, and the test will be concise. In my opinion, this approach does not always justify itself. For example, some may decide to pull out even one method into one class, which leads to the creation of a heap of classes with one method. At the other extreme, dumping such methods into a single helper class, which turns into a GOD-helper-class. But this option with the helper may be the only one if the working assembly is signed with a strong name, and you cannot sign the test assembly, for some reason. InternalsVisibleTo will work when both assemblies are either signed or not at once.
Total
And in the end, because of the combination of such problems, the idea of TDD and unit tests suffers, since Nobody has a desire to write volume tests and support them. I will be glad to any examples of how strict adherence to this principle led to problems and a decrease in the motivation to write tests for the development team.