How to write untestable code (2008)
testing.googleblog.com
testing.googleblog.com
This is meant sarcastically, but I think this is actually one example of where a lot of backlash against OO comes from, especially in 2022.
Code coverage is like those skin creams that say "Scientifically tested!" on the label. Yeah, well, tested doesn't mean that it worked. It just means that there was a test.
Watch this video of Andrei Alexandrescu's talk in CppCon 2015: https://www.youtube.com/watch?v=LIb3L4vKZ7U
I've done things like this in my career, and it's always worked out brilliantly. Bugs just evaporate when you let the compiler "combine" things for you, instead of just plopping everything into a bowl and mashing it together with your hands like some sort of animal.
Especially in today's C++ or Rust, you can "have your cake and eat it too". It's possible to build up these nice, single-purpose class hierarchies and combine them into complex, powerful constructs via templates / traits. The compiler takes care of the edge cases for you, because it never gets bored or distracted. There's no performance downside because of static dispatch, inlining, and aggressive optimisers.
OOP is not the answer here. Class hierarchies can be, as long as they are not OOP and "classes" are just synonym for "modules".
Code and data colocation has its uses, but it's normally best fit for interface layers than internal code.
This seems nuts to me? Sacrificing guarantees for the sake of testability? I mean defensive coding is one of good tools to fail early and with meaningful error messages and confidence on state within your method.
Moreover there are tons of points just to enable mocks'n'stuff. Why not just go higher up the abstraction layer and test things functionally there? It's more useful IMHO than some state object that may never be true in real life.
It reads like chasing some code coverage metric with fake state. When in reality, you may still get nullreferenceexception because your assumption on false state was incorrect. Or some method call there may fail with unhanled error code or whatever. It doesn't take branches for fully covered code to fail.
IMHO unittests are useful, but in limited scope.
This primarily applies to internal code. For external APIs there is no such thing as too much validation.
I've started more of defensive coding when I've read that for a software I forgot how it called - there was security audit which find no important security flaws. Because their ideology was to not trust the input, even if its your input. Today your assumptions are correct, tomorrow, because of changes, they can be incorrect.
I'll post link here if I find it. I think I read it on HN few years ago.
Edit: A-ha, here it is. Dovecot. HN Article: https://news.ycombinator.com/item?id=13407233 and https://wiki.mozilla.org/MOSS/Secure_Open_Source/Completed#d...
> The Cure53 team were extremely impressed with the quality of the dovecot code. They wrote: "Despite much effort and thoroughly all-encompassing approach, the Cure53 testers only managed to assert the excellent security-standing of Dovecot. More specifically, only three minor security issues have been found in the codebase, thus translating to an exceptionally good outcome for Dovecot, and a true testament to the fact that keeping security promises is at the core of the Dovecot development and operations."
Practices documented in source: https://github.com/dovecot/core/blob/master/doc/securecoding...
> Don't rely on input validation. Maybe you missed something. Maybe someone calls your function somewhere else where you didn't originally intend it. Maybe someone makes the input validation less restrictive for some reason. Point is, it's not an excuse to cause a security hole just because input wasn't what you expected it to be.
Perhaps it's just my way of dealing with "I know that input is validated". Yeah, defensive coding can get verbose, but makes me feel confident. + The NASA Coding Guidelines of states:
> Each calling function must check the return value of nonvoid functions, and each called function must check the validity of all parameters provided by the caller.
> The code's assertion density should average to minimally two assertions per function.
/just extracting useful bits of advices from veterans
I think the problem boils down to a violation of separation of concerns. Defensive code inevitably concerns itself with ensuring guarantees about state that is the concern of other modules.
Silently, trying to work with invalid inputs just results in bugs that people may not notice and are much harder to track down.
Not that your example cannot be achieved in C# - it can. But what I suspect F# just brings those static typing guarantees further https://fsharpforfunandprofit.com/posts/designing-for-correc...
Essentially what you’re describing is automating such checks.
If you want to harden your stuff against that category of errors, then the solution exists with in hardware. RF-shielding and ECC memory for starters.
That said, I agree you can’t guarantee everything is working via software but detecting hardware failure via software is simply good practice.
Eg, if you checksum all the data, what you can expect from bad RAM is either to detect a mismatch, or to mis-calculate the checksum and something else will detect a mismatch later.
You certainly can't count on it, but enough of that sort of thing makes it a lot more likely you'll notice something is not quite right eventually.
It's exactly the same reason bindly following "textbook" OOP design is a bad idea. Unless you're able to do some very prescient upfront design of a large part of your application, you're in for a big headache later.
What does this mean? I have not figured out any way to interpret this, such that it is a consequence of choosing strong typing.
Your second paragraph is more of the straw man that we are discussing in an adjacent thread.
If you add say a thousand types for all the things combinations of things you may want to represent, then you've effectively added the need to have a thousand type names.
This is something of a straw man argument. If someone is doing this, they are simply doing it wrong. It is certainly possible to write validations that only check for constraints that must hold within the code they are gatekeeping for.
That is, this pattern
F(x, y, Z) {
if (x == null) bad();
if (y == null) bad();
if (Z == null) bad();
Z.doThing(x, y);
}
This is a style of programming I've seen in the field, and it's seemed to emerge within code bases with major data quality problems, but done little to actually fix the problem. You're less likely to get disastrous train wrecks, but the number of errors largely remains the same unless the cause is addressed.
It is a straw man to take this as being intended to justify incorrect and useless validation.
If this is something to be decided by personal anecdote, I will just add mine: I haven't seen this, while I have seen many cases where validation has picked up subtle and edge-case errors.
Look, this is the paragraph we are discussing:
> Be Defensive - They're out to Get Your Code! - Defensively assert about the state of parameters passed in methods, constructors, and mid-method. If someone can pass in a null, you've left your guard down. You see, there are testing freaks out there that like to instantiate your object, or call a method under test and pass in nulls! Be aggressive in preventing this: rule your code with an iron fist! (And remember: it's not paranoia if they really are out to get you.)
In which way am I misrepresenting it?
As for your anecdotal evidence, it does not count twice through repetition.
This may not be the flavor of defensive programming you favor, but it is something that exists, and also something that is explicitly mentioned in the start of the discussion.
>...something that is explicitly mentioned in the start of the discussion.
Yes - it started with a straw man in the article.
TDD zealots tend to not care about efficiency, maintainability, readability, or any of a number of very important characteristics of a program if it interferes with the kind of false-security testability they are zealots about.
These types should be clearly meant to be used ONLY in testing environments.
Well, yes, if those classes are implementation details why would you want otherwise?
Just to write “unit” tests with mocks all over the place that later will be broken after the first refactor?
On the former, using concrete classes instead of interfaces can be worked around if they're still configurable, but now you have inappropriate (or potentially inappropriate) subclasses to compensate for the lack of an interface. On the latter, well, it makes testing harder which is the entire point of the list. If A has two components B and C which are both tied to concrete classes and not interfaces, then you have to bring A, B, and C into the test harness when you really just want A and (possibly) B or C or stubs/mocks for them.
If you don't care about testing (and, in particular, unit testing), and don't care about substitutability, then don't worry about it.
It’s not very useful to me if I get red tests just because the structure of my code changed. Those red tests don’t tell me if the functionality is the same or not. If my tests break every time even if the functionality is the same, then I start to ignore the warning, I just blindly make them pass. They become “The Boy Who Cried Wolf”.
More specifically, I follow the maxim of only using in the tests what is publicly available to a user of the class. If it is a private instance of a class, or a class created in a function that does not have a mechanism to be switched for something else, you shouldn't be poking the internals to modify it, you should either:
1. provide a mechanism to change that class -- e.g. if the class under test is talking to a database;
2. not change the code -- e.g. if the class under test is using a particular data structure class.
You should not have/use mocks if you have a class A that holds and uses class B to perform its work. If you do, you are not properly testing class A's behaviour. That is, if you have a rotation helper class that uses a quaternion class, you shouldn't be mocking the quaternion class behaviour -- that way only leads to madness, expecially if the internal implementation of the rotation class changes its internal representation/logic to use matrices.
Acceptable places to use mocks or similar techniques are:
1. for the class that wraps the database object, or some other external system -- for which, I don't class the filesystem to be external;
2. for setting up parts of an application (e..g IntelliJ) when testing a plugin -- and there, I try to get as close to the application behaviour as possible/necessary to perform the tests (i.e. wherever possible, use the application's real implementation class).
3. for setting up things like Spring, or other dependency injection components.
No, I'm mostly with you here. Closure Library is OK if you don't have anything else that replaces it (or if something you use ships with it, hello ClojureScript) but other than that, their libraries leave a lot to offer. But Google Closure Compiler is magic (in a good way) though, but you can't call that a library.
> Every library I've greatly enjoyed has come out of Facebook, never Google.
I'm sure I'm reading this the wrong way, because surely you can't mean that every library you've greatly enjoyed has only came from Facebook? Most of the really well made libraries I've used in my time, has come from private people, besides React. But now React has better replacements available as well, like Preact. But other than that, larger companies seem to be really bad at writing libraries for the world at large, which makes sense, they mostly care about their own use-cases.
Also on a slightly different note, the advice about using small constructors and non-defensive methods. That goes pretty much against the whole point of encapsulation IMO, which is to protect and more importantly guarantee invariants. What's the point of a class that doesn't do much of anything besides naming things?
And if the tests are annoying that’s usually a sign that they can be improved.
Red->Green->Keep Moving
I'm often surprised how thoughtful past me was with code, knowing how people might try to screw it up. Even to the point of having useful and sometimes fun error messages. Feedback from other people agrees with that assessment.
Avoiding misuse is avoiding the untestable.
I honestly don't get it.
Endless classes of bugs eliminated through these "anti-patterns".
Guess what: if unit tests pass in an OO system, it doesn't mean a fucking thing, especially dynamic languages where simply changing a name in a large codebase is a risky operation.
And fuck virtual methods.
but, some kinds of code can permit a huge number of execution paths with different behaviour and interactions that need to be tested. If you want to cover all the different paths using only integration tests without decomposing a system into simpler subsystems that can be tested more locally, then you end up with a combinatorial explosion in the total number of execution paths and the number of test cases you have to write and maintain.
Class-based unit tests lock you into a specific implementation, providing a checksum on the current behavior, making it as hard as possible to fix an actual bug when it's discovered. Collaborators-included unit tests on the other hand, do not break unless your product is broken. It can be harder to chase code coverage when you're doing laparoscopic surgery on your code, but the end result is that every conditional has an actual use case behind it, and if the requirements change, you can see the name of the test with the bad requirement and delete it.
I still think that a lot of the patterns that make your code testable are still good though. It's not always clear at what level of abstraction you want to start mocking. Do you mock HTTP? Do you mock a martialed API? Do you mock a business logic abstraction that depends on that API? If you use DI for all of these layers, then you don't have to get it right the first time. You have options when you're writing your tests, and can use concrete or mock collaborators according to whatever your current need is.
Good luck testing your cloud functions, dataflows, BigQuery integrations etc. etc. etc.
From my first commercial software job: a few thousand lines of complex C++ (mix of decoding the output of a mathematical model mixed with business rules) structured as a single "god class" with heaps of internal state and a dozen methods -- comically all of the methods would return void and took no arguments, and would call each other. It took days of careful refactoring and consultations with a colleague until it was possible to instantiate one of the things in a test harness and do nothing with it. In some sense this was dead simple code as there were no dependencies on external resources or concurrency.
Are they suggesting that all/most of my methods should be virtual? That's just nuts. Next they will say that all of the members should be public, since it's easier to test that way.
https://eel.is/c++draft/macro.names#2
Otherwise this is one of the unintended effects (on Itanium ABI):
mingw gcc (11.2.0) does have the same 8 vs 12 difference.
https://godbolt.org/z/ddqd3vr5x
Notice the call to `?foo@A@@AEAAXXZ` in one compilation and `?foo@A@@QEAAXXZ` in the other, supposedly to the same function. You might get fun linker errors this way, possibly nothing nastier though.
Luckily I don't need to link anything, so I don't care about that for now.
It also covers how to avoid writing them to begin with.
Only 20% left is to focus on testing business logic.
I'm not saying that's not coding, because lots of things are coding. But I am saying that's not the usual case that I see.
My experience in dynamically typed language is that it's very rare to see a typing error that doesn't co-habitate with at least one logic error.
I thought I'd seen this here more recently, but I guess it was another similar article.
>Create Utility Classes and Functions/Methods
This doesn't make something untestable. Take for example a max function. A max function such as Math.max is a part of a utility class, but it's easy to test.
MathOperationsProvider::Init(endianness_config, locale, wolfram_alpha_connection).
Max . . . max is pretty easy to justify. But MaxMinusMagicFiveForUnseenReasons . . . not so much.
The point of all of this (I assume TotT) is to have the conversations. If you're not having them at design time or code review time or incident response postmortem times, you're doing it wrong. And you're reminded every time you visit the restroom (again, if it's TotT).
That's my interpretation.
Except they can, in most ecosystems.
* Throw useless errors on getter or has methods!
I recently gave up on using MapboxGL.js because of this.
How to write untestable code: Use OOP ;-)