3

I'm not asking here what the arguments are for/against testing internal methods (though I'll restate some, and don't mind hearing others). My questions relate to the implications of only testing public interface. Especially, whether I understand the extent of the "refactor" part of Red-Green-Refactor in by-the-book TDD, and if there are non TDD techniques TDD practitioners use that address the problems that make me want to test internals.

My question is: If you write the most straightforward code possible to get the light green, and if you don't test internal methods, does it follow that from time to time you allow yourself to start off by writing a class that does far more than it should with a bunch of member variables you know will not survive refactoring? And wind up after refactoring with a bunch of methods that aren't explicitly tested?

As I get familiar with TDD, it feels like anyone would at least be tempted to test internal methods. And many practitioners flatly say you shouldn't. E.g. (of about 23 million results):

Item 2 here and it's comments and this StackOverflow post

Given an implementation will involve more than one non-trivial problem, if you only test the public interface, there are two possibilities: 1) non-trivial logic winds up getting tested only indirectly, AND tested in the same calls as other non trivial logic. 2) you make methods public that you don't expect clients other than the SUT and the tests to call. I think what I hear from the advocates of by-the-book TDD is "1 is right. Yes, a lot of non trivial code is tested only indirectly, and that isn't a problem. After all, you don't want your tests to start failing when you improve the implementation. Even if they don't fail, it's not innocuous if tests keep passing when they're targetting a bunch of no-longer-used code, because the tests show how to use your class. You Should Only Test the Public Interface." And that sounds like a reasonable thing to say, but I haven't seen it said explicitly.

But even though it sounds reasonable -- if I don't test internals I feel like I lose part of what helps keep the tests DRIVING the coding (maybe part of my difficulty applying the precepts is that I don't distinguish between coding and developing). If there's some internal logic that has to turn a String to a valid int32, int64 or decimal value depending on what's in the string and the type of some other object, I want to test that little bit of logic, not just find out that the whole method failed. I want one test to pinpoint those few lines of code and exercise what should fail.

The thinking in the doubly downvoted answer here seems so obviously attractive that I have to wonder if the by-the-book fellows are neglecting to mention some instrumentation/tracing/logging code they use in addition to TDD, or if they watch it work in a symbolic debugger. Or do they just recognize that as a failure point and write a test with input that will cause a failure pinpointing those few lines of code? If they do that it's testing the implementation though. It seems so natural to want to test that code in isolation, and verify it fails where it should, even though it's an implementation detail.

I suppose that with talent and experience you can classify bigger chunks of code as "non-trivial" but I haven't seen anyone bluntly saying or condescendingly implying that that's why they don't have to test implementation details.

Lots of time it makes perfect sense to me to test the public interface, lots of methods really are like what you see in TDD examples. But for the feel of where I wonder if purists are doing something more than writing tests on a public member: Say you want to put data from an ISAM file into a database. There is one public method to the envisioned consumer of the code: public void LoadIt(SqlConnection c). "Loadit" has a bunch of other dependencies that will be resolved using configuration files and environment variables: some code has to determine that it can find the name of the folder where the ISAMs live; to find, load and parse the schema file for the isams using the ISAM vendor's DDL library; so forth+so on. But none of that is of any interest to any envisioned calling code.

My only reason for moving the dependency resolvers out of the Loadit class would be so I could test them OR to make the code easier to understand/maintain.

Moving to another class to make them testable is Doing It Wrong -- they're really just implementation details of LoadIt. And in my first implementation, I am just trying to write enough code to get the light to turn green. So I won't write anything with reasonable maintainability if it's easier to just manhandle a bunch of variables in a big method or two. I won't write helper classes. Instead, I'll get the light green and then do the Refactor part diligently. Even when I clean it up, any classes introduced keep "internal/friend" accessibility, and only get tested via the call to LoadIt.

Thanks for reading this far; as a reminder, my question is in the second paragraph.

migrated from stackoverflow.com Dec 14 '11 at 6:45

This question came from our site for professional and enthusiast programmers.

  • 1
    Interesting question, but probably better suited on Programmers.SE - I'm voting to migrate there. – Pekka 웃 Dec 11 '11 at 19:25
  • Development relies on developer talent. Clean code entails diligent refactoring. Why would testing be any different? – TrueWill Dec 11 '11 at 19:48
  • @TrueWill touche. The real question is in the 2nd paragraph. The headline makes sense in context of the post I hope. – MakeMinePanacea Dec 11 '11 at 20:10
  • @prosepective answerers... this is a real question that I'll mark answered when I get an answer. Having gotten two thoughtful replies though that weren't about what I was asking, I want to reiterate that the question is in paragraph two, right after "My question is" And I have no objection to moving to another SE site if the SO overlords think that's where I'll get answers. – MakeMinePanacea Dec 12 '11 at 4:09
  • An internal methods so complex that testing it is non-trivial is IMO a code smell. – user281377 Dec 16 '11 at 10:26
3

You have a lot of intertwined questions.. the post was long and I'm confused. But let's work at it..

First off second paragraph.

  • The common answer is not that you don't test internal/non-public methods, rather you do not test them directly via tests. The internal methods should be called by one or more types and their tests would end up verifying the internal methods as part of their execution. Reason: Refactoring becomes hard. e.g. It should be easy to move around private methods as long as you don't break the behavior. But if you have tests for these methods, then you end up breaking and fixing a lot of tests even though the functionality is intact. Remember refactoring is behavior-preserving and structure-improving.
  • Ending up with code that isn't tested. As long as you write the simplest thing that can work to make something green and refactor, you never end up with any code that isn't pulled in by at least one test. You can make the test pass with a hack like return "canned value". But then the next test should expose this and pull in the real code.

You shouldn't have to increase visibility just for testing (make things public just for testing) but you need to design for testability. This may sound like a paradox, but an example would be.. don't new up dependencies within your method. Rather accept dependencies via ctor/method arguments. This allows your tests to easily substitute a fake dependency.

Your example: I would have

  1. one test for Migrator.Transfer(DataSource, DataDestination) where the params are tech-agnostic roles/interfaces.
  2. one integration test-suite to verify a real ISAM implementation of DataSource; that verifies the class can get data out of a real ISAM store
  3. one integration test-suite to verify a real [insert DB tech] implementation of DataDestination; that verifies the class can insert data correctly into your DB>
  • I see why you shouldn't test non-public items, and I'm not arguing you should [it probably shows thru that I'm skeptical so it's natural you might think I am arguing that]. I'm asking what the implications are to the coder's practice. Your second bullet: "But then the next test should expose this and pull in the real code" -- your tests are designed to test the private implementation's corner cases, right? So formally they only test the interface, but semantically some are targeting privates and you wouldn't write them if your implementation were different? Does that happen? Thanks. – MakeMinePanacea Dec 16 '11 at 21:44
  • @MakeMinePanacea - Ok. I think I see your dilemma : my tests are "specs" ; they are not implementation dependent. e.g. If I want a method that sorts, I write a test that feeds in input data sets and verifies output sets. My test would not be different if I satisfied the spec by implementing BubbleSort or MergeSort. Corner-cases can be tested by passing in different input-sets that would cause the internal code to be executed. If you can post such an example, maybe I can reword it OR I end up learning something new. HTH – Gishu Dec 23 '11 at 5:56
1

The paradigm of object-oriented programming is to have a large collection of simple types that perform a independent unit of work accessed through public interfaces to create a network of communication that can convey tasks being performed in an abstract manner.

When you define a public interface your imposing a permanent constraint on the kind of work that can be performed. When your public interface starts concerning itself with implementing behavior that's not really related to it's focus you provide it in a separate class.

So yes, it's exactly as you described it. You'd have a bullet-proof protocol that wouldn't require any prior tests or consumers to be modified should the implementation of LoadIt() ever change.

What your test could tell you at the point of writing, though, is that by testing such an implementation of LoadIt() you'd be asserting several times over that it can load a ISAM file into the database because you'd have to set it up once that the directory path isn't configured; that it can discover all the ISAM files; that they're being represented in a suitable type. At the time of writing the tests you should observe a pattern emerging that hints that you should actually have a separate class that collects all the ISAM files and test that in isolation.

Well, in TDD, good code reflects good tests and good tests reflect good design principles. Definitely don't burden yourself with trying to get everything right immediately, but if at any point you become queasy and get this creepy feeling it may be evidence of a anti-pattern emerging somewhere nearby. Just write your tests freely and with practice you'll quickly be able to asses whether your taking on a good approach.

Update on comments

Well, what you definitely do apply extensively is building code coverage reports. There is no reason to introduce something that should be private as public for the sake of testing it, but the fact of the matter is that when you simply go for green lights to prove that you understand what is it that your doing, your code will probably be lacking design polish: it'll be tightly coupled into a complex structure where your public interfaces branch of doing varying amounts of work. Your tests will pound against them to assert they're exhibiting correct behavior under all circumstances until you simplify the code structure.

I've practiced testing on my own accord and made my share of mistakes. I'm not the best person to give you a concise educated answer. The problems your finding solutions for require approaches specific to your case. The single most hardest part of testing is when you reach the point that you don't know how and what to test, the circumstances that would force you to sway and abandon TDD and jump straight into the fray and just get it done. You have to think independently and develop a mode of hazard discovery test driving and damage control.

Having said that, I guess there is an aspect to applying TDD that you have to develop an approach for circumstances where your clueless as to how the implementation is supposed to turn out. We don't always operate within the boundaries of a well-defined architecture, working with specialized components that form groups of objects that have established patterns of communication that we fully understand and are comfortable to test against, sometimes you just need to start off with a mess.

Say that the implementation of LoadIt() was potentially volatile. What you do know is that all the tests up to that point are clear and simple. That means your can safely assume your public interfaces are properly designed, a bullet-proof protocol, which means you've isolated the development efforts away from the rest of the application.

I would say that on a per-situational basis anything goes. You then write tests to the best of your ability: write implementations and continue tracing your progress and refining the tests bit by bit. If you reach a point that your describing non-trivial code that seems to be closely related and resembling something that's well asserted, you refactor and extract it into an isolated component, come back to LoadIt(), simplify the tests and continue on, constantly reiterating.

For more involving examples, when your forced to venture deep into the unknown, you can start doing interactive programming or build small prototypes to get your bearings and work your way backwards and write tests as new insight arise, but even in such cases it should be that you have are containing the volatile code and surrounding every entry point: which is to say, never loose safe ground.

So in essence, you can cut to the chase and just go for green lights, but don't forget to apply at least some damage control when you leave code lying around that's not spacecraft-grade. At times you can be satisfied that the work being performed is at the very remote edge of your network with few consumers and defer from refactoring immediately; depending on your refactoring policy, you can wait to refine the design after you let everything settle in a bit or have more time, for the sake of maintainability, but definitely refactor if you ever revisit the code again and need to introduce new changes--it's just like putting flame to fire if you ignore that the existing tests require more attention.

  • Given the two practices: 1) don't test what's not public and 2) write the minimum to go green, do you allow yourself sometimes to write horrible code to get a green light, then refactor with nothing but the green light confirming your improvements work? I understand I want the goal's a lot of small types that communicate with each others' interfaces. But "minimum to be green" makes me think by-the-book TDDers don't attempt to get it right at first. Is that so? Most classes aren't like what I describe, but it's not like good design eliminates them all, is it? – MakeMinePanacea Dec 12 '11 at 3:56
  • I do appreciate your comments and love the wording of your first paragraph. I'm getting a tattoo of it. But my question isn't about how the code should end up; it's about how you get there, about the rhythms of TDD coding and the tools you apply. In particular, is there some other technique that strictly-public-testing TDDers apply mechanically to verify all that code that is not part of the contract gets exercised, when their tests aren't allowed to be designed to exercise it. – MakeMinePanacea Dec 12 '11 at 4:02
1

If you have no problem writing a good set of test cases, and no problem getting the tests to pass, so that each line of code is critical to at least one test, then you have no problem.

If you are having difficulty creating test cases to cover all the logic needed in some internal method, then you need to test that method separately. If it should not be a public method in it's current class, then make it a public method in some new class. At first you may think "It's a waste of time to create all those little classes". Soon you will find that all those little classes are quite handy, easy to understand, and are reused more than you might expect.

0

Well there's scope to it.

I consider testing private methods directly foolish. Internal only in very specific circumstances, instantiating via reflection, using a public method that that calls a factory pattern for instance.

Protected might be wise, given you could be adding to the ancestor class somewhere completely different.

The main reason TDD dies in commercialville is because it gets introduced by some evalgelist type, and they go mad with it. Let them do that and TDD will be "temporarily" postponed while we get some code out of the door, and then postponed again, while we get version 2 done beacseu we didn't have any flipping tests....

Get the basics in there, beef it up as you go along. If you are really fortunate, fill in any when you fix the bugs from UAT, BETA stages.

The developer talent in question is very simple. Only people who know the rules should be able to get away with breaking them.

  • The question is- for those people who confine testing to the public interface (and it sounds like you're among that number except for very specific circumstances) - does it effect the way you write code in the ways I outlined, or anything similar to them? Maybe you're saying that only someone who's gone made with it would do. Is it COMMON for people to write code they intend to refactor once the light is green, and that they know won't ever be directly tested. If my post doesn't just seem like a lot of random words, is there some technique I'm obviously missing? – MakeMinePanacea Dec 11 '11 at 20:43
  • Hmmm, I'd say that having tests for private methods, makes refactorring harder. Effectively you are breaking encapsulation, though only to test. Seeing as refactoring should not change functionality, public tests should do the job. If it's more re-enginneering then the public tests are going to change as well. So to me refactoring and testing proviate methods is a red herring. And yes it is common to right code to refactor, even more common to write code that should be refactored, but we never get round to it. – Tony Hopkinson Dec 14 '11 at 0:44
0

A tester does not care about HOW the black box works, they only care THAT it works.

This is the core issue with your notion on TDD. At every point, you show a deep interest in the internal workings, but that is something you (as a tester) should not care about. Similarly, as the driver of a car, you should only care whether the car works or not. When the car doesn't work, you shouldn't actually care why it doesn't work. What you do then is go to a mechanic. It is the mechanic's job to figure out why the car doesn't work.

You need to observe the two very different roles of tester/developer, just like how in my example the driver/mechanic are two very different roles.

My question is: If you write the most straightforward code possible to get the light green, and if you don't test internal methods, does it follow that from time to time you allow yourself to start off by writing a class that does far more than it should with a bunch of member variables you know will not survive refactoring? And wind up after refactoring with a bunch of methods that aren't explicitly tested?

As I get familiar with TDD, it feels like anyone would at least be tempted to test internal methods. And many practitioners flatly say you shouldn't. E.g. (of about 23 million results)

Never lost track of what your test aims to confirm.

The purpose of your tests is to confirm that the application is behaving as intended. Behavior is externally observable, and thus limited to the public (as opposed to internal/private) realm.

Example: you want an application that calculates the average of two numbers. I give you a compiled application and claim that this is what you want. You do not know the source code and I refuse to tell you.
You can still test if this application is correct simply by using it and confirming that its output is what you expect it to you. You don't need to know HOW the application achieves the correct outcome. Whether my application uses mathematical operations, googles it, or simply guesses the result; does not matter. As long as the application consistently renders the correct output, it should pass all of your tests.

Pedantically, my example is a UAT and not a unit test or integration test, but the principle remains the same. There is always some degree of "black box guts" that your test should not care about.

1) non-trivial logic winds up getting tested only indirectly, AND tested in the same calls as other non trivial logic.

Trivially readably logic does not warrant a test. Logic which trivially calls non-trivial logic very much needs to be tested.

2) you make methods public that you don't expect clients other than the SUT and the tests to call.

A public method which is only used by a test and not by production code should not exist. It makes no sense and it defeats the purpose of testing.

When you're testing, you want to test the actual code path. But if you have your test use a method that isn't actually used by your production code, the test result is no longer fully indicative of whether the application works as intended.

  • If there is an issue in this test-only public method, then your test will fail. But your production code would have passed.
  • If there is an issue in the "production counterpart" of what this test-only method provides, then your test will pass (as it's still using the test-only methods which does not contain this issue) but your production code will fail.
  • If there is no "production counterpart" to the test-only method, then what is the point of including this method in a test? You're testing it just to test it; without producing any actually meaningful results from the fact that it passes/fails the test.

But even though it sounds reasonable -- if I don't test internals I feel like I lose part of what helps keep the tests DRIVING the coding.

I hear this argument often, and I always think back to the same quote by Antoine de Saint-Exupéry:

“If you want to build a ship, don’t drum up the men to gather wood, divide the work, and give orders. Instead, teach them to yearn for the vast and endless sea.”

What you're advocating is that while the ship is being built, we are testing if people are hammering the right nails in the right planks. But that is excessive micromanagement. What you should instead test is:

  • Are these men intending to build a ship?
  • Do these men have the particular skills (hammering, sawing) to build a ship?

Bring it back to your case, what you're trying to do is use tests to confirm whether your developers are working. But tests are not a timetracking tool!

Look at it this way: you've defined the public interface (in a sense, that is what the analysis defines for you). The developers are tasked with implementing this interface. How they choose to achieve the adherence to the public interface is not yours to decide, and should not be policed using unit tests. This is the developers' personal resonsibility: they are the code smiths, they retain the autonomy to create the code that they deem to be the best solution to the problem.

If there's some internal logic that has to turn a String to a valid int32, int64 or decimal value depending on what's in the string and the type of some other object, I want to test that little bit of logic, not just find out that the whole method failed.

You are conflating two very different levels of detail. Those who write tests should not care that a string needs to be converted. That is an implementation detail that no one (other than the developers) should care about.

What you're doing is the developer's equivalent of a manager who micromanages. The manager should leave the micro tasks to their employees, and they should only focus on the macro tasks of managing the employees. The drawbacks to micromanaging are twofold:

  • By taking on additional micro tasks, they are taking away valuable time they could be spending on the macro tasks
  • They are acting as an obstacle for the employees who are responsible for the micro tasks (because 9 times out of 10 the micro employees have more applicable experience with the micro).

Instead, stick to your own role. When you test, you test if it works as expected. Only if it doesn't do you then assign this task to the developers. It is then up to the developers to figure out why it isn't passing the test.

While the tester and the developer can be the same physical person, they are two separate roles, each with their own responsibilities, and trying to blend the two only leads to a lack of accountability that is clearly tied to either role (if you want an off-topic analogy, this is exactly why there is such a strong focus on the separation of church and state, or of different governmental branches. Blending the separate roles muddies the water for both roles).

Or do they just recognize that as a failure point and write a test with input that will cause a failure pinpointing those few lines of code? If they do that it's testing the implementation though. It seems so natural to want to test that code in isolation, and verify it fails where it should, even though it's an implementation detail.

What you're calling "testing that code in isolation" is really just debugging. Part of the approach to debugging is to run the code and see what happens, which you can call a sort of "test" in semantical English. But it is not a test in the TDD sense.

When you say "write a test with input that will cause a failure pinpointing those few lines of code", this implies that the test writer has accurate knowledge of the internal code, which they should not have (or at least not base their tests on).

This is the key part of where you're wrong: the tester should not care about HOW the black box works, they only care THAT it works.

Say you want to put data from an ISAM file into a database. There is one public method to the envisioned consumer of the code: public void LoadIt(SqlConnection c). "Loadit" has a bunch of other dependencies that will be resolved using configuration files and environment variables: some code has to determine that it can find the name of the folder where the ISAMs live; to find, load and parse the schema file for the isams using the ISAM vendor's DDL library; so forth+so on. But none of that is of any interest to any envisioned calling code.

Okay, so let's san there is an IsamToDatabaseLoader class which has the Loadit(SqlConnection c) method.

When you are (unit) testing the IsamToDatabaseLoader, you are testing its behavior regardless of what the exact dependencies are. If anything, you should be mocking any dependency that's anything more than a simple primitive value or data class.

The key thing you need to understand here is that IsamToDatabaseLoader is not responsible for the correctness of the dependencies it has been provided with. And if it's not responsible for it, then it shouldn't be blamed for any dependency being incorrect. And if it can't be blamed for it, then you shouldn't be testing it!

However, what the IsamToDatabaseLoader class is responsible for, is how it chooses to behave in case something goes awry. Suppose you give it a bad connectionstring. What is it you want to happen, what do you consider correct behavior?

  • Should the class throw an exception? (any exception will do)
  • Should the class throw a specific exception? (e.g. BadConnectionstringException)
  • Should the class log an error?
  • ...

Notice that the things are listed are all public behavior. We don't care what happens inside of the IsamToDatabaseLoader class, we only care what comes out of it.

Depending on what you want to happen, you will have to write the tests that confirm that this happens. Let's say that you want a specific exception (BadConnectionstringException).

You could then write a unit test which gives IsamToDatabaseLoader a bad connection string, and catches any potential exception. The test's assertions seem simple enough:

  • If no exception is encountered, the test fails.
  • If the encountered exception is a BadConnectionstringException, the test passes.
  • If the encountered exception is not a BadConnectionstringException, the test fails.

I think you'll agree that this is a simple and clean test.

But now I have one question for you: how does IsamToDatabaseLoader decide to throw a BadConnectionstringException? You don't actually know.

That's the key point. You understood the requirements (throwing the needed exception), you understood how you needed to confirm whether this requirement was met, you were able to agree that the proposed tests is a valid way to confirm that this requirement is met, and at no point did you care about HOW this requirement was going to be met.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.