5

I'm unit testing a class where I need a certain amount of time to pass before I can check results. Specifically I need x minutes to pass before I can tell whether the test worked or not. I have read that in unit testing we should be testing the interface and not the implementation, so we should not be accessing private variables, but other than putting a sleep in my unit test I don't know how to test without modifying private variables.

My test is set up like this:

@Test
public void testClearSession() {
    final int timeout = 1;
    final String sessionId = "test";
    sessionMgr.setTimeout(timeout);
    try {
        sessionMgr.createSession(sessionId);
    } catch (Exception e) {
        e.printStackTrace();
    }
    DBSession session = sessionMgr.getSession(sessionId);
    sessionMgr.clearSessions();
    assertNotNull(sessionMgr.getSession(sessionId));
    Calendar accessTime = Calendar.getInstance();
    accessTime.add(Calendar.MINUTE, - timeout - 1);
    session.setAccessTime(accessTime.getTime()); // MODIFY PRIVATE VARIABLE VIA PROTECTED SETTER
    sessionMgr.clearSessions();
    assertNull(sessionMgr.getSession(sessionId));
}

Is it possible to test this other than modifying the accessTime private variable (via creating the setAccessTime setter or reflection), or inserting a sleep in the unit test?

EDIT 11-April-2012

I am specifically trying to test that my SessionManager object clears sessions after a specific period of time has passed. The database I am connecting to will drop connections after a fixed period of time. When I get close to that timeout, the SessionManager object will clear the sessions by calling a "finalise session" procedure on the database, and removing the sessions from it's internal list.

The SessionManager object is designed to be run in a separate thread. The code I am testing looks like this:

public synchronized void clearSessions() {
    log.debug("clearSessions()");
    Calendar cal = Calendar.getInstance();
    cal.add(Calendar.MINUTE, - timeout);
    Iterator<Entry<String, DBSession>> entries = sessionList.entrySet().iterator();
    while (entries.hasNext()) {
        Entry<String, DBSession> entry = entries.next();
        DBSession session = entry.getValue();
        if (session.getAccessTime().before(cal.getTime())) {
            // close connection
            try {
                connMgr.closeconn(session.getConnection(), entry.getKey());
            } catch (Exception e) {
                e.printStackTrace();
            }
            entries.remove();
        }
    }
}

The call to connMgr (ConnectionManager object) is a bit convoluted, but I am in the process of refactoring legacy code and it is what it is at the moment. The Session object stores a connection to the database as well as some associated data.

Itai Hanski
  • 8,540
  • 5
  • 45
  • 65
Allan5
  • 351
  • 5
  • 17
  • A side note: unit tests generally shouldn't test anything that works in background (in separate threads). So you may splitting the logic and running it in another thread. The first one then could be easily tested by unit tests, and the second - by functional/acceptance tests – zerkms Apr 05 '12 at 03:21
  • Does the code that you're testing call `System.getCurrentTimeMillis()` (or something equivalent such as `new Date()`)? If so, this is something of a testability anti-pattern; you want to delegate this to a "time source" class, that you can inject. Then, you can do your tests with a mock time source. If you can post the source that you're trying to test, I can help you understand how to do what I suggest. – Dawood ibn Kareem Apr 05 '12 at 08:40
  • @DavidWallace - I just assumed GetInstance is that call. JavaDocs confirm `Calendar's getInstance method returns a Calendar object whose time fields have been initialized with the current date and time:` – Gishu Apr 05 '12 at 11:20
  • Right, but I don't think you can inject that; `getInstance` is static. What the OP needs is a class that can give you the current time (optionally as a `Calendar`), but that can also be mocked and injected into the SUT. – Dawood ibn Kareem Apr 05 '12 at 11:29
  • I would recommend using Jodatime here: its DateTimeUtils class allows you to mock out the source of time. – Kkkev Apr 06 '12 at 07:12
  • @DavidWallace - I've added the source that I'm trying to test to the question. – Allan5 Apr 11 '12 at 07:26

4 Answers4

4
  • The test could do with some refactoring to make the intent clearer. If what I comprehend is correct...

.

public void TestClearSessionsMaintainsSessionsUnlessLastAccessTimeIsOverThreshold() {

    final int timeout = 1;
    final String sessionId = "test";
    sessionMgr = GetSessionManagerWithTimeout(timeout);
    DBSession session = CreateSession(sessionMgr, sessionId);

    sessionMgr.clearSessions();
    assertNotNull(sessionMgr.getSession(sessionId));

    session.setAccessTime(PastInstantThatIsOverThreshold()); // MODIFY PRIVATE VARIABLE VIA PROTECTED SETTER
    sessionMgr.clearSessions();
    assertNull(sessionMgr.getSession(sessionId));
}
  • Now to the matter of testing without having to expose private state
    • How is the private variable modified in real life ? Is there some other public method you could call which updates the access time?
    • Since the clock/time is an important concept, why not make that explicit as a role. So you could pass a Clock object to the Session, which it uses to update its internal access time. In your tests, you could pass in a MockClock, whose getCurrentTime() method would return whatever value you wish. I'm making up the mocking syntax.. so update with whatever you are using.

.

public void TestClearSessionsMaintainsSessionsUnlessLastAccessTimeIsOverThreshold() {

      final int timeout = 1;
      final String sessionId = "test";
      expect(mockClock).GetCurrentTime(); willReturn(CurrentTime());
      sessionMgr = GetSessionManagerWithTimeout(timeout, mockClock);
      DBSession session = CreateSession(sessionMgr, sessionId);

      sessionMgr.clearSessions();
      assertNotNull(sessionMgr.getSession(sessionId));

      expect(mockClock).GetCurrentTime(); willReturn(PastInstantThatIsOverThreshold());
      session.DoSomethingThatUpdatesAccessTime();
      sessionMgr.clearSessions();
      assertNull(sessionMgr.getSession(sessionId));
}
Gishu
  • 134,492
  • 47
  • 225
  • 308
  • You should mention that you used [mockito](http://code.google.com/p/mockito/) to get that nice mocking-syntax. – Björn Pollex Apr 05 '12 at 07:08
  • @BjörnPollex are you sure? It doesn't look like mockito syntax to me. – Dawood ibn Kareem Apr 05 '12 at 08:27
  • +1 also for exposing the clock as a dependency that can be mocked in test and changed in production. – Alan Escreet Apr 05 '12 at 08:36
  • @DavidWallace: I think you are right - i got confused. Should be mockito though :) – Björn Pollex Apr 05 '12 at 08:45
  • @BjörnPollex - I think it's a custom mashup of JMock. It's been a while since I did mocking in Java, however that never stopped me answering on SO :) – Gishu Apr 05 '12 at 10:30
  • @Gishu - you're understanding of the test's intent was correct. The access time is updated firstly when the DBSession is created (DBSession's constructor), and again each time the Connection object is retrieved from the session (session.getConnection()). The intent is to catch the latest activity on the connection, to ensure that the database connection is not timed out. I like the approach you are talking about here, I am setting up the SessionManager object using a factory in the jUnit setUp method (factory.createSessionManager(...)), so creating a mockClock and passing it in could work. – Allan5 Apr 11 '12 at 07:49
1

EDIT: I like Gishu's answer better. He also encourages you to mock the time, but he treats it as a first class object.

What exactly is the rule you're trying to test? If I'm reading your code right, it looks like your desire is to verify that the session associated with the ID "test" expires after a given timeout, correct?

Time is a tricky thing in unit tests because it's essentially global state, so this is a better candidate for an acceptance test (like zerkms suggested).

If you still want to have a unit test for it, generally I try to abstract and/or isolate references to time, so I can mock them in my tests. One way to do this is by subclassing the class under test. This is a slight break in encapsulation, but it works cleaner than providing a protected setter method, and far better than reflection.

An example:

class MyClass {
  public void doSomethingThatNeedsTime(int timeout) {
    Date now = getNow();
    if (new Date().getTime() > now.getTime() + timeout) {
      // timed out!
    }
  }

  Date getNow() {
    return new Date();
  }
}

class TestMyClass {
  @Test
  public void testDoSomethingThatNeedsTime() {
    MyClass mc = new MyClass() {
      Date getNow() {
        // return a time appropriate for my test
      }    
    };

    mc.doSomethingThatNeedsTime(1);

    // assert
  }
}

This is a bit of a contrived example, but hopefully you get the point. By subclassing the getNow() method, my test is no longer subject to the global time. I can substitute whatever time I want.

Like I said, this breaks encapsulation a little, because the REAL getNow() method never gets tested, and it requires the test to know something about the implementation. That's why it's good to keep such a method small and focused, with no side effects. This example also assumes the class under test is not final.

Despite the drawbacks, it's cleaner (in my opinion) than providing a scoped setter for a private variable, which can actually allow a programmer to do harm. In my example, if some rogue process invokes the getNow() method, there's no real harm done.

Ryan Nelson
  • 4,466
  • 5
  • 29
  • 45
  • Yes, I'm trying to catch the session just before the associated database connection times out, so I can close the connection cleanly, rather than getting timed out. Thanks for the comments, the different perspective was helpful. – Allan5 Apr 11 '12 at 08:01
1

It looks like functionality being tested is SessionManager evitcs all expired sessions.

I would consider creating test class extending DBSession. 

AlwaysExpiredDBSession extends DBSession  {
....
// access time to be somewhere older 'NOW'

}
Jayan
  • 18,003
  • 15
  • 89
  • 143
  • This is an interesting possibility although my DBSession objects are created by the SessionManager object that I am testing, so I would need to make some way to inject the object. My current interface allows creation and retrieval of DBSessions but no insertion. I think I would end up coming to the same problem of having to modify a private variable on the SessionManager object rather than a private variable of the DBSession object. – Allan5 Apr 11 '12 at 08:08
  • I had a think about this and realised it would work. In the general case this approach would definitely work, but also in my case I am creating the DBSession objects using a factory, so I just need to pass a different factory to the SessionManager and it should work. – Allan5 Apr 12 '12 at 07:03
0

I basically followed Gishu's suggestion https://stackoverflow.com/a/10023832/1258214, but I thought I would document the changes just for the benefit of anyone else reading this (and so anyone can comment on issues with the implementation). Thank you to the comment's pointing me to JodaTime and Mockito.

The relevant idea was to recognise the dependency of the code on time and to extract that out (see: https://stackoverflow.com/a/5622222/1258214). This was done by creating an interface:

import org.joda.time.DateTime;

public interface Clock {
    public DateTime getCurrentDateTime() ;
}

Then creating an implementation:

import org.joda.time.DateTime;

public class JodaClock implements Clock {

    @Override
    public DateTime getCurrentDateTime() {
        return new DateTime();
    }

}

This was then passed into SessionManager's constructor:

SessionManager(ConnectionManager connMgr, SessionGenerator sessionGen,
        ObjectFactory factory, Clock clock) {

I was then able to use code similar to what Gishu suggested (note the lower case 't' at the beginning of testClear... my unit tests were very successful with the upper case 'T' until I realised that the test wasn't running...):

@Test
public void testClearSessionsMaintainsSessionsUnlessLastAccessTimeIsOverThreshold() {
    final String sessionId = "test";
    final Clock mockClock = mock(Clock.class);

    when(mockClock.getCurrentDateTime()).thenReturn(getNow());
    SessionManager sessionMgr = getSessionManager(connMgr,
            sessionGen, factory, mockClock);
    createSession(sessionMgr, sessionId);

    sessionMgr.clearSessions(defaultTimeout);
    assertNotNull(sessionMgr.getSession(sessionId));

    when(mockClock.getCurrentDateTime()).thenReturn(getExpired());

    sessionMgr.clearSessions(defaultTimeout);
    assertNull(sessionMgr.getSession(sessionId));
}

This ran great, but my removal of the Session.setAccessTime() created an issue with another test testOnlyExpiredSessionsCleared() where I wanted one session to expire but not the other. This link https://stackoverflow.com/a/6060814/1258214 led me to thinking about the design of the SessionManager.clearSessions() method, and I refactored the checking if a session is expired from the SessionManager, to the DBSession object itself.

From:

if (session.getAccessTime().before(cal.getTime())) {

To:

if (session.isExpired(expireTime)) {

Then I inserted a mockSession object (similar to Jayan's suggestion https://stackoverflow.com/a/10023916/1258214)

@Test
public void testOnlyOldSessionsCleared() {
    final String sessionId = "test";
    final String sessionId2 = "test2";

    ObjectFactory mockFactory = spy(factory);
    SessionManager sm = factory.createSessionManager(connMgr, sessionGen,
        mockFactory, clock);

    // create expired session
    NPIISession session = factory.createNPIISession(null, clock);
    NPIISession mockSession = spy(session);
    // return session expired
    doReturn(true).when(mockSession).isExpired((DateTime) anyObject());

    // get factory to return mockSession to sessionManager
    doReturn(mockSession).when(mockFactory).createDBSession(
        (Connection) anyObject(), eq(clock));
    createSession(sm, sessionId);

    // reset factory so return normal session
    reset(mockFactory);
    createSession(sm, sessionId2);

    assertNotNull(sm.getSession(sessionId));
    assertNotNull(sm.getSession(sessionId2));

    sm.clearSessions(defaultTimeout);
    assertNull(sm.getSession(sessionId));
    assertNotNull(sm.getSession(sessionId2));
}

Thanks to everyone for their help with this. Please let me know if you see any issues with the changes.

Community
  • 1
  • 1
Allan5
  • 351
  • 5
  • 17