1. 14
  1. 12

    Private methods should be tested intrinsically as a side-effect of testing the public methods. If they’re not, then why do they exist at all?

    1. 8

      The article goes into depth on this, but suffice to say that sometimes testing just the public API can be unwieldy. In such cases, testing private methods in isolation with various edge case values can be faster and easier to do, giving you more confidence in how that part of the code behaves without jumping through a lot of hoops to set up enough state for the public API.

      Of course (as the article also argues), you could argue that such clean internal APIs should be extracted into their own classes. YMMV, “it depends” and tastes differ, so it’s one of those things you can (vehemently) argue about until the cows come home without ever reaching consensus.

      1. 7

        I know @sjamaan already responded to you with basically the same answer, but I wanted to add to it.

        It’s basically the same calculus as trying to figure out how many “unit” tests to have vs. how many “integration” or “end-to-end” tests to have. One might ask “Why have any unit tests at all? The end user isn’t running ‘units’ of my program. The ‘units’ will be tested as a side-effect of running the end-to-end tests.”

        And, honestly, that’s a fair point of view, IMO. But, we tend to test ‘units’ because it’s easier to write a more thorough test for smaller chunks of code. So, the idea is that while having 100%-branch-covering end-to-end tests is the best way to insure that the code is correct, it might be an unreasonable amount of effort to accomplish that. Maybe deferring some of that code coverage to having faith in the individual units being correct and well-tested would reduce our confidence by 2%, but also reduce our dev effort by 15% (totally made up numbers, of course).

        So, I see testing private functions as the same thing. Maybe it would be a pain the ass to exercise the public API in such a way to guarantee that you’re fully exercising the private function. So, let’s just test that one function for off-by-ones or whatever.

        I certainly wouldn’t say that testing private functions should be the norm. But I think it’s definitely a valid thing to do in some cases.

      2. 7

        One technique I’ve seen to effectuate #4 is making public methods named things like this: public mergeArrays_ForTest(...) { return mergeArrays(...); }

        Java can use @TestOnly annotations that compile out the function, similarly for Rust with #[cfg(test)].

        This technique can also be used to expose setters of private fields that would blatantly break invariants if anyone actually managed to call the function, but are necessary to actually set up the fields for the scenario you want to test.

        1. 1

          This sounds simple, clean, clever and effective. I’m almost ashamed I’ve not thought about it before. Thanks, I’ll definitely do this in the future.

          1. 1

            @TestOnly annotations

            Is there a .Net equivalence? We do this but don’t annotate them, so it’s more an honor code.

            1. 2

              I normally mark these methods internal and use the InternalsVisibleTo attribute to allow the test suite to access them.

              1. 1

                C# has preprocessor directives, so you could wrap the public methods only to be used for testing in something like #if TESTING

            2. 7

              I’m very happy with the solution in Rust and D that allows putting tests wherever you want. The tests can access things that are private in their scope, so if you put a test next to private methods, you can test the private methods if you want to. This way you don’t spend energy on exposing private details for external tests, but also the tests are where the code testing them is, so if you refactor/delete the code, it’s easy to change/delete the tests too.

              There are plenty of rules for writing tests properly, but the sad reality is that many projects struggle to have any tests at all, so lowering barriers to writing tests helps more than anything.

              1. 2

                if you put a test next to private methods, you can test the private methods if you want to

                I haven’t given it more than 20 seconds of thought, but I now wonder if I could do this in Node, too.

                1. 1

                  Probably not without bloating your artefacts, or a custom (but possibly simple) compilation step.

                2. 2

                  Another way to dodge the dilemma is to split up the project into libraries. Each library will have a clearly defined public API that you can test, but without the baggage of testing and mocking the full combined product.

                  1. 1

                    That sounds… The same as unit testing? Have a public API, test the public API, don’t test internals

                    1. 1

                      It is, but the trick is in changing what the “unit” means to suit what you want to test, rather than have the “oh no, it’s private, so I can’t test it” dilemma. It’s the “make it public” solution, but deflects objection of making things public just for testing, because the exposed implementation detail is now called a “component” or “library”.

                3. 4

                  I’m not sure about methods, but for private functions I like inline tests similar to Rust’s #[test]. They don’t require hacks that get around the module system and they keep the tests close to the code, so they can be used as a sort of documentation.

                  1. 1

                    I love the way those inline tests work in Rust. I love being able to have the tests right next to the code in question, and I do love having the option to test a private function and not having some language designer decide what code of mine I’m allowed and not allowed to test.

                  2. 2

                    I don’t often say positive things about Go but:

                    One of the things that Go really got right was to understand that there are precisely two visibility specifiers that actually make sense, from a software engineering perspective:

                    • Anything can access this thing.
                    • Anything in the same package can access this thing.

                    In the first category, it is part of your API contract. In the second category it is not. Anything beyond those two is just documentation. Someone who can modify the contents of the package can trivially toggle something between different visibility levels, add friend declarations, or whatever other escape hatches your language provides. At best, they’re documentation but they’re misleading documentation: they don’t tell the next person who modifies the package why you’ve decided not to expose something directly to other classes in your package (is it because you didn’t need to yet? Is it because you want to have a single extension point later? Is it for some other reason?).

                    If you have only package and public as your visibility specifiers then this question doesn’t arise at all. If your tests are part of your package (your unit tests are part of your package, aren’t they?) then they have access to all of the internal implementation and so can test at whatever granularity makes sense. If they are outside then they must be integration or API-surface tests and so don’t need access to the internals. If your language implementation does good dead-code elimination then you can even ship the tests as part of the package and anyone who installs your package can run them from their test harness if they want but have them eliminated from the final binary that they ship.

                    1. 2

                      One point the article may have overlooked is that the private method may not have any reason to exist in the first place: if it’s used only once, you probably want to inline it.

                      This is especially visible in Not-Actually-Clean Code by Uncle Bob, where the SRP is taken to such ridiculous extremes that merely inlining private functions that are used only once often shrinks the code by half, and in the process makes it much easier to read… and test. Turns out readable code is often easier to test, because it’s easier to guess what potential errors we should test for.

                      1. 2

                        One point the article may have overlooked is that the private method may not have any reason to exist in the first place: if it’s used only once, you probably want to inline it.

                        I’ll go further and say private methods just don’t need to exist. Python gets by just fine with a naming convention, and no enforcement.

                        1. 1

                          if it’s used only once, you probably want to inline it

                          What you say about using private methods to simply break down a long function into smaller ones?

                          1. 1

                            It obfuscates the code more than it helps.

                            One thing those “OMG long functions are unreadable” people tend to forget, is that code locality isn’t just for the instruction cache, it’s for humans too. If a function is broken up into major steps, I must fetch each step and read it. If they’re elsewhere in the file, or worse, in a different file, it gets real inconvenient real quick. Not that I don’t want to keep those steps separate. The scope of variables for instance should be limited, and in practice I often end up using braces.

                            In any case, arbitrarily breaking up a function into smaller pieces just for the sake of it is doing you a disservice: you get the illusion of a short function, but it’s still just as long! Longer, if you take function call overhead into account. And harder to read, because of the loss of code locality. In any case, that’s no good. Either genuinely simplify it, or own up to the full horror of what you are doing.

                            1. 2

                              This really depends on whether that method encapsulates a single step in an algorithm or not. If it does, then someone reading the outer function doesn’t need to read the private method, they just read its name and skip over it. If you inline it, then you inline a comment explaining why it’s there and a block and the same reader will need to skip over a block, which has a higher cognitive load than skipping over a single line.

                        2. 1

                          See if you were using a good language like c++ this wouldn’t be a problem, you just need a few defines for class->struct, protected->public, and private->public. Voila, nothing is private any more, with only minor semantic differences ;D

                          I have a recollection of this actually being done by some testing framework in the distant past, but it does cause some problems as I /think/ there are some windows ABIs that vary on class vs struct (in mangling I think?), and c++ probably has overload resolution consider the protection of methods/fields :D

                          1. 5

                            In C++ the cleaner way of accomplishing this is to say something friend class TestHarness and then include methods in TestHarness that directly call the private methods that you want to test. Something like this:

                            class TestHarness;
                            class SuperComplicatedClass
                            {
                               friend class TestHarness;
                               void somePrivateMethod();
                               public:
                               // public stuff here
                            };
                            
                            // In a separate file that isn't part of your library build:
                            
                            class TestHarness
                            {
                              void callSuperComplicatedClassSomePrivateMethod(SuperComplicatedClass &cls)
                              {
                                cls.somePrivateMethod();
                              }
                            };
                            

                            You can then test exactly the build that you expect library consumers to use, with your tests calling the helper in TestHarness.

                          2. 1

                            Isn’t this what default access is for?

                            1. 1

                              In Java, package access also permits subclasses, which means it becomes part of your API.

                              1. 1

                                This document says default allows package access but not subclass access.