3 ms·

Yeah, the mere presence of unit tests is not enough. It has to actually assert something useful. When I code review, I try to make sure I call out "fake tests"

by bsuvc 4y ago

Yeah, the mere presence of unit tests is not enough. It has to actually assert something useful. When I code review, I try to make sure I call out "fake tests".

closeparen 4y ago
The useful assertion to be made about gateway/repository layer code is that it gets the expected behavior out of the dependency. This is not an assertion you can make when you've mocked out the dependency. You must make it in an integration test, not a unit test. Unit tests in these layers just make assertions about "it calls this method on the client" or "it sends this string," which tells you nothing about whether doing that is actually correct. It's relatively uncommon for handler/controller code to have logic worth testing, most of the time it's just maintaining the separation of layers and concerns by wrapping gateway/repository calls. All there is to assert about it is that "it calls this function in the next layer." Every once in a while there's nontrivial functionality to test in the middle, and unit tests can often be a good fit for that, but in my experience it's more the exception than the rule.
lscharen 4y ago
That’s a great point. Seeing lots of mocks and assertions that certain functions are called is often much ado about nothing since no actual code functionality is exercised. I do sometimes see the return value functionality of mocks used as a stub, just because the dev hasn’t internalized the distinction and can “make it work” with a mocking library. One of the only legit use cases for mocks that I have personally come across is validating things like a sequence of API calls to an external service, or queries to a database where there is a check that certain efficiencies are guaranteed, e.g. verify that an N+1 select problem doesn’t creep in, or knowing that a app-level caching layer will prevent redundant API calls.

by bsuvc 4y ago

Yeah, the mere presence of unit tests is not enough. It has to actually assert something useful. When I code review, I try to make sure I call out "fake tests".

closeparen 4y ago

The useful assertion to be made about gateway/repository layer code is that it gets the expected behavior out of the dependency. This is not an assertion you can make when you've mocked out the dependency. You must make it in an integration test, not a unit test. Unit tests in these layers just make assertions about "it calls this method on the client" or "it sends this string," which tells you nothing about whether doing that is actually correct. It's relatively uncommon for handler/controller code to have logic worth testing, most of the time it's just maintaining the separation of layers and concerns by wrapping gateway/repository calls. All there is to assert about it is that "it calls this function in the next layer." Every once in a while there's nontrivial functionality to test in the middle, and unit tests can often be a good fit for that, but in my experience it's more the exception than the rule.

lscharen 4y ago

That’s a great point. Seeing lots of mocks and assertions that certain functions are called is often much ado about nothing since no actual code functionality is exercised. I do sometimes see the return value functionality of mocks used as a stub, just because the dev hasn’t internalized the distinction and can “make it work” with a mocking library. One of the only legit use cases for mocks that I have personally come across is validating things like a sequence of API calls to an external service, or queries to a database where there is a check that certain efficiencies are guaranteed, e.g. verify that an N+1 select problem doesn’t creep in, or knowing that a app-level caching layer will prevent redundant API calls.