Still no consensus on testing private methods
jesseduffield.com
jesseduffield.com
If you want coverage then private method testing is giving you fake or inferior coverage. Those hard to reach paths are only truly tested via the public interface (i.e. called by the function that will call them in practice, not your contrived test function).
https://testing.googleblog.com/2013/08/testing-on-toilet-tes...
https://medium.com/expedia-group-tech/test-your-tests-f4e361...
There are different reasons to have tests.
Unit tests often break even during small refactorings but are great to quickly cover of lot of edge cases of some complex piece of code. They also quickly point to what exactly is wrong.
End to end tests on the other hand should be almost immune to refactoring problems, but they are very slow and often it is tricky to find where the bug is or even if there is a bug at all in the application - in those tests bugs are very often in test code.
Tests for those are unit tests, and fit the GP's description. And those are the best units to test, often you don't want to unit test anything else.
Of course, YMMV. For example, on some languages you must test if the data has the correct structure and the functions have the correct interface, what is best done by unit tests, and those tests will break often. Some problems are so complex that you must test intermediate steps, otherwise you will never get it right; those tests also break often, but when you need them, you need them.
Unit tests should be cheap to write and cheap to reimplement. Otherwise GP’s right and the tests end up shackling you to an implementation, instead of defining the requirements programmatically.
If you make a change and a unit test breaks, do your best to make sure it’s not an actual defect. If it isn’t and fixing the test looks like more work than you want, then delete the test. Maybe make a new one to cover a similar case if that makes sense.
“Less tests” is a really easy state to get to. Just highlight and delete. If I think having more tests is good and you think that it’s bad because you don’t want the tests there if they break, that’s not a problem! We can get back to the way you want things in less than a minute.
Someone writing a test is not a sworn oath to preserve it forever. It’s not even like a public method. No one is using it.
The sentiment of the test may still apply, but the implementation details mean you're testing, for instance, for empty set using a different strategy. Block delete and start over.
Or the feature you're adding is meant to allow us to deal with empty set: let our customers put things in their shopping cart even though we don't have any shipping addresses for them. In that case we 'broke' that requirement, and just delete the test and move on.
But getting cheap tests and high coverage is a bit of an art, often learned via the school of hard knocks. If you let too many requirements leak up into integration tests, you may find you have many, many tests to fix when a "never" or "always" changes to "occasionally", "mostly". Some people get brave and delete these tests, and discover Chesterton's Fence later when someone yells at them for deleting regression tests, because the test described 2 scenarios it was asserting, but it surreptitiously asserted 2 more, one of which the New Guy just broke but got a green build.
With a unit test it's tenable to make a test that looks like it checks one rule while actually checking only one rule.
Isn’t that last step super weird? I don’t see how not writing tests would have prevented the regression. At most vague hand wavey “people would be more careful” which I don’t buy. The problem was breaking a dependency. Tests are not themselves dependencies, they are an automated stand-in for them.
An example I have is in a pay computation engine. I have a public function that computes how much someone ought to get paid for a given shift based on who was working the shift, what they were doing and a bunch of other criteria, what day they were working on, what time of day, etc.
Within that I have a private function that works a with a narrower scope that just does the pro-rata calculation for time-of-day and day-of-week. That function itself has a fairly high-level interface with a highly complex implementation. IMO it makes sense to test that function, even though that function is private and is only used by the one public function.
My point being whether it's private or public isn't really the point. The key thing is whether the functionality is self-contained and decoupled from the rest of the code with a stable interface.
The library itself is public and could have other users, eg. you when invoking manually from cli when testing things out.
Test your library code with unit tests - this is easier because it can be done in isolation feom external systems.
Itegration testing of apps is difficult. Often it is sufficient to test by use - have a dev environment which is usable or test in prod if it's not a critically important app.
This is often sufficient, but I'm sure testing gurus have a recipe to apply unit tests to the app itself as well. I personally prefer not to have 10:1 test to code ratio, so I keep things practical.
Absolutely not
Your private methods contain logic no?
Every coverage is worth
In fact I don't see any "controversy" here. Not testing private methods seems like a lousy excuse
exactly because they change less with implementation/api changes and are more "logic focused"
Sure, do test your API methods, but that might not always be easy or practical
This hides the private implementation details, but keeps them accessible for when it's needed for internal tests or workarounds. It's easy to find code which relies on the private namespace, and one can't accidentally use it.
These tests that cross from black to white box testing could be construed as functional tests, in which case my solution and yours aren’t so different.
Also important to remember that it’s a mistake to treat library and application code as the same thing. They are not the same, and from my observations a lot of arguments about The Right Way come down to trying to treat them as if they are.
Regardless, as you noted, my point is to not prevent access but rather make it obvious if you're using implementation details.
Framework doesn't care if half your unit tests are functional tests, if you slide E2E test code into the before*() call in an integration test. If your white box and black box tests are snuggled up to each other in the same test suite. If your tests only pass if they run sequentially, in this particular order.
It's no wonder we can't get the terminology straight. You can be years into writing tests before anyone or anything calls you out.
They have the same pattern at two different levels. What’s the essence of public? It’s that there’s an encouragement for people to use it and you don’t know who is using it. Private is the opposite. Its usage is discouraged or not allowed beyond a certain boundary, so you know everyone using it in an endorsed way.
In a library, your public methods are public and your private methods are private. In a service, all of your code is private! Critically, it meets the definition that you can see all of the usage.
So, why do we test public methods in services? (I’m sure someone would say not to). We just want to be more confident they work as expected now and will continue to do so in the future (I think people forget that’s a [the, even] big reason for tests). Sometimes testing something “more private” gives you faster, more accurate tests.
Maybe you can see it coming that I’ll argue by analogy it can also make sense to test private methods in libraries. They’re not part of your external API. But, just like public methods in a service are an internal API, private methods are part of an internal API for the class that is used by your future self and coauthors.
Indeed. In a good code, one could say it's libraries all the way down.
For example, I've refactored a lot of business code such that it can be easily re-used in different parts of our application. Ta-da, I effectively have created a piece of library code, and I'll want to write tests for it to make sure it doesn't break any consumers, current or future.
In the case of testing private methods it's no different. For example, testing private methods can lead to code having to be refactored every time you change something. This is bad, especially when presumably those methods are utilized by going through the public interface, which conveniently shouldn't change a lot.
However, it also depends on how the code is structured. Maybe it's a large systems project where there are huge private libraries that let you do important stuff. Obviously that needs to be tested. Or maybe you have an abstraction for dependencies that need to be injected at runtime to do real work, but you have a private implementation that's not exposed publicly. Even in that case, for example, it really depends.
So, it depends, and it probably depends on what do you get for doing things a certain way vs another. I'm going to say most of the time "no", but it's definitely not a rule.
Are these tests going fail any time someone makes the slightest change to the implementation?
Do these tests require a ridiculous amount of setup?
Are these tests going to run so slowly that they adversely impact the velocity of the entire team?
I agree that as a general rule it’s preferable not to test private methods directly. But if testing a private method is demonstrably more efficient than getting at it through public methods, and the tests don’t end up being too brittle, then that’s probably what you should do. You could also take a look at refactoring the code to make it easier to test - but again only if that actually improves things.
Most of the time, I see private methods being tested when the library is “in the middle” of other libraries, or is not properly abstracted.
This article in particular is weighing up and contrasting various viewpoints so very little value is added in repeating one of those viewpoints unless you also explain why you think the interpretation originally given was lacking or flawed.
I think it's the opposite, there absolutely doesn't have to be a wrong. Assuming someone must be 'wrong' is not a healthy viewpoint to take.
What we're running into is a limitation in the languages and frameworks we use, and we are assuming that those languages frameworks are right, they are the only way things should be done. That limitation is then being expressed as the various viewpoints that author has listed.
I'm glad the author took the time to enumerate the various viewpoints and I'm sure there are more!
Can you elaborate on this point more?
https://blog.racket-lang.org/2012/06/submodules.html
Framing discussions around "classes and methods" is a rather narrow-minded view, which ignores lots of potential solutions.
On your point about language limitations, I also wonder about this. I vaguely recall coming across a post that mentioned testing private methods just wasn't an option in various languages in the early days, and it's possible that were that not the case, we wouldn't have so many advocates of the never-test-directly viewpoint. That's not to say that the viewpoint is wrong though.
Of course this is hard/time-consuming, which I imagine is why you don't do it (and neither do most people arguing about issues like this), but it's the only way to honestly evaluate something. IMHO the answer you will come to is "it depends"... because of course it does. ;-) (For example, I'm pretty sure I could come up with examples of methods where e.g. making the function "pure" wouldn't really be a good trade-off, and I could also come up with examples where it would be.) But if you can arrive at a different answer, that would certainly be interesting.
Sometimes I find it is just more efficient to test `privateFunctionA()` and fail out early. This does not prevent me having a test for `publicFunction()` that runs through `privateFunctionA()`, `B()`, `C()`, and `D()` to test the full flow of interaction between them. I refuse to make a function public _just_ to test it and would like my public interfaces to be as small as possible. This means a lot of private behavior to test underneath, and sometimes it just makes sense for me to test those chunks individually.
Many people wouldn't agree, and that's cool - it just means another approach works better for them. If I have a choice and am writing something from scratch, I'll do it my way. If I'm working in an existing codebase I'll err toward sticking to the existing standard (or suggest changing it if I don't think it's working very well).
Therefore, for the purposes of demonstrating/articulating what I mean - with "Proper TDD™" I am always at a state during the Red and Green stages, that there is no private method to test. They just don't exist, because I have thus far only done the simplest possible thing to pass the test. (This is the same course of events when I am not using tdd.)
After the Refactor there might be some private methods, but they will already be covered.
As for the API argument, if you don't want implementation exposed, then don't expose the class. This is what Interfaces are for. Need to expose a class? Then make a "contract" class such that the stuff you don't want exposed isn't.
So if you are testing all the public methods you are at the same time testing the private methods that get called.
In the programs I write it is often the case that there are private methods which nobody calls because the program has evolved that way. It would seem wasteful if not misguided to test something which nobody calls but the tests.
> So if you are testing all the public methods you are at the same time testing the private methods that get called.
that's nice in theory but in practice if your API looks like
class my_algorithm {
public:
float compute_next_value(float);
private:
void deborgle_fields(...);
void reticulate_splines(...);
void deboor_network_max_span(...);
vector<float> m_tempArray1, m_tempArray2, ...;
vector<complex<double>> m_protoscalarField[3];
};
In this case you usually have 2 classes : a low-level algorithm-specific one, used by a higher-level consumer-centric one.
Most of the time, testing private methods is a poor solution to a larger design problem.
I don't think "just for testing purposes" is a good answer. Unit testing hooks shouldn't be in the public API of a library.
If the one public method just calls private method A, B, and C in order, passing the result of one step to the next, and if we don't need to handle different variants of A, B, and C (meaning dependency injection is not necessary), we arguably don't gain much from splitting the original class in two. We're left with a very small class of a single method that's tightly coupled to the extracted class. We can remove the coupling with dependency injection, but now we need to add some code that actually injects that dependency at runtime. So understanding how the pieces fit together is harder. The only benefit to splitting the class that I can think of is in preparing for a future situation where orchestrating the three helper methods becomes more complex, or where you actually do need to handle different variants of A, B, and C via dependency injection. But it feels like premature abstraction to me, single responsibility principle notwithstanding.
I don't see why the class needs to be extracted in that case. If it makes the code easier to follow, do it; if it adds bloat and boilerplate, don't do it.
The key thing is the ability to expose stuff for testing only, not in the public API. Depending on the language, you might not need to extract a class at all -- just make some fields or methods package-private.
I guess if your language doesn't provide a way of doing that, extracting a class is a reasonable workaround because you can segregate your public API into high-level and low-level parts, and recommend that end users avoid the low-level parts. But that's not ideal; better to hide the low-level stuff if you can.
From experience, what you care about in that case is to make sure you implemented the private functions correctly, because it might be some tricky algorithm or whatever. Tests with a lower granularity than the public API make a lot of sense to achieve that goal, it's pretty similar to TDD (though you do not necessarily have the write the tests before the code). On the other hand, in the vast majority of cases, the code, once written, will not be changed. If the public API is changed, then it's likely the internal implementation will be replaced, or changed. In any case, the "implementation tests" will break because the contract has changed. So I delete them, the maintenance cost is higher than the long term value they bring, and non-regression should be ensured by the public API tests anyway.
Sometimes, this is wrong, and you will actually need to extend the code in such a way that keeping the "implementation tests" would have helped. I haven't found a good way there to store the test "just in case". You could for instance have a commit that adds them then a commit that removes them, but this is a lot of noise in the history, and the probability that anyone but you can find the deleted tests if they need them is pretty much 0.
Why would you ever remove a test ? You don't know when your platform will change things from under your feet, for instance going from x86 to arm, which may break them in subtle ways without you changing anything to your code. And then you're losing hours searching in your 10k commit history the code that had been removed.
Because the cost of building/running/maintaining and generally just having the test around is higher than the value I get from the test. And because I still have unit tests on the public function that should catch issues. Really, I just wrote them to help me during the dev part. Every extra line of code is debt.
> And then you're losing hours searching in your 10k commit history the code that had been removed.
You're right, I said it was a possibility, but I don't do that, it's not worth it. It has happened twice so far I think that I wished I hadn't removed the tests. I bit the bullet and rewrote them, which is okay because of how rare it is.
Sage wisdom, I wish managers took this up a little. They'd improve throughput by 10x or so in a week, and new developers would have shorter lead times. Plus quality would go up, the fastest most correct code is the stuff you never write.
I think you are right, unneeded tests are debt especially because implementation evolves during development (and maintenance) which often means you have to rewrite the tests if you modify the implementation, or when you change the division of labor between public and private methods.
Note it is possible that a test of a private method might fail yet all public methods calling it would still work correctly according to their "spec".
It answers the question "what failed?" on a deeper level. Something you'll need to know when the public-method test fails anyway.
It's rarely needed on proper API designs (in general such scenarios can be split into a separately-testable object which is not by itself exposed), but it's not too uncommon in my experience.
In such cases, all the solutions to test private methods pretty much suck.
There is consensus and it's don't test private methods.
Testing private method destroys your coverage data. It effectively makes it a meaningless metric. Coverage isn't just there to show that you tested "everything"... It's there to show that things are actually used in the code and that code isn't dead.
Testing private method proves a private method works. But that's not something anyone should care about. It goes against encapsulation and just makes flaky tests that are a nightmare to maintain/refactor.
We choose not to run only end-to-end tests because it's expensive both in terms of writing the tests and running them. We choose not to run only unit tests at the lowest possible form of encapsulation because they're more likely to need rewriting when we refactor. Instead we pick out abstractions at various levels of encapsulation that warrant testing in isolation. The decision on whether to test a private method or a private class or a private package differs only in degree, not in kind.
To convince those who disagree with you, you'll need to make a case for why private methods should in fact be held to a different standard than other levels of encapsulation. I'm open to being persuaded on this point but I can't think of an argument that convinces me.
Yes. Ideally you want high coverage of integration tests. If you're going for best quality then sure. The high coverage of unit tests is valuable since we don't want to fail on integration tests. We want to fail on unit tests (easier to trace the failure, faster to run).
Private methods by definition are not an abstraction. Testing them makes absolutely no sense. They are either reachable or they are not.
The one reasonable argument is that it's hard to create a synthetic test that will reach some private methods. That's a valid claim but that's why writing tests is hard. Going directly to the private method is "cheating" and will cause code rot since again, it breaks coverage.
This goes beyond private methods. Say a branch is never reached through a public interface but you test it in your unit tests since you don't know. This aspect is completely hidden from you.
> To convince those who disagree with you, you'll need to make a case for why private methods should in fact be held to a different standard than other levels of encapsulation. I'm open to being persuaded on this point but I can't think of an argument that convinces me.
Encapsulation literally defines this as a core principal. If you don't accept that then you don't accept the most important concept of OOP.
You're thinking about this in the wrong direction. If you want to break encapsulation you need to come up with an argument that's better than: "It's easier for me in this specific case".
A private method is indeed an abstraction. If you take a chunk of code and extract it out into a method, that's an abstraction, regardless of whether it's private or not.
> Say a branch is never reached through a public interface but you test it in your unit tests since you don't know. This aspect is completely hidden from you.
This is purely a tooling problem. There is no good reason why your linter can't flag that a private method is unused in non-test code.
> Encapsulation literally defines this as a core principal. If you don't accept that then you don't accept the most important concept of OOP.
Do you have a source evidencing this claim? I would be surprised to find any canonical source stating that private methods should be held to a different standard to private classes and private packages
> If you want to break encapsulation you need to come up with an argument that's better than: "It's easier for me in this specific case".
My argument is this: we 'break' encapsulation every time we write a test that's not end-to-end. The question is how much we want to break it, and that question depends on how stable and well-defined the abstraction is that we want to test. If a private method is stable and well-defined, and if the cost of extracting it out into its own class is too great, then it's appropriate to test. A private method is less likely to be stable and well defined compared to a class or a package, because it's at the bottom of the encapsulation hierarchy, but it is not fundamentally different to other levels of encapsulation, and therefore there will be times when it is appropriate to test.
In fact, inducing the private methods to be adequately tested via this indirection can require far more test code than testing directly because of the logic required to create suitably parameterized calls to the private method. It also tacitly exposes internal implementation in the test of the public interface, since this is the only way you can push parameters to the private method interface. In cases like this, you should always test the private method directly because it actually improves the abstraction of your tests away from the implementation details.
It's often much easier to write a test if you can poke at the internals a little bit, but it's a bad idea to widen your public API just so you can do that.
So, make your internals package-private and put the test code in the same package.
Unfortunately this isn't easy to do in all languages (e.g. Java gets it right, but Kotlin bases visibility on build packages rather than namespaces).
This is a nice overview of the viewpoints anyway.
The solution is "stupidly simple". I have at my module level a public subroutine with the name "test_module" which performs the tests of all the private subroutines of the module I want to test (these are usually analytical derivative calculations and stuff like that).
Yes, I have test code in my module, but in practice, after 10+ years of using this pattern, nobody had issues with it.
Not too dirty (ahem...) and allows you to test various internal parts. And I must admit I got the idea after looking into Rust :)
All the tests end up in the "release" binary, along with the application code. Separate test_main entry point to run the tests instead of the application. If I don't want the tests available, marking test_main static and deadstripping rips them all out.
I don't even really use "methods" strictly speaking, in general I'll use a dictionary of functions if I want to group functions together semantically, and I'll emit such a dictionary from a closure if for example I need to precompute a lookup table to be used for multiple related calls.
I've had more negative experiences with being denied the ability to do something by overwrought architecture than shooting myself with a footgun someone left in, but importantly I think my approach generally leads to more composability and agility.
Isn't this analogous to using private methods, though? I.e. you have some logic to precompute the lookup table that isn't accessible to users of your dictionary of functions? Am I misunderstanding how this works? Or are there benefits of this approach vs. for example having a private method "precomputeLookupTable()" that gets called in a class constructor?
2. yes
3. no
4. no
5. no[2]
[1] CAMs do not change the issue of functional accuracy in any way. This is irrelevant.
[2] This is the result of developers thinking about their objects designs differently, which is good. Class access modifiers are for mixin property selection. That's what they have always been about. The idea that CAMs are for object interface definition is a cargo cult that developed around Java early on (because Java was rather primitive), unnecessarily mixing the concerns. Eventually it ended up in the classroom, to the detriment of the industry.
Similarly "final" is an ironic language mechanism that disempowers the users it's meant to aid, because it's assumed the indicator "constant" is insufficient.
however, increasingly my view is: the less you test methods and components in isolation, the better. Unit tests are highly overrated and used too much to the detriment of all other testing.
If a private method contains a lot of complex business logic, you do want to test it, however, not necessarily if this code can be easily tested by calling one of the public methods.
I've seen private methods used within internal projects at companies, and admittedly I've created many private methods in this condition, but unless what you're shipping is a library then what exactly is the point of making your methods private? To stop the developers from misusing their own code?
For the most part, I sometimes use private methods in my own projects as just a way to make it more immediately apparent that something is just supporting code for something else. If no one else is using the code as an API, and it somehow turns out that the private code needs to be tested directly, it becomes a candidate to be promoted to public because there'd be no reason for it to be private at that point.
One anecdote, I'm currently building a bit, complicated (in terms of domain logic, it's just a CRUD app; rest api, database, xml file writing) Go app, which has a very simple visibility system; any identifier that starts with a Capital letter is public, anything with a lower case is (package) private. There's some more details if you call a folder `internal`, but that's for libraries.
Anyway, one of my packages, `xml`, has two or three public functions; input is a domain model, output is a (byte array containing) an XML document. Simple and straightforward.
But internally, what is included in the XML depends on a ton of business related rules; if this field is this value, that tag should not be written; if that field is set to that value, this tag should be written but emptied out; if this field is this domain struct value, it should be written as this string. That kinda logic, times twenty.
I can test it using the public API just fine, it just makes the unit tests very... voluminous; for every module, times every edge case, I'd need to create a full domain object and a full expected XML module. That's over 50 LOC per test. This package currently has >200 test cases.
I decided to test private methods. Each private method has a table test containing each module's particulars and edge cases. It makes more sense in my head, it's faster to execute, etc.
Especially now that I work in Go, I'm more and more Aware that code is not so much about code volume or cleverness, but it's about managing complexity. I'm building an application that will juggle >100 domain entities with dozens of fields each, and I have to write it in such a way that I - and hopeful future maintainers - can manage this complexity. If that involves testing private methods, so be it. It doesn't feel bad, and I'm very confident that once this logic is written, it'll remain mostly untouched for the next decade.
This is called “over-engineering” by some, but really it is the most testable and refactorable approach to creating programs (other than being a team of PhD Haskell programmers).
The problem is simply that programming hours are expensive and we don’t got time for this methodology, in general, just as we don’t have time for Ada or formal methods, in general.
AI co-programming will change all of this.
Edit: this approach is different from Option 1 in the article because it’s about refactoring and not just flipping method visibilities even where it makes no sense for the interface.
Am I the only one that feels like this all just points out to flaws of the language itself? We should not have to consider changing private to public just because we want to test that function. Moreover I'v seen comments on docs.microsoft.com along the lines "not to be used", "for internal use only" etc that may be a result of language shortcomings.
C# has this attribute InternalsVisibleToAttribute[1] where you open up all your internal stuff to other (test) assembly that solves one of the problems: your test assembly accessing YOUR code assembly.
Now for the wishlist. The issue with InternalsVisibleToAttribute is that the assembly author controls this. I'd like to control this as a consumer too. Given that (citation) "The idea is that when writing library code you couldn’t possibly know what method your clients will want to use ahead of time, and defaulting to private methods will cause more problems for you and your clients than defaulting to public (or protected)." - so why not take responsibility to myself of using code that may break in next update? Like PrivateVisibleFromAttribute. Or a language construct like unsafe {}, but private {}/internals {} where intellisense will happily list all private/internal methods. Maybe not a good idea for all cases, but I can see this useful: "that library X won't need many updates and it has that complex function I'd like to use to exactly mimic functionality X". Or: this legacy system written 20 years ago won't have any updates, but I must implement XYZ, part of which already private functions by original developers provide. Or I may want to extend that private class, override with my functionality (re-using parts of that code) and provide bindings for runtime to use MyVendorClass instead of VendorClass.
The thing is, you can do this with C# Reflection (access private members), but it's cumbersome. Better have language provide necessary tools and most of this discussion wouldn't happen as test assembly will happily tie into internals.
[1] https://docs.microsoft.com/en-us/dotnet/api/system.runtime.c...
If your code is a collaboration of objects communicating via public methods, it's almost certainly going to be overly configurable (if it's using dependency injection) and thus be very hard to test every code path, especially given that consumers will probably be able to replace individual objects in the collaboration. It'll also be harder to understand, as objects which are broken out for modularity reasons show up publicly.
Minimizing public API surface is particularly important for versioning. If you don't control your consumers - if your API isn't fully internal to an organization and living in some kind of monorepo - then a big public API inhibits refactoring and feature extension as consumers take inappropriate dependencies on your implementation details.