14 ms·
Yes, you should delete tests for everything that isn't a required external behavior, or a bugfix IMO. Otherwise you're implicitly testing the implementation, w
by VHRanger 6y ago
Yes, you should delete tests for everything that isn't a required external behavior, or a bugfix IMO.
Otherwise you're implicitly testing the implementation, which makes refactoring impossible.
A big smell here is if the large majority of your tests are mocked. This might mean you're testing at too fine-grained a level.
- mumblemumble 6y agoAgreed. I would take an even stronger position, and say that a high degree of mocking actually implies two things: First, yes, you're testing at too fine-grained a level. Second, it's a code smell that suggests you may be working with a fundamentally untestable design that relies overmuch on opaque, stateful behavior.
- 411111111111111 6y agoMocks are worthwhile though. Otherwise you end up not being able to unittest anything which accesses an external api such as databases, rest services etc.
- deleted 6y ago[deleted]
- Joker_vD 6y agoYou mean fakes/stubs, right? Unless you're testing whether you're correctly implementing the protocol exchange with an external party, you don't need to record the API calls.
- mumblemumble 6y agoIt really depends on your definition of unit. In the London school of TDD, no, a unit cannot extend across an I/O boundary. The classicist school takes a more flexible, pragmatic approach.
- the_other 6y agoI'd love to know the story behind "the London school".
- insertnickname 6y agoIt's all in this book: http://www.growing-object-oriented-software.com/ http://www.growing-object-oriented-software.com/
- the_other 6y agoIntriguing. Thanks!
- matkoniecz 6y agoI have plenty of tests actually calling database. Maybe it is not a proper unit test, but it is not a problem for me. I want test to protect me against regressions, and I do not really care how they are classified.
- mannykannot 6y agoHow do you test your mocks?
- jacobsenscott 6y agoHow do you test the tests that are testing your mocks? That said verifying mocks are a great help - they won't let you mock methods that don't exist on the real object. Some mocking libraries, like the VCR library in ruby, can be turned off every now and then so you tests hit real endpoints. It is worth doing from time to time.
- dagss 6y agoIMO, databases is often an integral part of the program and should be part of the test (a real database in a docker image). For instance, if you are not relying on unique constraint in the DB to implement idempotency you are probably doing something wrong, and if you are not testing idempotent behaviour you are probably doing something wrong.
- crazygringo 6y ago> you should delete tests for everything that isn't a required external behavior Wait, I'm terribly confused here. Aren't a huge part of tests to prevent regression? In attempting to fix a bug, that could cause another "internal" test to fail and expose a flaw in your bugfix that you wouldn't have caught otherwise. And it's not uncommon for your flawed bugfix to not cause an "external" test to fail, because it's related to the codepath there was never a good enough external test for in the first place -- hence why the bug existed. I can't imagine why you would ever delete tests prematurely. I mean, running internal tests is cheap. I see zero benefit for real cost. And not only that, when devs don't document the internal operation of a module sufficiently, keeping the tests around serves as at least a kind of minimal reference of how things work internally, to help with a future dev trying to figure it out. If you're refactoring an implementation, then obviously at that point you'll delete the tests that no longer apply, and replace them with new ones to test your refactored code. But why would you delete tests prematurely? What's the benefit?
- a1369209993 6y agoAll legitimate tests are[0] regression tests. TDD, to the extent that it's actually useful, is the notion that sometimes the bug being regression-tested is a feature request. Edit: 0: I guess "can be viewed as" if you want to be pedantic.
- nkingsy 6y agoIf it can cause a regression, it's not internal. My rule of thumb is "test for regression directly", meaning a good test is one that only breaks if there's a real regression. I should only ever be changing my unit tests if the expected behavior of the unit changes, and in proportion to those changes.
- the_af 6y ago> If it can cause a regression, it's not internal Fair enough. And how do you know, before causing a regression, whether your test could detect one? In other words, how can you tell beforehand whether your test checks something internal or external?
- tonyedgecombe 6y agoSo today I have been writing a lexer and parser. The public interface is the parser, the lexer isn't exposed. The problem is if I delete all the tests for the lexer then any bugs in the lexer will only get exposed through the parser's tests. This makes no sense to me.
- VHRanger 6y agoThe lexer is a unit then. The lexer has a clear boundary from the parser. The issue that takes experience here is how to determine what's a unit. "The whole program" is obviously too big. "every public method or function" is obviously too small. Just be pragmatic.
- AnimalMuppet 6y ago> "The whole program" is obviously too big. Of course. > "every public method or function" is obviously too small. Why "obviously"? If it's public, someone outside the class can call it. That's an external behavior.
- deleted 6y ago[deleted]
- VHRanger 6y agoIf the class is only consumed in the context of one code unit (module, service, whatever) then the class itself is an implementation detail.
- ajna91 6y ago"every" being the operative word. The "feel" for good code that comes with experience is not reducible in practice to a set of black-and-white rules.
- Chabsff 6y agoIdeally, the lexer should be a system in of itself, exposing a public interface that is consumed by its client, the parser. Public doesn't necessarily mean "not you".
- war1025 6y ago> A big smell here is if the large majority of your tests are mocked. We fell into this hard for a few years at my work. Going back to any of that code is a nightmare because the test suites are so fragile. Testing behavior and setting up as much of the system as possible leads to much better results in my experience.
- hinkley 6y agoBertrand Meyer had the right of it, but I had to figure this out myself before I ever saw him quoted on the subject. Me: Code that makes decisions has branches. Branches require combinatoric tests. Code with external actions requires mocks. Therefore: Code that makes decisions and calls external systems requires combinatorics for mocks. Bertrand, more (too?) concisely: Separate code that makes decisions from code that acts on them. Follow this pattern to its logical conclusions, and most of you mocks become fixtures instead. You are passing in a blob of text as an argument instead of mocking the code that reads it from the file system. You are looking at a request body instead of mocking the PUT function in the HTTP library. The tests of external systems are much fewer, and tend to be testing the plumbing and transportation of data. If I give you a response body do you actually propagate it to the http library? And even here, spies and stubs are simpler than full mocks.
- bmh100 6y agoI used this strategy when developing a client library for a web socket API. It was hugely helpful. I could just include the string of the response in my tests, instead of needing a live server or even a mock server for testing. Tests were much simpler to write and faster to execute.
- danenania 6y agoThis is great until the API response changes and you have to painstakingly update all your string fixtures to match.
- 6y ago
- hinkley 6y agoI’ve watched this play out a few times with different teams and different code bases (eg, one team two projects). Part of the reason existing tests lock in behavior and prevent rework/new features is that the tests are too complicated. Complicated tests were expensive to write. Expense leads to sunk cost fallacy. I’ve watched a bunch of people pair up for a day and a half trying to rescue a bunch of big ugly tests that they could have rewritten solo and in hours if they understood them, learn nothing, and do the same thing a month later. The same people had no problem deleting simple tests and replacing them with new ones when the requirements changed. Conclusions: - the long term consequences of ignoring the advice of writing tests with one action and one assertion are outsized and underreported. - change your code so the don’t need elaborate mocks - choose a test framework that supports setup methods - choose a framework that supports custom/third party assertions, sometimes called matchers. You won’t use this often, but when you do, you really do.
- _pius 6y agoYes, you should delete tests for everything that isn't a required external behavior, or a bugfix IMO. For the edification of junior programmers who may end up reading this thread, I’m just going to come right out and say it: this is awful advice in general. For situations where this appears to be good advice, it’s almost certainly indicative of poor testing infrastructure or poorly written tests. For instance, consider the following context from the parent comment: Otherwise you're implicitly testing the implementation, which makes refactoring impossible. A big smell here is if the large majority of your tests are mocked. This might mean you're testing at too fine-grained a level. These two points are in conflict and help clarify why someone might just give up and delete their tests. The argument for deleting tests appears to be that changing a unit’s implementation will cause you to have to rewrite a bunch of old unrelated tests anyway, making refactoring “impossible.” But indeed that’s (almost) the whole point of mocking! Mocking is one tool used for writing tests that do not vary with unrelated implementations and thus pose no problem when it comes time to refactor. Now there is a kernel of truth about an inordinate amount of mocking being a code smell, but it’s not about unit tests that are too fine-grained but rather unit tests that aren’t fine-grained enough (trying to test across units) or just a badly designed API. I usually find that if testing my code is annoying, I should revisit how I’ve designed it. Testing is a surprisingly subtle topic and it takes some time to develop good taste and intuition about how much mocking/stubbing is natural and how much is actually a code smell. In conclusion, as je42 said below: Make sure you tests run (very) fast and are stable. Then there is little cost to pay to keep them around. The key, of course, is learning how to do that. :)
- ptx 6y ago> Mocking is one tool used for writing tests that do not vary with unrelated implementations What if I chose the wrong abstractions (coupling things that shouldn't be coupled and splitting things in the wrong places) and have to refactor the implementation to use different interfaces and different parts? All the tests will be testing the old parts using the old interfaces and will all break.
- dragonwriter 6y agoYou seem to think the rationale is testing performance; but from GP it seems that the rationale is avoiding the tests ossifying implementation details against refactoring rather than protecting external behavior to support refactoring.
- mumblemumble 6y agoI would also test implementation details that are legitimately complicated and might fail in subtle ways, or where the intended behavior isn't obvious. If I've implemented my own B+ tree, for example, you better bet your butt I'll be keeping some property tests to document and verify that it conforms to all the necessary invariants.
- shados 6y ago> Otherwise you're implicitly testing the implementation, which makes refactoring impossible. Red green refactoring isn't, and shouldn't, be a goal of unit testing. Integration and E2E tests provide that. Unit tests are mostly about making sure the individual pieces works as you author them, as well as implicitly documenting the intent of those individual pieces. If done properly, they're always quick/easy/cheap to author, and thus are throwaway. When you refactor significantly (more than the unit), you just throw them away and write new ones (at which point their only goal is for you to understand the intent of the code you were shuffling around, and making sure you're breaking what you expected to break). Delete, rewrite. People are resistant to getting rid of unit tests when they did complex integration tests that took forever to write instead. So the tests feel like they were wasted effort. Those tests are totally valuable, in this case for things such as red green refactoring, but then yes, you have to carefully pick and choose what you're testing to avoid churn.