Using JUnit to test privately contained List?

K

kk_oop

Hi.

In Java, we have a class MyClass that privately contains an ArrayList
as an attribute called myList. We were looking for a way to enable a
JUnit test case class to call MyClass methods, then check the state of
myList.

Here's a what we are considering:

a. Make a protected method in MyClass called getMyList that returns
myList and performs lazy instantiation (it'll only instantiate myList
the first time it is called but will return myList everytime).
MyClass should only access myList through this getter.

b. Make a test version of MyClass called something like TestMyClass.
Give that a public version of the myList attribute called myTestList.


c. Override getMyList so it will use myTestList instead of myList.

d. The test case class will instantiate TestMyClass, and call its
methods polymorphically through an variable of type MyClass.

e. When the test case wants to check the list values, it will just
cast MyClass back to TestMyClass and access the public myTestList.

The upside of this is that myList gets to remain private in the
production code. The downside is that we are not actually using
myList in the test runs. My thought was that if myTestList is the
same type as myList and the override version of getMyList does exactly
the same thing as the production code's getMyList (except for acting
on the test version of myList), this downside becomes negligible.

Note that we would define TestMyClass as an inner class of the JUnit
test case so it will not have a .java file generated for it.

Any thoughts on this approach? Good? Bad? Recommend a different
approach?

Thanks for any feedback!

Ken
 
M

Marcin Grunwald

Hi.

In Java, we have a class MyClass that privately contains an ArrayList
as an attribute called myList. We were looking for a way to enable a
JUnit test case class to call MyClass methods, then check the state of
myList.

Here's a what we are considering:

a. Make a protected method in MyClass called getMyList that returns
myList and performs lazy instantiation (it'll only instantiate myList
the first time it is called but will return myList everytime).
MyClass should only access myList through this getter.

b. Make a test version of MyClass called something like TestMyClass.
Give that a public version of the myList attribute called myTestList.


c. Override getMyList so it will use myTestList instead of myList.

d. The test case class will instantiate TestMyClass, and call its
methods polymorphically through an variable of type MyClass.

e. When the test case wants to check the list values, it will just
cast MyClass back to TestMyClass and access the public myTestList.

The upside of this is that myList gets to remain private in the
production code. The downside is that we are not actually using
myList in the test runs. My thought was that if myTestList is the
same type as myList and the override version of getMyList does exactly
the same thing as the production code's getMyList (except for acting
on the test version of myList), this downside becomes negligible.

Note that we would define TestMyClass as an inner class of the JUnit
test case so it will not have a .java file generated for it.

Any thoughts on this approach? Good? Bad? Recommend a different
approach?

Thanks for any feedback!

Ken

You shouldn't use private members in tests. Tests should be write for class
interfaces.
But theory and practice sometimes differ a lot :)
By reflection you can get to object private members, in that way you needn't
to modify your class for testing.
 
D

Daniel Tryba

Any thoughts on this approach? Good? Bad? Recommend a different
approach?

IMHO that it would be bad (atleast not good practice). This does however
sounds like a job for reflection.
 
K

kk_oop

Marcin said:
(e-mail address removed) wrote:


You shouldn't use private members in tests. Tests should be write for class
interfaces.

What is the alternative? I'm hoping to avoid adding methods to the
production code just to make the private member visible to a test. One
of our challenges is that our code is highly algorithmic, so a single
public method will manipulate a lot of internal state to provide a
result. If I only test the public method and it fails, it will be
difficult to hunt down where it failed without testing internal state
along the way.

Note that this approach would just be done for our class level unit
testing. We would still be using all production code for our higher
level integration testing.
By reflection you can get to object private members, in that way you needn't
to modify your class for testing.

Could you describe how this would work?

Thanks!

Ken
 
M

Marcin Grunwald

What is the alternative? I'm hoping to avoid adding methods to the
production code just to make the private member visible to a test. One
of our challenges is that our code is highly algorithmic, so a single
public method will manipulate a lot of internal state to provide a
result.
I assume that your class is quite complicated and large, maybe you should
split it into a few parts and test that parts separately. I know that it
isn't always possible but I think it is worth of trying. But do it only for
better design of your code, not just for testing; good design is more
important than testing.
If I only test the public method and it fails, it will be
difficult to hunt down where it failed without testing internal state
along the way.
You should write tests just for check if it is OK, not for hunting down
bugs. It is always simpler and faster to take debugger if test has failed,
than to write complicated tests. When you test what class is doing (just
interface) not how it is doing (private members) you can easily modify how
it is working and still have correct tests (for example change ArrayList to
LinkedHashMap or HashSet).
Could you describe how this would work?

Example:
MyClass myClass = new MyClass();
try {
Field field = MyClass.class.getDeclaredField("myList");
field.setAccessible(true);
//myList is reference to private member of myClass object
ArrayList myList = (ArrayList)field.get(myClass);
} catch(NoSuchFieldException ex) {
ex.printStackTrace();
} catch(IllegalAccessException ex) {
ex.printStackTrace();
}
 
A

Andrew McDonagh


Hi Ken,
In Java, we have a class MyClass that privately contains an ArrayList
as an attribute called myList. We were looking for a way to enable a
JUnit test case class to call MyClass methods, then check the state of
myList.

Lets start with a different perpective.

When the public methods of MyClass are invoked, they do something. Some
of them will do something with the ArrayList, others may not.

In either case, its preferable to test the public methods of a class and
use the output from those methods to verify. In the case of MyClass, one
or more public method may manipulate the ArrayList myList, however,
there most be some existing way to verify those changes.


Now these means of verification don't have to be a getter on the
MyClass, it could well be some other interaction that results from the
public methods being invoked, e.g. Listeners being called back, events
generated, etc.

for instance...

MockMyClassListener mockListener = new MockMyClassListener();

MyClass myClass = new MyClass(mockListener);
myClass.addSomethingToMyList("A String");

assertTrue("Listener not called back", mockListener.wasCalled());

or another example ....

MyClass myClass = new MyClass();
myClass.doSomethingWith("A String");

assertTrue("Listener not called back", myClass.hasDoneSomethingWith("A
String"));


So basically today, there must either be
1) something happening when the public methods are called
2) some way of inspecting the state of MyClass after the public methods
are called.

If there isn't then whats the point in having the public method?

So on to your ideas...
Here's a what we are considering:

a. Make a protected method in MyClass called getMyList that returns
myList and performs lazy instantiation (it'll only instantiate myList
the first time it is called but will return myList everytime).
MyClass should only access myList through this getter.

b. Make a test version of MyClass called something like TestMyClass.
Give that a public version of the myList attribute called myTestList.

c. Override getMyList so it will use myTestList instead of myList.

d. The test case class will instantiate TestMyClass, and call its
methods polymorphically through an variable of type MyClass.

e. When the test case wants to check the list values, it will just
cast MyClass back to TestMyClass and access the public myTestList.

The upside of this is that myList gets to remain private in the
production code. The downside is that we are not actually using
myList in the test runs. My thought was that if myTestList is the
same type as myList and the override version of getMyList does exactly
the same thing as the production code's getMyList (except for acting
on the test version of myList), this downside becomes negligible.

This is good techique, one which we use alot when testing classes that
need to do something which is hard to test. For example, if testing a
class that sends messages via RPC, we'd normally create a Mock object
that has the same interface as the real RPC implementation, and get the
class under test to use the mock instead of the real RPC one.

However... the only (potential) problem with creating a protected getXXX
method which your TestableDerived class overrides, is that the
production class must ALWAYS use the getXXX method when wanting to use
the arraylist. Its putting a risk into the class that another developer
will modify MyClass and forget to use the getXXX() method, and reference
the member var arraylist directly.

But don't worry, this is easily solved by slightly changing the
protected method.

In effect the protected method is really a factory method which is
creating an instance of an ArrayList. So if we name it as such, and only
call it once to create mylist (usually at construction time), then the
MyClass internals are free to acces myList list you normally would.

e.g.

public class MyClass {
public MyClass() {
// do usual stuff.
m_myList = createMyList();
}

protected ArrayList createMyList() {
return new Arraylist();
}

public void doSomethingWith(String aString) {
m_myList.add(aString);
}

private List m_myList = null;
}


public class TestableMyClass {

protected ArrayList createMyList() {
return m_listToUseWhenTesting;
}

public List m_listToUseWhenTesting = new Arraylist();

}
Note that we would define TestMyClass as an inner class of the JUnit
test case so it will not have a .java file generated for it.

Yes this is where we normally put the TestableMyClass as well, at least
until another test suite needs to mock out the same behaviour, then we
make the TestableMyClass a top-level class. It still lives in the
UnitTest source tree though.


Its worth noting that sometimes, using a factory method may not be
appropriate, in which case we use an AbstractFactory pattern to achieve
the same results. The AbstractFactory also means we do not have to
create any TestableDerivedClass in our unit tests, we simply change the
AbstractFactory to return our mockObject instead of a real object.
 
A

Andrew McDonagh

Marcin said:
You shouldn't use private members in tests. Tests should be write for class
interfaces.
Agreed.

But theory and practice sometimes differ a lot :)

very true.
By reflection you can get to object private members, in that way you needn't
to modify your class for testing.

I'd never suggest anyone uses reflection to aid unit testing. There are
easier ways that do not incur the tight coupling between unit test code
and production code.

If we used reflection to find the myList object within the MyClass
object, then our tests would have to change should we ever decide to
rename the member or change its type, or any number of other refactoring
that may happen over the life time of MyClass.

Its simply a too brittle and bad means of testing.
 
A

Andrew McDonagh

Marcin said:
I assume that your class is quite complicated and large, maybe you should
split it into a few parts and test that parts separately. I know that it
isn't always possible but I think it is worth of trying.

Ken, this is good advice here from Grundig.

This is a very good refactoring to use when faced with code as you
describe. The code is telling you (because its hard to find where the
problem is) that the class under test is doing to much.

By extracting some of the logic out of the Class into helper classes, we
can test those helper classes directly. Which means we will find out
much quicker if the problem cause the MyClass tests to fail is in the
extracted helper class of MyClass it self.

But do it only for better design of your code, not just for testing; good design is more
important than testing.

A good design will be easy to test, a bad design will be hard to test -
testing is always more important that design. This is true whether we
are unit testing a class, functionally testing a group of classes or
subsystem or acceptance test the application.
You should write tests just for check if it is OK, not for hunting down
bugs. It is always simpler and faster to take debugger if test has failed,
than to write complicated tests.

Its a good idea to write a unit test for a bug that you have found (with
or without the debugger), so we know when we have fixed the bug - as the
test will pass. Also, the bug test will then live on making sure we
don't re-introduce the bug.
 
M

Marcin Grunwald

Andrew said:
A good design will be easy to test, a bad design will be hard to test -
testing is always more important that design. This is true whether we
are unit testing a class, functionally testing a group of classes or
subsystem or acceptance test the application.

I mean that you shouldn't modify code/design just for testing. For example:
it isn't good practice to change class member from private to public
because you can't test it. I prefer good design with tight coupling between
unit test code and production code, than bad design with nice unit tests.
Its a good idea to write a unit test for a bug that you have found (with
or without the debugger), so we know when we have fixed the bug - as the
test will pass. Also, the bug test will then live on making sure we
don't re-introduce the bug.

Yes, but information that test has failed is enough. Sometimes you can also
easily tell why test has failed, but it isn't a good idea to check whole
internal state trying hunt down where and why it has failed (you have
debugger for that purpose). One test case in one test method, not many test
cases in one test method.
 
M

Marcin Grunwald

Andrew said:
very true.


I'd never suggest anyone uses reflection to aid unit testing. There are
easier ways that do not incur the tight coupling between unit test code
and production code.

If we used reflection to find the myList object within the MyClass
object, then our tests would have to change should we ever decide to
rename the member or change its type, or any number of other refactoring
that may happen over the life time of MyClass.

Its simply a too brittle and bad means of testing.

I know that reflection in testing isn't good practice and I try to avoid it
as I can, but sometimes it is really the best solution.

For example I have written quite simple Component in Swing composed from a
few basic private components (text, buttons, labels, ...). It has public
interface, but on my monitor not in the code :) How to test it?
1. Emulate user actions (mouse and keyboard) and check image on the screen.
Difficult, and even the simplest change (for example button color), would
break all tests.
2. Emulate whole Swing environment, set initial state, emulate user actions
and check result state. It's too much work to write emulate whole Swing
environment. The worst is that Swing emulation would work as I think it
should, not as real Swing (I'm just a human and I can't know every detail
about Swing).
3. Set initial internal state (by reflection), emulate user actions, check
result internal state (by reflection).

I prefer no. 3.
 
G

Guest

I mean that you shouldn't modify code/design just for testing. For example:
it isn't good practice to change class member from private to public
because you can't test it. I prefer good design with tight coupling between
unit test code and production code, than bad design with nice unit tests.

It depends on the type of change. Your example of changing protection
is a good example of a change not to make. Splitting a class up so it
can be easily testing is an example of a change I would make.

John
 
A

Andrew McDonagh

Marcin said:
Andrew McDonagh wrote:




I know that reflection in testing isn't good practice and I try to avoid it
as I can, but sometimes it is really the best solution.

For example I have written quite simple Component in Swing composed from a
few basic private components (text, buttons, labels, ...). It has public
interface, but on my monitor not in the code :) How to test it?
1. Emulate user actions (mouse and keyboard) and check image on the screen.
Difficult, and even the simplest change (for example button color), would
break all tests.
2. Emulate whole Swing environment, set initial state, emulate user actions
and check result state. It's too much work to write emulate whole Swing
environment. The worst is that Swing emulation would work as I think it
should, not as real Swing (I'm just a human and I can't know every detail
about Swing).
3. Set initial internal state (by reflection), emulate user actions, check
result internal state (by reflection).

I prefer no. 3.

I'd prefer number

4. The Humble Dialog approach. That way we can still verify the
components look and feel, emulate user actions and test the state of the
view, without resorting to reflection.


5. TFUI - see posting by Phlip
http://industrialxp.org/community/bin/view/Main/TestFirstUserInterfaces
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,755
Messages
2,569,536
Members
45,014
Latest member
BiancaFix3

Latest Threads

Top