Hacker News new | past | comments | ask | show | jobs | submit login
Still no consensus on testing private methods (jesseduffield.com)
68 points by zdw on March 10, 2022 | hide | past | favorite | 91 comments



A reason not mentioned is that testing public methods only is more black box testing. It's important to do this not just to allow significant changes to the implementation without affecting the test, but also because if you tie testing too much to your implementation you may miss holes in your interpretation of the requirements/interface that are better caught in a black box test.

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).


This is so important but even most professional software developers and testers don't seem to understand it. Tests are not supposed to break on every little change to the code, or even sweeping changes in implementation. If they do, they're not very good test.

https://testing.googleblog.com/2013/08/testing-on-toilet-tes...

https://medium.com/expedia-group-tech/test-your-tests-f4e361...


I disagree. You are basically saying that only end to end tests are very good tests.

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.


There are plenty of non-end-to-end interfaces that are well defined so tests don't break on changes and still encapsulate complex and important code. There is somebody on this thread talking about one that calculates the costs on a contract.

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.


If refactoring is causing you to struggle to fix the unit tests, then what you have really aren’t unit tests.

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.


Just stop trying to fix unit tests. All of this noise about test bloat and the pain of refactorings goes away if you make one mindset change: feel free to delete tests.

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.


There's an inflection point with unit tests where a good description, a good matcher, and a small number of preconditions (mostly handled by nested test suites) make it easier to rewrite the test than to fix it. When I say 'cheap' tests, these are what I mean. Tests you can treat like livestock instead of pets.

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.


See, I think your example of breaking the thing is where things are tricky. I know you’re not advocating for it, but I feel like a lot of people get burned by an experience like that. They think “Someone wrote a test. Then it broke. Then someone deleted it or fixed it wrong. Then something else broke and no one noticed because the test was deleted. Therefore, don’t write too many tests.”

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.


As always, all the extremes are wrong.


What if you have a "module" within your private code?

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 'module' is a library of pay computation functions that your app uses privately.

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.


Based on my experiences, I don't think anything should be truly private. I found it's almost always better to put the "private" code in a separate "implementation detail" namespace that makes it clear it's intended to be private.

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.


In Java I would sometimes make them protected methods and use a subclass at the top of the suite to expose them.

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.


> Also important to remember that it’s a mistake to treat library and application code as the same thing.

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.


> 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.


Indeed protected methods can be useful for this. Depending on the task at hand, I find it can be useful to separate code into it's own namespace (one way or another) to make it more clear what the user-facing code does.

Regardless, as you noted, my point is to not prevent access but rather make it obvious if you're using implementation details.


Most test frameworks are very un-opinionated, and perhaps that is a mistake.

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.


I find testing everything through public methods can lead to excessive mocking, which ties you even more to the implementation.


How does mocking tie you to the implementation?


> coverage then private method testing is giving you fake or inferior coverage

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


I think you misunderstand. GP is recommending that coverage of private methods be achieved by testing their public consumers. If you can't test part of a private method through a public consumer, then that's a sign that those unused code branches might not be necessary.


Testing public methods is not an argument for not testing private methods. In testing, unlike in construction of software, it can be good to combine many different approaches, because each has diminishing returns and they compensate for each others weak points.


The right answer to almost any question that's worth asking is "it depends".

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.


Exactly - we shouldn’t be overly concerned with whether the method is public or private. Instead we should be concerned with things like:

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.


I look at it like this: what are the inputs/outputs of what I’m testing? Is it calling another library to do work, spy on that. Is making side-effects in the system, watch that. The only time I’d use internal/private methods directly in a test is when setting up to use the public methods would be tremendous (imagine configuring a database driver to fail in a certain way, for example).

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.


Too many of the comments here are just restating points made in the article without advancing on them. I know "did you read the article?" is verboten on HN but people should at least attempt to respond to points made within and move the discussion forward.

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.


> With so many contradictory viewpoints, somebody has to be wrong! But who?

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!


I agree with your point about the dangers of right/wrong thinking, though when I wrote that line I was specifically thinking about people who say we should never test private methods vs people we say we should always test private methods, in which case they truly can't both be right. I've updated the post to make that clearer.

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.


I think your blog post would be much stronger if you could give the "most compelling" examples you can find in favor of all the viewpoints, then explaining why any of them should trump the other. (I think they call that steel-manning?)

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.


I completely agree. The post became longer than I would have liked even without examples and didn't want to make it any longer with examples, though it would be a good basis for a follow-up post.


> What we're running into is a limitation in the languages and frameworks we use

Can you elaborate on this point more?


An example which comes to mind is how Racket handles tests: they're generally written in "sub-modules" of whatever they're testing, and hence they have access to "private" (AKA un-exported) definitions.

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.


This is one of those debates I'd have totally gotten into a few years ago and now just cannot be bothered. I have my own set of testing approaches and preferences that _usually_ work, but sometimes something else makes more sense and then I do that.

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).


My take: private methods are not _designed_ they are _extracted_.

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.


Private methods only exist to serve public methods.

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.


> Private methods only exist to serve public methods.

> 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];
    }; 

you'll have a much better time if you can test every part of the algorithm individually, even if these steps are entirely specific to this very algorithm and useless to any other code and must not be mingled with with API users under any circumstance (and thus made private)


I call those "implementation tests", and I delete the tests once I have finished the implementation.

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.


I find it beyond crazy that you'd go through all this complicated decision state machine when you could just keep the tests and mark them as friends of the class.

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.


> Why would you ever remove a test ?

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.


> Every extra line of code is debt.

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.


> Every extra line of code is debt.

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.


Change your API then.

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.


Why expose the low-level algorithm-specific stuff in your public API at all, though?

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.


This is also good for maintainability. Say you need to upgrade the algorithm but keep the old version around. You can create a new version of the algorithm specific parts without changing the public implementation and allow users to select which version to use; eventually changing the default. In a single class, this would be a nightmare.


"Nightmare" is a bit strong; that seems like a very normal and straightforward refactoring step... iff you avoided putting the low-level stuff into your public API.


I’ve mostly see people do public_namespace.private_internal.MyClass so you can use it if you really want to… but you probably shouldn’t.


Well, don't expose them then?


I see this as the main tension in the extracting-class viewpoint. Assuming your language allows it, you can extract out a class that's still private to the current package, meaning that the public API is unchanged. But there are still costs to extracting out that class.

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 see this as the main tension in the extracting-class viewpoint. Assuming your language allows it, you can extract out a class that's still private to the current package, meaning that the public API is unchanged. But there are costs to extracting out that class.

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.


To be clear, I was responding to the combined argument of the above comments: that you should split the class but not expose the extracted class in the public API. I agree that there are cases where we don't need to extract the class, but am willing to be convinced otherwise.


Any API changes to this would have bigger drawbacks than just making the tests friends of this class.


I see your point and it's interesting to think why that is so. When I test the implementation I am basically writing tests for myself to help convince myself that the code is doing what I think it does. Whereas testing the public interface helps other people in the same manner.

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 often does not work this way in practice. The private methods may have sufficiently complex and contextual behavior that it is difficult to adequately test the private methods indirectly via the public interface. This isn't that uncommon for properly architected non-trivial classes.

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.


I think this is a strong counterpoint to the "don't test your abstract, implementation-detail machines" philosophy. If you have decomposed a system into iterative passes with pre-conditions and post-conditions it can be hard to ensure any given edge case deep in the process - hence more likely you want to exhaustively test a "private" piece


In instances where I needed to test private methods, the aim was to provide extra feedback to the failure of a higher-level public method/s that by themselves do not provide enough feedback to pinpoint the issue.

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.


This is the right answer.

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.


I address this point in the post but I'll summarise here: Your argument is equally applicable to higher levels of encapsulation. If you have a class which is private to a package, then code coverage tells you nothing about whether that class is actually used in your package's public API. If you have a package which is private to some parent package, you're in the same situation. You can follow this all the way up to the binary output of an application and say that unless you're doing end-to-end tests which capture user keystrokes and mouse clicks, you're breaking encapsulation (you can make the same argument for a large library).

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.


Code coverage covers package private classes too just like it covers public methods. When I say public I mean public. Parent class too...

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".


> Private methods by definition are not an abstraction.

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.


After some thinking, I now see what you were talking about wrt code coverage, however again the same argument can be applied to higher levels of encapsulation and it's not obvious private methods are special.


Agreed. Creating tests for the implementation means it gets more expensive to replace the implementation with something else, with something better.


I think the best tool for this is package-private visibility.

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).


I said this on Reddit but I think it's pretty clear that the "test private methods sometimes" view is correct. Some public APIs make it infeasible to test the code using just the public API, but if you can do that then it's clearly preferable because you won't have to rewrite tests when the implementation changes.

This is a nice overview of the viewpoints anyway.


It's equally clear to some of us that the "don't test private methods" view is correct. But clarity is not consensus.


I think the people that think that just haven't come across a situation where that doesn't work yet. I'd guess there are entire fields where it does work, e.g. crypto or compression.


In my chemical engineering work, I have often private routines I want to test. Pardon me, but this is Fortran code, so I cannot have "complex" class structures to allow different level of testing without complicating the code too much and making it unreadable.

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.


This is also the recommended way of structuring things in Rust. It works pretty nicely.


In the C world I sometimes do something similar: the C file with the test code simply #include's the actual source file and gets compiled into one executable (together with some mocks).

Not too dirty (ahem...) and allows you to test various internal parts. And I must admit I got the idea after looking into Rust :)


In C I just write the tests in the same source files as the things they're testing. Some interface tests get their own source file.

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'll have to keep this in mind for the future, it seems like the KISS solution to the problem.


I don't test private methods and I like to use them a lot. When refactoring, if a private method is unused, both IDE's and linters will warn you, easing the process of deleting dead code. If you extract private methods into separated classes and expose them as public methods, deleting unused code becomes hard (and harder with multiple repositories).


I appreciate that the author calls out "not having private methods" as a possible solution. That's certainly my approach, I feel in general private methods are akin to docker. Yeah they're a useful tool sometimes, but only because of the road strewn with blunders that got us here in the first place.


Didn't unit testing come out of the Smalltalk culture? Smalltalk has no private methods, or public properties. So if there's stuff that's an implementation detail it goes in another object.


Can you elaborate more on some the challenges you have faced with private methods?


Honestly I've never used them of my own volition. So it's not so much that I've faced problems with them, it's just that I've never felt like they solved any problem I had.

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.


> 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

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?


1. no[1]

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.


One alternative/compromise not mentioned is to make the method 'protected'. This is usually a reasonable compromise in languages like Java. The test can access such method since it follow the same package hierarchy as the src.


They could be tested if languages supported doctests like Elixir: https://elixir-lang.org/getting-started/mix-otp/docs-tests-a...

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.


There are too many "dogmatics" in Software Engineering. You want to unit test code that is worth while testing from a business point of view.

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 think this is overthinking it. The only thing that matters are the collaborating interfaces. If you are writing a test case it should possible to act upon it through interfaces only. If you can’t do that then you probably screwed up the design and should refactor it. This is where SOLID comes in and black box testing.


Overthinking what? The article enumerates and compares different viewpoints - one of which is the one you've expressed above.


Private methods shouldn't be tested, and if you feel like they need to be tested then chances are your private methods should be public.

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.


As always there's no one answer because "it depends".

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.


In an ideal world, a class would be all public fulfilling some abstract interface which is pertinent to the logical substance of the solution, and those methods would perform their operation using other components which are private final and supplied as constructor arguments.

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.


TLDR: Language should provide additional constructs to interact with private code.

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...


You might like Uncapsulator.

https://github.com/albahari/uncapsulator


Private methods generally are a code smell anyway. If you have dozens of private methods in your class, something is probably wrong in your general design.


Private symbols reduce your public API surface, which usually means that it's easier to understand and use and has fewer code paths to test.

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.


In that case how to refactor a class that has 20 private methods? Moving them to another class and making them public would just shift the problem, not solve it


It's impossible to give a generic answer to this, it always depends on the specific domain. But I found that in these cases, there's always a design problem somewhere. Some responsibilities aren't clear, very often not much thought has been put into the design, and most of the time you can't give an immediate answer, which is why a bad design is chosen and causes a lot of damage down the line.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: