5 ms·
Seven years into my career I'm increasingly convinced that the emperor has no clothes with respect to unit tests that are just transcripts of the code under tes
by closeparen 4y ago
Seven years into my career I'm increasingly convinced that the emperor has no clothes with respect to unit tests that are just transcripts of the code under test with "mock.EXPECT()" prepended to everything - 95% by volume of the unit test code I've ever read, written, or maintained.
- bsuvc 4y agoYeah, 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 agoThe 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 agoThat’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.
- sanderjd 4y agoYep, I think mocks are mostly a smell. The "functional core" of a module should be entirely or almost entirely unit testable with (possibly fake) dependencies passed in. The glue code ("imperative shell") should be tested at a higher level - "integration" or "end to end" or whatever you want to call it - which looks at the externally observable effects of running the code (database changes or API responses or whatever) rather than the details of its execution.
- deleted 4y ago[deleted]
- closeparen 4y agoThis is the way, but it’s tough to sell “we’re not going to unit test this part” in a professional setting where there are incentives to look more responsible than thou, directors are looking at unit test coverage reports, etc.
- sanderjd 4y agoYeah I have no qualms about writing mostly-BS unit tests when the existing structure of a project makes it infeasible to write good ones. When in Rome! But when I'm starting from scratch or have a chance to do refactoring, I move toward the "functional core" approach as much as possible.
- therealdrag0 4y agoYou can still get code coverage points with broader tests. I think it’s reasonable to disavow expecting single unit tests for nearly every method on every class with mock/boilerplate/copy-paste hell. However, I would still expect “local integration” unit tests that exercise broader chunks of code and keep code coverage up. So it should be “I don’t need to write a new unit test for this because an existing unit test calls the method that calls this method without it being mocked and you can see it’s covered in the code coverage.”
- closeparen 4y ago
- grogenaut 4y agoI call those lockdown tests and they're a smell. You don't want to dictate the implementation, just the inputs and outputs. The former leads to very brittle code, the latter describes and validates the contract. It's also important where you put the logic of the test in the code base when you have multiple layers. This latter part is harder and system dependent. In many cases mocks are now over used where previously they were important in say 2008. Especially now with languages that support functions as objects, better generics, and other features which weren't common a while back. Likewise frameworks are and languages are generally way more testable now which means you're doing less backflips like static injecting wrappers for DateTime.now into libraries to make tests work. This further allows more contract and less implementation specific testing. As with most things there is a lot of nuance/art to doing it well and smoothly
- codemonkey-zeta 4y ago> You don't want to dictate the implementation, just the inputs and outputs When programmers really embrace dependency injection like the parent comment, the implementation largely is the inputs, and your code expects them all to be implemented correctly (or mocked correctly by test code). I completely agree that mocks are overused, and their importance in 2008 were in my view a direct result of the popularity of dependency injection patterns. Following that logic, the best way to remove mocks from your tests is to minimize dependency injection, relegating it all to a single place in the code wherever possible, and implementing all other domain logic as pure functions of input to output. How to test that code which touches other systems? Integration tests (or multi-system tests or whatever you like to call them).
- grogenaut 4y agoI generally agree with you, DI went really far in many ways through up to like 2015. In 2008, I actually found code often got more testatable when it was dependency injected. I spent a month ripping singletons out of a gui application so we could make it into a web service, that would have been a lot easier with a DI model. The system was nigh-on untestable until we got rid of those singletons. I link it in another comment but https://github.com/gaffo/CSharpHotChocolateZeroQLIntegrationTestExample https://github.com/gaffo/CSharpHotChocolateZeroQLIntegration... is how I do things these days, api integration test, and more focused unit tests where you don't understand the library well yet, there's lots of tricky edge cases, or error conditions such as setup that are harder to integration test. I'm still feeling it out.
- Izkata 4y agoI think my favorite of these was a function that called 4 other functions and didn't do anything else, its unit test mocked out all 4 other functions and asserted that each was called.
- closeparen 4y agoEvery damn day. It’s the thing that puts me most at risk of falling out of love with software engineering.
- the_only_law 4y agoI had a take home test a year or two ago and I got grilled during the interview for not writing crap like that.
- tra3 4y agoHow would you unit test that?
- Izkata 4y agoThis was in django, where a good such test would set up the database, call the function, then check that the new state in the database is as expected. (Which IIRC is what I changed it to)
- tra3 4y agoI've seen tests like that ("has the mock been called")... and in general I've felt that they were kinda useless. Thinking about the original TDD approach -- red, green, repeat maybe it's not that bad? I prefer your approach, where you actually verify the state change. But in the absence of that, I wouldn't mind the "call the mock" approach. At least it does change some of the contract?
- Izkata 4y agoI would say it's worse than no test, because it discourages refactoring. Any alterations would make the test fail, even if it still does what it's supposed to do.