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.
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 2012-04-05 08:40
Calendar's getInstance method returns a Calendar object whose time fields have been initialized with the current date and time:
Gishu 2012-04-05 11:20
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 2012-04-05 11:29
.
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));
}
.
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));
}
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.
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'
}
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.