When preparing our test suite we might be tempted to make some shortcuts that will render our tests less readable, comprehensible or will significantly decrease the flexibility of further implementation. Below, I try to list some of the most commonly seen techniques we should avoid to keep our test suite healthy. Please be advised, that examples provided are usually oversimplifications helpful to highlight the discussed issues.
Testing private methods
The bad thing about testing private methods is that it should not actually happen as naturally, we should be testing public interface instead. This may indicate, that either tests were written after implementation (what is an antipattern itself) or public interface was changed at some point into private interface. The consequence of testing private methods is code resistant to refactoring, as implementation become tightly bound to correlated tests.
To counter testing private methods directly, we should always test public interface that uses them (so private methods are tested indirectly). The use of a test coverage tool (e.g. simplecov) might become handy in the detection of private methods that are not tested (and therefore possibly never used). Also behavior-driven development (BDD) is a technique that naturally encourages testing public interface only.
Stubbing framework methods
Stubbing framework methods is a great way to leave yourself as little choices during implementation as possible which is not a positive thing. For sake of simplicity, consider the following snippet:
allow(User).to receive(:where).with(name: “Tony”) {
[instance_double(User, name: “Tony”)]
}By stubbing framework (in this case ActiveRecord query interface), we are forcing ourselves to exactly one way of implementing a requirement, with little place for any refactoring.
Instead of stubbing a framework features, just use factories (or other techniques) to provide framework with data it needs. In the case shown above, simple create(:user, name: “Tony”) would be sufficient.
Stubbing library methods
Stubbing library methods is very similar to stubbing framework methods, but it brings other kind of problems to the table. Libraries evolve much faster than frameworks and are prone to frequent changes. Also, it is not uncommon to change one library into another one due to various reasons. If we depend on stubbing public interface of a library across the system, we might need to update our tests in many places every time our library evolves or we decide to change it into a different one.
In order to mitigate this problem, it is a good idea to provide a wrapper (facade) around the given library and then stub only interface introduced by this library. Then, it might be right to stub library methods within tests for the wrapper but usually, it is even better to mock something the actual library depends on (i.e. HTTP traffic).
Low value tests
Some tests do not bring any significant value and only focus on implementation details. Such tests should be removed in favor of the details being tested indirectly by means of other tests. Testing attributes presence, simple migrations or relationships are good examples of specs that really bring little value to the table and just bloat our tests suite.
Tests oriented implementation
By tests-oriented implementation we mean pieces of code that exist solely to make specs pass, but do not bring any functionalities or business logic. Anytime we see if Rails.env.test? wherever in our codebase, it should raise our suspicions. A code that conditionally runs only in test environment not only increases perceived complexity, but also might make our tests unreliable. Therefore, we should only allow such conditionals in tests implementation and code related to project configuration and initialisation.
Test induced design damage
A bit similar problem is test induced design damage. To cut the long story short, this is making code/architecture less readable and comprehensible as a result of making it highly testable in isolation, easier to test-first or making the test run faster. There is a great article by DHH shedding more light on the topic.
