Software craftsman, author of Software Craftsmanship: Professionalism Pragmatism Pride (http://leanpub.com/socra) and founder of the London Software Craftsmanship Community (LSCC). Sandro started coding at a very young age but just started his professional career years later, in 1996. He has worked for startups, software houses, product companies and a few international consultancy companies. Having worked as a consultant for the majority of his career, he had the opportunity to work in a good variety of projects, different languages, technologies and industries. Currently he is a director and a software craftsman at UBS Investment Bank, where he writes code, look after the quality of the applications, mentor developers and help teams around the world to get better at delivering quality software. Sandro is a DZone MVB and is not an employee of DZone and has posted 34 posts at DZone. You can read more from them at their website. View Full User Profile

Testing legacy: Hard-wired dependencies (part 1)

07.18.2011
| 3060 views |
  • submit to reddit

When pairing with some developers, I've noticed that one of the reasons they are not unit testing existing code is because, quite often, they don't know how to overcome certain problems. The most common one is related to hard-wired dependencies - Singletons and static calls.

Let's look at this piece of code:

public List<Trip> getTripsByUser(User user) throws UserNotLoggedInException {
    List<Trip> tripList = new ArrayList<Trip>();
    User loggedUser = UserSession.getInstance().getLoggedUser();
    boolean isFriend = false;
    if (loggedUser != null) {
        for (User friend : user.getFriends()) {
            if (friend.equals(loggedUser)) {
                isFriend = true;
                break;
            }
        }
        if (isFriend) {
            tripList = TripDAO.findTripsByUser(user);
        }
        return tripList;
    } else {
        throw new UserNotLoggedInException();
    }
}

Horrendous, isn't it? The code above has loads of problems, but before we change it, we need to have it covered by tests.

There are two challenges when unit testing the method above. They are:

User loggedUser = UserSession.getInstance().getLoggedUser(); // Line 3 
    
tripList = TripDAO.findTripsByUser(user);                    // Line 13

  As we know, unit tests should test just one class and not its dependencies. That means that we need to find a way to mock the Singleton and the static call. In general we do that injecting the dependencies, but we have a rule, remember?

We can't change any existing code if not covered by tests. The only exception is if we need to change the code to add unit tests, but in this case, just automated refactorings (via IDE) are allowed.

Besides that, many of the mocking frameworks are not be able to mock static methods anyway, so injecting the TripDAO would not solve the problem.

Overcoming the hard-dependencies problem

NOTE: In real life I would be writing tests first and making the change just when I needed but in order to keep the post short and focused I will not go step by step here .

First of all, let's isolate the Singleton dependency on it's own method. Let's make it protected as well. But wait, this need to be done via automated "extract method" refactoring. Select just the following piece of code on TripService.java:

UserSession.getInstance().getLoggedUser()

 

Go to your IDE's refactoring menu, choose extract method and give it a name. After this step, the code will look like that:

public class TripService {
 
    public List<Trip> getTripsByUser(User user) throws UserNotLoggedInException {
        ...
        User loggedUser = loggedUser();
        ...
    }
 
    protected User loggedUser() {
        return UserSession.getInstance().getLoggedUser();
    }
}

Doing the same thing for TripDAO.findTripsByUser(user), we will have:

public List<Trip> getTripsByUser(User user) throws UserNotLoggedInException {
    ...
    User loggedUser = loggedUser();
    ...
        if (isFriend) {
            tripList = findTripsByUser(user);
        }
    ...
} 
  
protected List<Trip> findTripsByUser(User user) {
    return TripDAO.findTripsByUser(user);
}
  
protected User loggedUser() {
    return UserSession.getInstance().getLoggedUser();
}

In our test class, we can now extend the TripService class and override the protected methods we created, making them return whatever we need for our unit tests:

private TripService createTripService() {
    return new TripService() {
        @Override protected User loggedUser() {
            return loggedUser;
        }
        @Override protected List<Trip> findTripsByUser(User user) {
            return user.trips();
        }
    };
}

And this is it. Our TripService is now testable.

First we write all the tests we need to make sure the class/method is fully tested and all code branches are exercised. I use Eclipse's eclEmma plugin for that and I strongly recommend it. If you are not using Java and/or Eclipse, try to use a code coverage tool specific to your language/IDE while writing tests for existing code. It helps a lot.

So here is the my final test class:

public class TripServiceTest {
         
    private static final User UNUSED_USER = null;
    private static final User NON_LOGGED_USER = null;
    private User loggedUser = new User();
    private User targetUser = new User();
    private TripService tripService;
 
    @Before
    public void initialise() {
        tripService  = createTripService();
    }
         
    @Test(expected=UserNotLoggedInException.class) public void
    shouldThrowExceptionWhenUserIsNotLoggedIn() throws Exception {
        loggedUser = NON_LOGGED_USER;
                  
        tripService.getTripsByUser(UNUSED_USER);
    }
         
    @Test public void
    shouldNotReturnTripsWhenLoggedUserIsNotAFriend() throws Exception {            
        List<Trip> trips = tripService.getTripsByUser(targetUser);
                  
        assertThat(trips.size(), is(equalTo(0)));
    }
         
    @Test public void
    shouldReturnTripsWhenLoggedUserIsAFriend() throws Exception {
        User john = anUser().friendsWith(loggedUser)
                            .withTrips(new Trip(), new Trip())
                            .build();
                  
        List<Trip> trips = tripService.getTripsByUser(john);
                  
        assertThat(trips, is(equalTo(john.trips())));
    }
 
    private TripService createTripService() {
        return new TripService() {
            @Override protected User loggedUser() {
                return loggedUser;
            }
            @Override protected List<Trip> findTripsByUser(User user) {
                return user.trips();
            }
        };
    }       
}

Are we done?

Of course not. We still need to refactor the TripService class. Check the part two of this post.

If you want to give it a go, here is the full code: https://github.com/sandromancuso/testing_legacy_code

From http://craftedsw.blogspot.com/2011/07/testing-legacy-hard-wired-dependencies.html

Published at DZone with permission of Sandro Mancuso, 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

Sirikant Noori replied on Sun, 2012/01/15 - 10:32am

To get rid of Singleton dependency for unit testing, you can make the singleton implement an interface. Then the class can get the interface as a parameter (dependency) in the constructor.

Comment viewing options

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