Lives in the UK. Likes blogging, cycling and eating lemon drizzle cake. Roger is a DZone MVB and is not an employee of DZone and has posted 143 posts at DZone. You can read more from them at their website. View Full User Profile

Creating Stubs for Legacy Code - Testing Techniques 6

12.01.2011
| 2605 views |
  • submit to reddit

Any one who reads this blog will probably have realised that at present I’m working on a project that contains a whole bunch of legacy code that’s large, expansive, and written without any tests what so ever. In working with this legacy code, there’s been one very badly behaved class that’s all pervasive, which the whole team have tripped over time and time again. In order to protect the guilty, I’ll call it Mr. X although its real name is SitePropertiesManager as it manages the web site’s properties. It’s badly behaved because it:

  • breaks the single responsibility principal
  • has uses singleton pattern summarised by a getInstance() factory method,
  • has an init() method that must be called before any other method,
  • loads its data using direct access to the database rather than using a DAO,
  • uses an complex map of maps to store its data,
  • accesses the file system to cache data returned by the database
  • has a timer to decide when up update its cache.
  • was written before generics and has a multitude of superfluous findXXXX() methods.
  • doesn’t implement an interface,
  • employs lots of copy and paste programming.
This makes it hard to create stubs when writing unit tests for new code and leaves the legacy code littered with:

    SitePropertiesManager propman = SitePropertiesManager.getInstance();

This blog takes a look at ways of dealing with awkward characters and demonstrates how to create stubs for them, whilst negating the effect of the Singleton pattern. As with my previous Testing Techniques blogs, I’m basing my demo code on my Address web app sample1.


In the other blogs in this series, I’ve been demonstrating how to test the AddressService and this blog is no different. In this scenario, however, the AddressService has to load a site property and decide whether or not to return an address, but before we take a look at that I first of all needed the badly written SitePropertiesManager to play with. However, I don’t own that code, so I’ve written a stunt-double version, which breaks as many rules as I can think of. I’m not going to bore you with the details here as all the source code for SitePropertiesManager is available at: git://github.com/roghughe/captaindebug.git

As I said above, in this scenario, the AddressService is using a site property to determine if it’s enabled. If it is, then it’ll send back an address object. I’m also going to pretend that the AddressService is some legacy code that uses the site properties static factory method as shown below:
  public Address findAddress(int id) {

    logger.info("In Address Service with id: " + id);

    Address address = Address.INVALID_ADDRESS;

    if (isAddressServiceEnabled()) {
      address = addressDao.findAddress(id);
      address = businessMethod(address);
    }

    logger.info("Leaving Address Service with id: " + id);
    return address;
  }

  private boolean isAddressServiceEnabled() {

    SitePropertiesManager propManager = SitePropertiesManager.getInstance();
    return new Boolean(propManager.findProperty("address.enabled"));
  }

In taming this type of class, he first thing to do is to stop using getInstance() to get hold and an object, removing it from the above method and to start using dependency injection. There has to be at least one call to getInstance(), but that can go somewhere in the program’s start-up code. In the Spring world, the solution is to wrap a badly behaved class in a Spring FactoryBean implementation, which becomes the sole location of getInstance() in your application - at least for new / enhancement code.

public class SitePropertiesManagerFactoryBean implements
    FactoryBean<SitePropertiesManager> {

  private static SitePropertiesManager propsManager = SitePropertiesManager
      .getInstance();

  @Override
  public SitePropertiesManager getObject() throws Exception {

    return propsManager;
  }

  @Override
  public Class<SitePropertiesManager> getObjectType() {

    return SitePropertiesManager.class;
  }

  @Override
  public boolean isSingleton() {

    return true;
  }
}

This can now be autowired into the AddressService class:
  @Autowired
  void setPropertiesManager(SitePropertiesManager propManager) {
    this.propManager = propManager;
  }

These changes, however, don’t mean that we can write some proper unit tests for AddressService, they just prepare the ground. The next step is to extract an interface for SitePropertiesManager, which is easily achieved using eclipse.
public interface PropertiesManager {

  public abstract String findProperty(String propertyName);

  public abstract String findProperty(String propertyName, String locale);

  public abstract List<String> findListProperty(String propertyName);

  public abstract List<String> findListProperty(String propertyName, String locale);

  public abstract int findIntProperty(String propertyName);

  public abstract int findIntProperty(String propertyName, String locale);

}

In moving to interfaces, we also need to manually configure an instance of SitePropertiesManager in the Spring config file so that Spring knows which class to connect to what interface:

<beans:bean id="propman" class="com.captaindebug.siteproperties.SitePropertiesManager" />

We also need to update the AddressService’s @Autowired annotation with a qualifier:

  @Autowired
  @Qualifier("propman")
  void setPropertiesManager(PropertiesManager propManager) {
    this.propManager = propManager;
  }

With an interface, we can now easily write a simple SitePropertiesManager stub:
public class StubPropertiesManager implements PropertiesManager {

  private final Map<String, String> propMap = new HashMap<String, String>();

  public void setProperty(String key, String value) {
    propMap.put(key, value);
  }

  @Override
  public String findProperty(String propertyName) {
    return propMap.get(propertyName);
  }

  @Override
  public String findProperty(String propertyName, String locale) {
    throw new UnsupportedOperationException();
  }

  @Override
  public List<String> findListProperty(String propertyName) {
    throw new UnsupportedOperationException();
  }

  @Override
  public List<String> findListProperty(String propertyName, String locale) {
    throw new UnsupportedOperationException();
  }

  @Override
  public int findIntProperty(String propertyName) {
    throw new UnsupportedOperationException();
  }

  @Override
  public int findIntProperty(String propertyName, String locale) {
    throw new UnsupportedOperationException();
  }
}

From having stub, it’s pretty easy to write a unit test for the AddressService that uses the stub and is isolated from the database and the file system

public class AddressServiceUnitTest {

  private StubAddressDao addressDao;

  private StubPropertiesManager stubProperties;

  private AddressService instance;

  @Before
  public void setUp() {
    instance = new AddressService();

    stubProperties = new StubPropertiesManager();
    instance.setPropertiesManager(stubProperties);
  }

  @Test
  public void testAddressSiteProperties_AddressServiceDisabled() {

    /* Set up the AddressDAO Stubb for this test */
    Address address = new Address(1, "15 My Street", "My Town", "POSTCODE",
        "My Country");
    addressDao = new StubAddressDao(address);
    instance.setAddressDao(addressDao);

    stubProperties.setProperty("address.enabled", "false");

    Address expected = Address.INVALID_ADDRESS;
    Address result = instance.findAddress(1);

    assertEquals(expected, result);
  }

  @Test
  public void testAddressSiteProperties_AddressServiceEnabled() {

    /* Set up the AddressDAO Stubb for this test */
    Address address = new Address(1, "15 My Street", "My Town", "POSTCODE",
        "My Country");
    addressDao = new StubAddressDao(address);
    instance.setAddressDao(addressDao);

    stubProperties.setProperty("address.enabled", "true");

    Address result = instance.findAddress(1);

    assertEquals(address, result);
  }
}

All of which is pretty hunky-dory, but what happens if you can’t extract an interface? I’m saving that for another day...

1 The source code is available from GitHub at:

git://github.com/roghughe/captaindebug.git

 

From http://www.captaindebug.com/2011/11/creating-stubs-for-legacy-code-testing.html

Published at DZone with permission of Roger Hughes, author and DZone MVB.

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Tags:

Comments

Andrew Spencer replied on Thu, 2011/12/01 - 4:16am

I take issue with the approach of extracting an interface and writing the stub by hand.  With tools like Mockito, you can stub concrete classes.  This generally involves less boilerplate than writing stubs by hand. And, it means that we no longer need to program to interfaces for reasons of testability alone.

Programming to interfaces has been promoted as a good design practice, independent of its effect on testability.  I held that belief myself not so long ago.  With the benefit of greater experience, I now think that it's justified only when there is a genuine need -- in the production code -- for the abstraction that the interface represents.  A genuine need means either that we are going to write multiple implementations ourselves, or that we are giving client code the possibility to provide an implementation.  If there's only ever going to be one real implementation, I'd stick with a concrete class.

Spring Batch is a good example of a case where programming to interfaces is appropriate: a flexible library, that client code can extend and customise almost without limitation.  A service class in a business app, on the other hand, is never going to be exported outside the application, and it's very unlikely that there'll ever be multiple implementations of its API.

Props btw for going to the trouble of writing a stunt double.  I've often seen dodgy classes in customers' projects that I've wished I could put in a blog post, if only for the purpose of letting off steam, but I'm too lazy to write stunt doubles.  (Or maybe it is just that I can't bring myself to add to the quantity of bad code in the world, even for example purposes. Yes, that must be it.)

Roger Hughes replied on Thu, 2011/12/01 - 10:22am in response to: Andrew Spencer

Andrew
Thanks for the comments. Yes, you're right, there are tools available for writing stubs more easily, but I really wanted to demonstrate the technique of using stubs, which some people are not aware of, and to show what functionality they should contain. In my blog on Mocks I demonstrated this by writing a homemade mock and then doing the same thing using EasyMock.

I agree that there's no point in programming to interfaces just for the sake of it; however, I'm also of the opinion that test code should be treated as a genuine client of the production code and that a test implementation should be treated as a real implementation. If extracting an interface from a third party class for test purposes means that my code is tested then that's not a bad trade off, but it shouldn't be done unnecessarily. In terms of writing homemade stubs I think that I prefer the idea of using inheritance, which was published earlier today. This is just smaller, simpler and neater than extracting an interface.

...and also I have to say that I found trying to write code badly for the stunt double really hard.

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.