tags:

views:

138

answers:

7

Hi,

how would one test a method that returns a complicated object that does not allow access to its fields. Consider this snippet--

public ResultState getResultState(){ 
  ResultState resultState = new ResultState();
  // logic //
  this.resultState = resultState; //set parent class field
  return resultState;
}

ResultState is an object that does not allow much of its methods and fields. I am clueless as to how else to test such a method other than:

assertNotNull(object.getResultState()) //the return ResultState is used by other class methods

My question is general, but I am hoping that it shows how desperate I am to get a hint about how to go on testing... thanks for help

+10  A: 

Well, what's important about the result? Presumably it can do something or it would be a useless result. What would differentiate working code from broken code in this case?

(I'm not trying to make light of the problem - sometimes it really is very hard to keep tight encapsulation but test effectively. Sometimes it's worth opening up an object a little bit to make testing easier...)

Jon Skeet
+1 - just about word for word what I was going to say
Eric Petroelje
Jon,good question -- what is important about the result. After thinking about this-- the most important thing about the result is that it should be an object that is expected by the methods that will use it. And this is accomplished in the //LOGIC// section of the code, where LOGIC manipulates the fields of "this"So I did the following test scenarios: 1. I set up setters and getters for this fields2. set them up with +ve (good values) and -ve(bad values)3. and build it without exceptions...this are trivial tests but a good beginning-- thanks
+5  A: 

What does this object do ? If you can't query fields etc., I'm guessing it acts on some object that you pass into it ? If that's the case, can you pass in a dummy object that implements the same interface, but checks that the result object calls it with the appropriate arguments, in the expected sequence etc.?

e.g.

public class ResultObject {
   public void actOn(ActedUpon obj) {
      // does stuff

where ActedUpon is an interface, and would have a 'normal' implementation, and a validation implementation (using a mocking framework like JMock here would be ideal)

Brian Agnew
+2  A: 

First of all, by convention a getter should not have side effects. So it's almost certainly a bad idea to do have

this.resultState = resultState;

in your getter.

That said, returning resultState must have some purpose, otherwise why are you returning it? So test whether resultState does what it should.

sleske
A: 

If what you want to test is simply that after you call o.setResultState(resultState), you can get the resultState with o.getResultState(), the test would be something very simple like:

ResultState resultState = new ResultState(...);
o.setResultState(resultState);
assertEquals(resultState, o.getResultState());
Bruno Rothgiesser
+2  A: 

Based on that code. I would say the check for Not Null is enough. Checking the functionality of the returned ResultState belongs in tests for ResultState. You're trying to test the functionality of getResultState(). The only thing it does is create a new object and save a reference to it in this.resultState. So check if it created one and see if it saved the reference.

mamboking
+1  A: 

It's unfortunate that you're using getResultState() to create the ResultState object, itself, which is really your blocker. Consider refactoring into the following interface:

protected ResultState initResultState ( )
{
    this.resultState = new ResultState();
    // Initialization Logic...
    return this.resultState;
} // END initResultState

public ResultState getResultState ( )
{
    if ( this.resultState === null )
    {
        return this.initResultState();
    }

    return this.resultState;
} // END getResultState()

From this position, it's easier to test your getter method. Define a descendant class that returns a known ResultState Stub for initResultState() that can be interrogated, ie:

/**
 * The test fixutre's initResultState() method merely returns a simple Stub of
 * the ResultState object, which can be more easily interrogated.
 */
public ResultState initResultState ( )
{
    return new Stub_ResultState();
} // END initResultState

That still leaves you with the protected initResultState(), of course. Consider the following refactoring:

protected ResultState initResultState ( ResultState newResultState )
{
    this.resultState = newResultState;
    // Initialization Logic...
    return this.resultState;
} // END initResultState

public ResultState getResultState ( )
{
    if ( this.resultState === null )
    {
        return this.initResultState( new ResultState() );
    }

    return this.resultState;
} // END getResultState

This permits you to override the getResultState() method in a descendant class such that you can pass another ResultState Stub object to be manipulated and interrogated afterward, for example:

/**
 * The test fixture's getResultState() method always acts as a passthrough for
 * initResultState(), which is protected, so that the output of the protected
 * method can be interrogated.
 */
public ResultState getResultState ( )
{
    return this.initResultState( new Stub_ResultState() );
} // END getResultState

From this position, you need two test fixtures: one that overrides the behavior of initResultState() to test the functionality of the getters; another that overrides the behavior of getResultState() to test the behavior of initResultState(). Both use an instance of the Stub_ResultState class, which is necessarily a descendant of the ResultState class but provides public access to the internal values of its parent object for interrogation in unit tests.

I hope that makes sense, but feel free to ask for clarifications.

David Rogers
A: 

By doing the new in the getter, you're committing to a concrete implementation of ResultState. In your current case, that means the implementation that is opaque about its state.

Imagine for a moment that instead of new'ing up ResultState in place, you obtained it from a factory, and that the factory provided some way to substitute a mock ResultState, so that you could verify that getResultState() was doing with the ResultState object between creating it and returning it.

A simple way to get that effect is to break out the creation of ResultState into on overridable method. For example,

public ResultState getResultState() {
    ResultState resultState = createResultState();
    ...

protected ResultState createResultState() {
    return new ResultState();
}

Then, assuming your Test class is in the same package, you can create an in-line subclass that overrides getResultState() to return a mock.

While that works, it's a fragile way to write tests.

An alternate approach is to head towards something like

public ResultState getResultState() {
    ResultState resultState = _resultStateFactory.newResultState();
    ...

This requires that you figure out an appropriate way of injecting a ResultStateFactory into the object, which is something a dependency injection framework is great for. For tests, inject a factory that returns a mock ResultState that's been primed with appropriate expectations. This reduces the testing problem to "is getResultState() manipulating the ResultState object correctly before returning it?"

Dave W. Smith