Is the visibility of tested methods important?

We are having quite a discussion lately within Typemock about theimage importance of the visibility of tested methods.

For example, should it really be harder to test a private method?
If it is harder, then many developers will opt to change the scope of the tested method to public, even if business wise, it should stay private. If Visual Studio allowed using private methods from test projects (as it does with internal methods) then developers can choose to invoke private methods and the test will not pollute the design. (I know that Visual Studio does have a private accessor, but it is still awkward.)

The same goes with mocking or isolating. During the test, there are many times that we don’t care about some calls and want to fake those out.

Should we care if these calls are static, public, protected or private? If we just don’t care about those interactions (known as stubbing) should we care about the visibility of these methods?

On one hand we are saying that it does matter and that we need to explicitly say this when arranging our tests.

example of a new API with a difference between scopes

// fake a public instance method
RealLogger fake = MyTest.Make.FakeInstance<RealLogger>();
MyTest.WhenCalled(()
=>fake.Log("")).WillReturn("sfsfs");

// fake a non-public instance method
MyTest.NonPublic.WhenCalled(fake,"PrivateInstanceMethod").WillReturn("l");

// fake a public static method
MyTest.Static.WhenCalled(() => MyType.StaticMethod()).Fake();

// fake a non-public static method
MyTest.NonPublic.Static.WhenCalled("PrivateStaticMethod").Fake();

On the other hand we should keep a minimum API and that will lead us to.

// fake accessable methods - can use private accesors too
RealLogger fake = MyTest.Make.FakeInstance<RealLogger>();
MyTest.WhenCalled(()
=>fake.Log("")).WillReturn("sfsfs");
MyTest.WhenCalled(()
=> MyType.StaticMethod()).Fake();

// fake any method
MyTest.WhenCalled(fake,"PrivateInstanceMethod").WillReturn("l");
MyTest.WhenCalled(
"PrivateStaticMethod").Fake();

My current take is that the visibility issue is a technical one and not a real API issue, I think that the biggest issue developers have is over-specified tests.

What would make developers fall into the put of success is the following:

// I don't care about any calls to HttpWebRequest in this test
// and all methods will return more Fakes so that there is no NPE
MyTest.Fake.Instance<HttpWebRequest<(Members.ReturnFakes);

This will fake all calls to HttpWebRequest, and will automatically recursively fake all ‘chained’ methods, so  HttpWebRequest.GetResonse() will return a fke Response, like a recursive NullObject Desgin.

What is your take?

  • ulu

    http://blog.typemock.com/2008/07/is-visibility-of-tested-methods.html
    Folks usually recommend moving such methods to another class and making them public; I’d say, either make them public unless you have really solid reasons not to. In which case, you don’t want to test them. A public method is still another way of using your code, which makes your customers happier.

    For example, in Ivonna, I’ve got a couple of utility methods not related to testing at all, but I wanted to test them separately, so I made them public. Would be great if someone finds them useful, and they are harmless enough so they can’t be really misused.

    On the other hand, I’ve spent countless hours of Reflectoring over System.Web in search of suitable public classes and methods in order to make Ivonna work (I’m quite proud that I don’t use a single private member in it). That’s because somebody has decided that the framework should be used in some particular way, and blocked all unexpected entries. It’s like putting something in a cage instead of letting it run in the wild.

  • Eli Lopian

    Although this is not what I was talking about, the issue that you brought up is a known issue.
    Some folks saying that you should make them public, while others (including mstest and mbunit) having ways to simply activate these methods that you have decided to be private.

    That said, having too many public methods has a price too – hard to learn, discover and prone to usage errors (you don’t have to check for valid inputs to private methods, but do have to for public ones)

    But I was really talking about the visibility of ISOLATING (stubbing out), not a way to use the code, but a way to isolate it.

    Here there is a balance between having many API’s (harder to learn) that will make the developer conscious and explicit of what they are isolating (and brittle when these change) to having one API (easier to learn) but implicit of the visibility of the methods the are insolating (perhaps loosing some flexability as you pointed out).

  • ulu

    Well, I forgot to add that everything I had said applies to isolating private methods as well.

    If a method has some semantic meaning (not just duplicated piece of code refactored), I usually want to test it isolated from the others, and when testing other parts, I don’t want it to be tested along, so I stub it out. Such a meaningful method deserves to be publicly available, unless there are serious reasons to keep it private. So, for me, these things are closely related, testing, isolating, and making them public.
    The noisy API can be a problem indeed, but there are ways to deal with that.

    That said, it’s always nice to have options. Especially when it comes to legacy code, like your example
    with HttpContext.

    Should it be harder to stub the privates? It refers to a popular opinion that certain restrictions of certain mock frameworks lead to a better design. While I do like the design decisions that are forced by such frameworks, I strongly believe that this is largely a matter of personal preference, and these decisions should not be made blindly, much less by framework restrictions. There are times when I need guidance, but I’d rather read a blog or two than blindly put myself in a restricted situation.

    This is also a matter of personal preference, of course.

    The bottom line is, I don’t think that stubbing out private members is a good idea, but I’m happy that such an option exists.

  • Paulo Morgado

    Hi ulu,

    I’ll have to say that neither System.Web or System.Windows.Forms are good examples of frameworks or extensibility (too many private and internal things that prevent you from replacing behavior/functionality that is announced as replaceable).

    That being said, what I love about Isolator is that it doesn’t constrain my design (and bypassing validation, like Eli mentioned, is just one of the things I want to keep private) and allows me to make public only what I want and, at the same time, allows me to unit test (I call unit testing every method on its own for its all code paths) everything.

    Let me remind you that protected and internal members although not being public are not fully private. And I do like to test them.

    In conclusion, I like the way MSTest (only really tried VS2005 so far) exposes non public members and how Isolator handles it. And I wouldn’t like to loose it.

  • Lior Friedman

    Just to make things clear.
    In any case the Isolator will support handling of all types of methods, including private,protected and internal methods.

    The real debate we are having is whether or not the programmer should explicitly state that he going to Mock/test a private method or not. Some of us think that since tempering with privates although needed,is somewhat out of main stream, we would like those reading the test to be clear about that (therefore the need for the NonPublic statement)

  • ulu

    Now we are getting more to the point ;)

    Personally I’d like to look at my test and immediately see that here I mocked a private (protected, internal) method. I, however, wouldn’t want my test to break if I ever make this method public. So, I’d vote for the syntax that is valid for both public and non-public methods.

    Let’s say that’s my fraction of a vote, since I rarely do such things.

TOP