How do I unit test this?
-
Imagine you have the following code.
public void X()
{
A();
B();
C();
// etc...
}Assuming that A, B, and C etc. are private function members, how would you write a unit test for this? One way is to make them protected instead of private, but then you are changing the application code. Is there a way to do it without modifying the code above? Visual Studio can also setup private accessors, but I think they're quite ugly. Or are they they best option?
Member 4487083 wrote:
One way is to make them protected instead of private, but then you are changing the application code.
This is bad, you shouldn't change your code specifically for testing. When writing test code, you might discover shortfalls in the code, and of course the code should be changed in that case, but not just because you need to test.
Member 4487083 wrote:
Is there a way to do it without modifying the code above?
You can call by reflection. (see last point)
Member 4487083 wrote:
Visual Studio can also setup private accessors, but I think they're quite ugly. Or are they they best option?
I don't know if they are the best, that's a matter of opinion I suppose. They are a big improvement on doing it refelctively. Hope this helps!
Antoine de Saint-Exupery: Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away.
-
Imagine you have the following code.
public void X()
{
A();
B();
C();
// etc...
}Assuming that A, B, and C etc. are private function members, how would you write a unit test for this? One way is to make them protected instead of private, but then you are changing the application code. Is there a way to do it without modifying the code above? Visual Studio can also setup private accessors, but I think they're quite ugly. Or are they they best option?
Depends what tool you are using for unit testing. With VS, right-click and create unit tests for each method A, B, and C. Create a unit test for X also if it is dependent on results from any of the other methods.
I know the language. I've read a book. - _Madmatt
-
Imagine you have the following code.
public void X()
{
A();
B();
C();
// etc...
}Assuming that A, B, and C etc. are private function members, how would you write a unit test for this? One way is to make them protected instead of private, but then you are changing the application code. Is there a way to do it without modifying the code above? Visual Studio can also setup private accessors, but I think they're quite ugly. Or are they they best option?
Here are a couple of solutions, none of them is perfect though, as you basically want to extend an existing class or access all its members, even the private ones, without changing the class, and without allowing others to access them. 1. change the private methods to public ones. 2. add one method for testing purposes, give it one or more parameters, and let it dispatch to the methods that need testing. 3. add your test code to the same class; partial classes are helping here. 4. use reflection to access everything that isn't public. 5. create a new class that holds the test code, and add a copy of the original class in to it. You need to chamge the class name and hence the constructor(s) for this. 6. my personal preference: change all private attributes to protected ones; then derive a class that simply adds the test code and can access everything; you probably would keep the test class in a separate assembly which you do not distribute. :)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read code that is properly formatted, adding PRE tags is the easiest way to obtain that.
All Toronto weekends should be extremely wet until we get it automated in regular forums, not just QA.
-
Member 4487083 wrote:
One way is to make them protected instead of private, but then you are changing the application code.
This is bad, you shouldn't change your code specifically for testing. When writing test code, you might discover shortfalls in the code, and of course the code should be changed in that case, but not just because you need to test.
Member 4487083 wrote:
Is there a way to do it without modifying the code above?
You can call by reflection. (see last point)
Member 4487083 wrote:
Visual Studio can also setup private accessors, but I think they're quite ugly. Or are they they best option?
I don't know if they are the best, that's a matter of opinion I suppose. They are a big improvement on doing it refelctively. Hope this helps!
Antoine de Saint-Exupery: Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away.
keefb wrote:
you shouldn't change your code specifically for testing
I don't quite agree. One should design for testability, i.e. take precautions so the class is testable (testability is one of the goals, same as correctness, completeness, performance, maintainability, ...); if you have overlooked testability while designing, then you'll have to modify the class later on. :)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read code that is properly formatted, adding PRE tags is the easiest way to obtain that.
All Toronto weekends should be extremely wet until we get it automated in regular forums, not just QA.
-
keefb wrote:
you shouldn't change your code specifically for testing
I don't quite agree. One should design for testability, i.e. take precautions so the class is testable (testability is one of the goals, same as correctness, completeness, performance, maintainability, ...); if you have overlooked testability while designing, then you'll have to modify the class later on. :)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read code that is properly formatted, adding PRE tags is the easiest way to obtain that.
All Toronto weekends should be extremely wet until we get it automated in regular forums, not just QA.
Luc Pattyn wrote:
I don't quite agree
Which is why I wrote this: When writing test code, you might discover shortfalls in the code, and of course the code should be changed in that case, but not just because you need to test. Which was (possibly very unclear) shorthand for the reasons you wrote in your replying post :-) [Edit] I've just read your thing about why Q&A Sucks. I totally agree.
Antoine de Saint-Exupery: Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away.
modified on Sunday, February 14, 2010 12:01 PM
-
Imagine you have the following code.
public void X()
{
A();
B();
C();
// etc...
}Assuming that A, B, and C etc. are private function members, how would you write a unit test for this? One way is to make them protected instead of private, but then you are changing the application code. Is there a way to do it without modifying the code above? Visual Studio can also setup private accessors, but I think they're quite ugly. Or are they they best option?
I would never change a private method to public or anything else for testing. I'd always test it through calls to the public[1] members of the class because that's the way that they were designed to be used. If you make them public and test them directly then you have access to them in ways that the other classes and modules don't and you can stress them in ways that won't be possible once deployed. When unit testing a class I access it only in ways it was designed to be used and if code private that means that it was not designed to be used by anything other than the other code in the class. Take the following class:
public class TestClass
{
private void PrivateMethod(bool someBool)
{
if (someBool)
{
// do something
Console.WriteLine("Reachable Code");
}
else
{
// do something else
Console.WriteLine("Dead Code");
}
}public void PublicMethod(double x) { // do some stuff // Here we're going to do some calculation with the input and use the result to determine what to pass // PrivateMethod(...). I'm using Math.Abs just as an example. double calculationResult = Math.Abs(x); PrivateMethod (calculationResult >= 0.0); // do some more stuff }}
The private method has different behavior based on its parameters. If you make that public and test it directly then you can stress both cases but that's not how the code was designed! If you examine the code you'll see that the only place it's called from is from the one public method and that method will always pass it a value of
true. What this means is that private method doesn't need to have different behaviors, it doesn't need to check its input and it doesn't even need any input parameters. The parameter,ifstatement andelseblock are all dead code and should be removed. At least this is what it looks like at first glance; what if the fact that the method can only be passed one value a bug? What if the calculation is wrong? If you make the private method public just for the sake of testing then this question might never be asked. Granted, this is a contrived example that shows a minor inefficiency but it highlights how easy it is to hide bugs private code if you tweak for testing. If you have a class where everything is private then what's the point of the class? If you have private code that isn't callable in one way or another from any public methods then what's the point of the code? If code can't -
I would never change a private method to public or anything else for testing. I'd always test it through calls to the public[1] members of the class because that's the way that they were designed to be used. If you make them public and test them directly then you have access to them in ways that the other classes and modules don't and you can stress them in ways that won't be possible once deployed. When unit testing a class I access it only in ways it was designed to be used and if code private that means that it was not designed to be used by anything other than the other code in the class. Take the following class:
public class TestClass
{
private void PrivateMethod(bool someBool)
{
if (someBool)
{
// do something
Console.WriteLine("Reachable Code");
}
else
{
// do something else
Console.WriteLine("Dead Code");
}
}public void PublicMethod(double x) { // do some stuff // Here we're going to do some calculation with the input and use the result to determine what to pass // PrivateMethod(...). I'm using Math.Abs just as an example. double calculationResult = Math.Abs(x); PrivateMethod (calculationResult >= 0.0); // do some more stuff }}
The private method has different behavior based on its parameters. If you make that public and test it directly then you can stress both cases but that's not how the code was designed! If you examine the code you'll see that the only place it's called from is from the one public method and that method will always pass it a value of
true. What this means is that private method doesn't need to have different behaviors, it doesn't need to check its input and it doesn't even need any input parameters. The parameter,ifstatement andelseblock are all dead code and should be removed. At least this is what it looks like at first glance; what if the fact that the method can only be passed one value a bug? What if the calculation is wrong? If you make the private method public just for the sake of testing then this question might never be asked. Granted, this is a contrived example that shows a minor inefficiency but it highlights how easy it is to hide bugs private code if you tweak for testing. If you have a class where everything is private then what's the point of the class? If you have private code that isn't callable in one way or another from any public methods then what's the point of the code? If code can'tSome interesting comments here. I don't completely agree with what you are saying about only testing public members. Firstly a unit test is supposed to test a unit eg. a single method, and if you don't call private members directly then it's not really unit testing. I also like to test things thouroughly which may include using things in a way that they weren't necessarily supposed to be used. Dead code is potentially a bug waiting to happen IMHO. Imagine the code is changed in the future and the dead code comes to life. People will think the dead code has been tested and will trust is, but in reality it has probably never been executed and may have bugs in it. Statistically there is a higher chance of finding bugs around redundant code so I would want to know about the dead code. If it wasn't needed I would delete it or at least comment it out. I agree that it's not a good idea to change the accessiblity for testing, and I don't plan to do this. However, in the real world it may be worth making a few modifications for testing. In the past I have made a change that allowed me to change the database connection string for testing, and in my opinion, it didn't really reduce the quality of the code.
-
Here are a couple of solutions, none of them is perfect though, as you basically want to extend an existing class or access all its members, even the private ones, without changing the class, and without allowing others to access them. 1. change the private methods to public ones. 2. add one method for testing purposes, give it one or more parameters, and let it dispatch to the methods that need testing. 3. add your test code to the same class; partial classes are helping here. 4. use reflection to access everything that isn't public. 5. create a new class that holds the test code, and add a copy of the original class in to it. You need to chamge the class name and hence the constructor(s) for this. 6. my personal preference: change all private attributes to protected ones; then derive a class that simply adds the test code and can access everything; you probably would keep the test class in a separate assembly which you do not distribute. :)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read code that is properly formatted, adding PRE tags is the easiest way to obtain that.
All Toronto weekends should be extremely wet until we get it automated in regular forums, not just QA.
Luc Pattyn wrote:
my personal preference: change all private attributes to protected ones; then derive a class that simply adds the test code and can access everything; you probably would keep the test class in a separate assembly which you do not distribute.
That's quite interesting because everybody else is saying that this is a bad idea. I think it can be quite neat in some places, but I wouldn't want to fill the main application with lots of unnecessary protecteds.
-
Some interesting comments here. I don't completely agree with what you are saying about only testing public members. Firstly a unit test is supposed to test a unit eg. a single method, and if you don't call private members directly then it's not really unit testing. I also like to test things thouroughly which may include using things in a way that they weren't necessarily supposed to be used. Dead code is potentially a bug waiting to happen IMHO. Imagine the code is changed in the future and the dead code comes to life. People will think the dead code has been tested and will trust is, but in reality it has probably never been executed and may have bugs in it. Statistically there is a higher chance of finding bugs around redundant code so I would want to know about the dead code. If it wasn't needed I would delete it or at least comment it out. I agree that it's not a good idea to change the accessiblity for testing, and I don't plan to do this. However, in the real world it may be worth making a few modifications for testing. In the past I have made a change that allowed me to change the database connection string for testing, and in my opinion, it didn't really reduce the quality of the code.
Member 4487083 wrote:
only testing public members
I'm not saying don't test them, I'm saying don't call them directly to test them. To test the private method in my example I'd write test cases that started with the public method and manipulate the inputs in order to fully exercise the private one.
Member 4487083 wrote:
a unit eg. a single method
I'd call a unit a class, not a method or a property. When I link to a third party DLL to import some functionality, I'm importing the classes not individual methods.
Member 4487083 wrote:
which may include using things in a way that they weren't necessarily supposed to be used
Of course, but I always honor the contract of the classes that I'm testing. If a class has a method that takes a string parameter I'll throw everything I can think of into that parameter to stress the method but if you go outside the contract then what are you testing? If something is private then it's not supposed to be accessible by anything other than things internal to the class so it should be tested only by way of things internal to the class.
Member 4487083 wrote:
Dead code is potentially a bug waiting to happen IMHO
I agree completely.
Member 4487083 wrote:
it may be worth making a few modifications for testing
I agree almost completely. Testability is something that can/should be built in so that no modifications are required to do it. The connection string is a good example. Ideally those should be built to be configurable from the beginning - it shouldn't have to be added on later just to accommodate testing. Still, this is something completely different from changing the accessibility level of members to facilitate testing. Making something configurable is an enhancement that promotes flexibility and testability; making something unnecessarily public makes it more testable but introduces security risks. More to the point, making a connecting string configurable doesn't change the contract of the class but changing something from private to public does.
:badger:
-
Luc Pattyn wrote:
my personal preference: change all private attributes to protected ones; then derive a class that simply adds the test code and can access everything; you probably would keep the test class in a separate assembly which you do not distribute.
That's quite interesting because everybody else is saying that this is a bad idea. I think it can be quite neat in some places, but I wouldn't want to fill the main application with lots of unnecessary protecteds.
Luc Pattyn wrote:
change all private attributes to protected ones
that was exagerated, it should read: change private attributes to protected ones as required... Yes, making everything protected (or public) is not highly regarded as it opens the door more than may be required by the app itself. I listed some possibilities and already said they all have some drawback; do you have a better solution? :)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read code that is properly formatted, adding PRE tags is the easiest way to obtain that.
All Toronto weekends should be extremely wet until we get it automated in regular forums, not just QA.