7 ms·

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

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.

jcelerier 5y ago
> 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)
- Toine 5y ago
  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.
  - iainmerrick 5y ago
    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.
    - ahoka 5y ago
      Well, don't expose them then?
      - jesseduffield 5y ago
        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.
        iainmerrick 5y ago
        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.
  - jcelerier 5y ago
    Any API changes to this would have bigger drawbacks than just making the tests friends of this class.
- Leherenn 5y ago
  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.
  - jcelerier 5y ago
    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.
    - Leherenn 5y ago
      > 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.
- galaxyLogic 5y ago
  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".
wakeupcall 5y ago
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.
invalidname 5y ago
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.
- galaxyLogic 5y ago
  Agreed. Creating tests for the implementation means it gets more expensive to replace the implementation with something else, with something better.
- jesseduffield 5y ago
  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.
  - invalidname 5y ago
jandrewrogers 5y ago
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.
- ludamad 5y ago
  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

jcelerier 5y ago

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

Toine 5y ago
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.
- iainmerrick 5y ago
  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.
  - ahoka 5y ago
    Well, don't expose them then?
    - jesseduffield 5y ago
      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.
      - iainmerrick 5y ago
        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.
- jcelerier 5y ago
  Any API changes to this would have bigger drawbacks than just making the tests friends of this class.
Leherenn 5y ago
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.
- jcelerier 5y ago
  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.
  - Leherenn 5y ago
    > 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.
galaxyLogic 5y ago
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".

wakeupcall 5y ago

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.

invalidname 5y ago

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.

galaxyLogic 5y ago
Agreed. Creating tests for the implementation means it gets more expensive to replace the implementation with something else, with something better.
jesseduffield 5y ago
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.
- invalidname 5y ago

jandrewrogers 5y ago

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.

ludamad 5y ago
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