Skip to content

Commit 5edc613

Browse files
chrjohnchrjohn
authored
Unit test improvements (#198)
* Minor improvements in ResynchTest. * Fix race condition in SocketAcceptorTest ** sometimes the shutdown was done without waiting for the Logout message * Cleanup sessions and stop session timer. * Removed deprecated methods. * Minor cleanups. * Handle InterruptedException on ATServer stop.
1 parent d6e96de commit 5edc613

File tree

7 files changed

+134
-101
lines changed

7 files changed

+134
-101
lines changed

‎quickfixj-core/src/test/java/quickfix/SessionDisconnectConcurrentlyTest.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919

2020
package quickfix;
2121

22-
import junit.framework.TestCase;
2322
import org.junit.After;
2423
import org.junit.Assert;
2524
import org.junit.Test;

‎quickfixj-core/src/test/java/quickfix/SocketAcceptorTest.java‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ public void testRestartOfAcceptor() throws Exception {
106106
log.error(e.getMessage(), e);
107107
}
108108
}
109-
assertEquals("application should receive logout", 1, testAcceptorApplication.logoutCounter);
110-
assertEquals("application should receive logout", 1, testInitiatorApplication.logoutCounter);
109+
testAcceptorApplication.waitForLogout();
110+
testInitiatorApplication.waitForLogout();
111111
}
112112
}
113113

@@ -209,10 +209,11 @@ private Session lookupSession(SessionID sessionID) {
209209
private static class TestAcceptorApplication extends ApplicationAdapter {
210210

211211
private final CountDownLatch logonLatch;
212-
public volatile int logoutCounter = 0;
212+
private final CountDownLatch logoutLatch;
213213

214214
public TestAcceptorApplication() {
215215
logonLatch = new CountDownLatch(1);
216+
logoutLatch = new CountDownLatch(1);
216217
}
217218

218219
@Override
@@ -229,11 +230,19 @@ public void waitForLogon() {
229230
}
230231
}
231232

233+
public void waitForLogout() {
234+
try {
235+
assertTrue("Logout timed out", logoutLatch.await(10, TimeUnit.SECONDS));
236+
} catch (InterruptedException e) {
237+
fail(e.getMessage());
238+
}
239+
}
240+
232241
@Override
233242
public void fromAdmin(Message message, SessionID sessionId) throws FieldNotFound, IncorrectDataFormat, IncorrectTagValue, RejectLogon {
234243
try {
235244
if (MsgType.LOGOUT.equals(MessageUtils.getMessageType(message.toString()))) {
236-
logoutCounter++;
245+
logoutLatch.countDown();
237246
}
238247
} catch (InvalidMessage ex) {
239248
// ignore
@@ -244,10 +253,11 @@ public void fromAdmin(Message message, SessionID sessionId) throws FieldNotFound
244253
private static class TestInitiatorApplication extends ApplicationAdapter {
245254

246255
private final CountDownLatch logonLatch;
247-
public volatile int logoutCounter = 0;
256+
private final CountDownLatch logoutLatch;
248257

249258
public TestInitiatorApplication() {
250259
logonLatch = new CountDownLatch(1);
260+
logoutLatch = new CountDownLatch(1);
251261
}
252262

253263
@Override
@@ -263,12 +273,20 @@ public void waitForLogon() {
263273
fail(e.getMessage());
264274
}
265275
}
266-
276+
277+
public void waitForLogout() {
278+
try {
279+
assertTrue("Logout timed out", logoutLatch.await(10, TimeUnit.SECONDS));
280+
} catch (InterruptedException e) {
281+
fail(e.getMessage());
282+
}
283+
}
284+
267285
@Override
268286
public void fromAdmin(Message message, SessionID sessionId) throws FieldNotFound, IncorrectDataFormat, IncorrectTagValue, RejectLogon {
269287
try {
270288
if (MsgType.LOGOUT.equals(MessageUtils.getMessageType(message.toString()))) {
271-
logoutCounter++;
289+
logoutLatch.countDown();
272290
}
273291
} catch (InvalidMessage ex) {
274292
// ignore

‎quickfixj-core/src/test/java/quickfix/mina/SessionConnectorTest.java‎

Lines changed: 76 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -68,36 +68,39 @@ public void testConnector() throws Exception {
6868

6969
connector.addPropertyChangeListener(new SessionConnectorListener());
7070

71-
Session session = connector.createSession(sessionID);
72-
assertNotNull(session);
73-
74-
Map<SessionID, Session> sessions = Collections.singletonMap(session.getSessionID(), session);
75-
connector.setSessions(sessions);
76-
77-
assertEquals(1, propertyChangeEvents.size());
78-
79-
assertEquals(1, connector.getManagedSessions().size());
80-
assertEquals(session, connector.getManagedSessions().get(0));
81-
82-
assertFalse(connector.isLoggedOn());
83-
84-
Field stateField = session.getClass().getDeclaredField("state");
85-
stateField.setAccessible(true);
86-
SessionState state = (SessionState) stateField.get(session);
87-
88-
state.setLogonSent(true);
89-
state.setLogonReceived(true);
90-
assertTrue(connector.isLoggedOn());
91-
92-
assertTrue(session.isEnabled());
93-
connector.logoutAllSessions(true);
94-
// Acceptors should get re-enabled after Logout
95-
assertTrue(session.isEnabled());
96-
97-
assertEquals(9999, connector.getIntSetting(Acceptor.SETTING_SOCKET_ACCEPT_PORT));
98-
99-
assertNotNull(connector.getScheduledExecutorService());
100-
assertEquals(settings, connector.getSettings());
71+
try (Session session = connector.createSession(sessionID)) {
72+
assertNotNull(session);
73+
74+
Map<SessionID, Session> sessions = Collections.singletonMap(session.getSessionID(), session);
75+
connector.setSessions(sessions);
76+
77+
assertEquals(1, propertyChangeEvents.size());
78+
79+
assertEquals(1, connector.getManagedSessions().size());
80+
assertEquals(session, connector.getManagedSessions().get(0));
81+
82+
assertFalse(connector.isLoggedOn());
83+
84+
Field stateField = session.getClass().getDeclaredField("state");
85+
stateField.setAccessible(true);
86+
SessionState state = (SessionState) stateField.get(session);
87+
88+
state.setLogonSent(true);
89+
state.setLogonReceived(true);
90+
assertTrue(connector.isLoggedOn());
91+
92+
assertTrue(session.isEnabled());
93+
connector.logoutAllSessions(true);
94+
// Acceptors should get re-enabled after Logout
95+
assertTrue(session.isEnabled());
96+
97+
assertEquals(9999, connector.getIntSetting(Acceptor.SETTING_SOCKET_ACCEPT_PORT));
98+
99+
assertNotNull(connector.getScheduledExecutorService());
100+
assertEquals(settings, connector.getSettings());
101+
} finally {
102+
connector.stop(true);
103+
}
101104
}
102105

103106
@Test
@@ -145,7 +148,10 @@ public void testOneSessionLoggedOnOneSessionNotLoggedOne() throws Exception {
145148
assertFalse(connector.isLoggedOn());
146149
assertTrue(connector.anyLoggedOn());
147150
}
151+
} finally {
152+
connector.stop(true);
148153
}
154+
149155
}
150156

151157
/**
@@ -161,37 +167,40 @@ public void testAddingRemovingDynamicSessions() throws Exception {
161167

162168
SessionConnector connector = new SessionConnectorUnderTest(settings, sessionFactory);
163169
connector.setSessions(new HashMap<>());
164-
Session session = connector.createSession(sessionID);
170+
try (Session session = connector.createSession(sessionID)) {
171+
// one-time use connector to create a slightly different session
172+
SessionSettings settings2 = setUpSessionSettings(sessionID2);
173+
SessionConnector connector2 = new SessionConnectorUnderTest(settings2, sessionFactory);
174+
connector.setSessions(new HashMap<>());
175+
try (Session session2 = connector2.createSession(sessionID2)) {
176+
177+
assertNotNull(session);
178+
assertNotNull(session2);
165179

166-
// one-time use connector to create a slightly different session
167-
SessionSettings settings2 = setUpSessionSettings(sessionID2);
168-
SessionConnector connector2 = new SessionConnectorUnderTest(settings2, sessionFactory);
169-
connector.setSessions(new HashMap<>());
170-
Session session2 = connector2.createSession(sessionID2);
171-
assertNotNull(session);
172-
assertNotNull(session2);
173-
174-
assertEquals(0, connector.getManagedSessions().size());
175-
connector.addDynamicSession(session);
176-
assertEquals(1, connector.getManagedSessions().size());
177-
connector.addDynamicSession(session2);
178-
assertEquals(2, connector.getManagedSessions().size());
179-
// the list can be in arbitrary order so let's make sure that we get both
180-
HashMap<SessionID, Session> map = new HashMap<>();
181-
for (Session s : connector.getManagedSessions()) {
182-
map.put(s.getSessionID(), s);
180+
assertEquals(0, connector.getManagedSessions().size());
181+
connector.addDynamicSession(session);
182+
assertEquals(1, connector.getManagedSessions().size());
183+
connector.addDynamicSession(session2);
184+
assertEquals(2, connector.getManagedSessions().size());
185+
// the list can be in arbitrary order so let's make sure that we get both
186+
HashMap<SessionID, Session> map = new HashMap<>();
187+
for (Session s : connector.getManagedSessions()) {
188+
map.put(s.getSessionID(), s);
189+
}
190+
assertEquals(session, map.get(session.getSessionID()));
191+
assertEquals(session2, map.get(session2.getSessionID()));
192+
193+
connector.removeDynamicSession(session.getSessionID());
194+
assertEquals(1, connector.getManagedSessions().size());
195+
assertEquals(session2, connector.getManagedSessions().get(0));
196+
connector.removeDynamicSession(session2.getSessionID());
197+
assertEquals(0, connector.getManagedSessions().size());
198+
} finally {
199+
connector2.stop();
200+
}
201+
} finally {
202+
connector.stop(true);
183203
}
184-
assertEquals(session, map.get(session.getSessionID()));
185-
assertEquals(session2, map.get(session2.getSessionID()));
186-
187-
connector.removeDynamicSession(session.getSessionID());
188-
assertEquals(1, connector.getManagedSessions().size());
189-
assertEquals(session2, connector.getManagedSessions().get(0));
190-
connector.removeDynamicSession(session2.getSessionID());
191-
assertEquals(0, connector.getManagedSessions().size());
192-
193-
session.close();
194-
session2.close();
195204
}
196205

197206
/**
@@ -242,6 +251,7 @@ public void testDynamicInitiatorSession() throws Exception {
242251
for(Session s:sessions){
243252
s.close();
244253
}
254+
connector.stop();
245255
}
246256

247257
private SessionSettings setUpSessionSettings(SessionID sessionID) {
@@ -274,9 +284,6 @@ private SessionSettings setUpInitiatorSessionSettings(SessionID sessionID) {
274284
settings.setString(Initiator.SETTING_PROXY_DOMAIN,"Test Proxy Domain");
275285
settings.setString(Initiator.SETTING_PROXY_HOST,"Test Proxy Host");
276286
settings.setString(Initiator.SETTING_PROXY_PORT,"888");
277-
278-
279-
280287
settings.setBool(Initiator.SETTING_DYNAMIC_SESSION,false);
281288
settings.setString(sessionID, SessionFactory.SETTING_CONNECTION_TYPE,
282289
SessionFactory.INITIATOR_CONNECTION_TYPE);
@@ -301,14 +308,14 @@ public void start() throws ConfigError, RuntimeError {
301308
}
302309

303310
public void stop() {
311+
super.stopSessionTimer();
304312
}
305313

306314
public void stop(boolean force) {
307-
}
308-
309-
public void block() throws ConfigError, RuntimeError {
315+
super.stopSessionTimer();
310316
}
311317
}
318+
312319
private static class AbstractSocketInitiatorUnderTest extends AbstractSocketInitiator {
313320

314321
public AbstractSocketInitiatorUnderTest(SessionSettings settings, SessionFactory sessionFactory) throws ConfigError {
@@ -317,19 +324,18 @@ public AbstractSocketInitiatorUnderTest(SessionSettings settings, SessionFactory
317324

318325
public void start() throws ConfigError, RuntimeError {
319326
}
320-
public void createDynamicSession(SessionID sessionID) throws ConfigError {
321-
super.createDynamicSession(sessionID);
322-
}
327+
323328
public void stop() {
329+
clearConnectorSessions();
324330
}
331+
325332
public void stopInitiators(){
326333
super.stopInitiators();
327334
}
335+
328336
public void stop(boolean force) {
329337
}
330338

331-
public void block() throws ConfigError, RuntimeError {
332-
}
333339
@Override
334340
protected void createSessionInitiators() throws ConfigError {
335341
super.createSessionInitiators();

‎quickfixj-core/src/test/java/quickfix/mina/SingleThreadedEventHandlingStrategyTest.java‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,7 @@
4444
import java.util.HashMap;
4545
import java.util.Map;
4646
import java.util.concurrent.CountDownLatch;
47-
import java.util.logging.Level;
48-
import java.util.logging.Logger;
47+
import org.junit.AfterClass;
4948

5049
/**
5150
*
@@ -64,10 +63,16 @@ public void cleanup() {
6463
try {
6564
Thread.sleep(500);
6665
} catch (InterruptedException ex) {
67-
Logger.getLogger(SingleThreadedEventHandlingStrategyTest.class.getName()).log(Level.SEVERE, null, ex);
66+
// ignored
6867
}
6968
}
7069
}
70+
71+
72+
@AfterClass
73+
public static void afterClass() {
74+
assertQFJMessageProcessorThreads(0);
75+
}
7176

7277
@Test
7378
public void testDoubleStart() throws Exception {
@@ -159,7 +164,7 @@ public void run() {
159164

160165
Thread.currentThread().interrupt();
161166
} catch (Exception e) {
162-
e.printStackTrace();
167+
// ignored
163168
} finally {
164169
acceptor.stop();
165170
}
@@ -193,7 +198,7 @@ public void run() {
193198

194199
Thread.currentThread().interrupt();
195200
} catch (Exception e) {
196-
e.printStackTrace();
201+
// ignored
197202
} finally {
198203
initiator.stop();
199204
}
@@ -223,7 +228,7 @@ public void run() {
223228
try {
224229
acceptor.start();
225230
} catch (Exception e) {
226-
e.printStackTrace();
231+
// ignored
227232
} finally {
228233
acceptor.stop();
229234
}
@@ -253,7 +258,7 @@ public void run() {
253258
try {
254259
initiator.start();
255260
} catch (Exception e) {
256-
e.printStackTrace();
261+
// ignored
257262
} finally {
258263
initiator.stop();
259264
}
@@ -318,7 +323,7 @@ private SocketInitiator createInitiator(int i) throws ConfigError {
318323
acceptorSettings, new DefaultMessageFactory());
319324
}
320325

321-
private void assertQFJMessageProcessorThreads(int expected) {
326+
private static void assertQFJMessageProcessorThreads(int expected) {
322327
ThreadMXBean bean = ManagementFactory.getThreadMXBean();
323328
ThreadInfo[] dumpAllThreads = bean.dumpAllThreads(false, false);
324329
int qfjMPThreads = getMessageProcessorThreads(dumpAllThreads);
@@ -328,7 +333,7 @@ private void assertQFJMessageProcessorThreads(int expected) {
328333
try {
329334
Thread.sleep(100);
330335
} catch (InterruptedException ex) {
331-
Logger.getLogger(SingleThreadedEventHandlingStrategyTest.class.getName()).log(Level.SEVERE, null, ex);
336+
// ignored
332337
}
333338
dumpAllThreads = bean.dumpAllThreads(false, false);
334339
qfjMPThreads = getMessageProcessorThreads(dumpAllThreads);
@@ -342,7 +347,7 @@ private void assertQFJMessageProcessorThreads(int expected) {
342347
Assert.assertEquals("Expected " + expected + " 'QFJ Message Processor' thread(s)", expected, qfjMPThreads);
343348
}
344349

345-
private int getMessageProcessorThreads(ThreadInfo[] dumpAllThreads) {
350+
private static int getMessageProcessorThreads(ThreadInfo[] dumpAllThreads) {
346351
int qfjMPThreads = 0;
347352
for (ThreadInfo threadInfo : dumpAllThreads) {
348353
if (SingleThreadedEventHandlingStrategy.MESSAGE_PROCESSOR_THREAD_NAME.equals(threadInfo
@@ -353,7 +358,7 @@ private int getMessageProcessorThreads(ThreadInfo[] dumpAllThreads) {
353358
return qfjMPThreads;
354359
}
355360

356-
private void printStackTraces(ThreadInfo threadInfo) {
361+
private static void printStackTraces(ThreadInfo threadInfo) {
357362
System.out.println( threadInfo.getThreadName() + " " + threadInfo.getThreadState());
358363
StackTraceElement[] stackTrace = threadInfo.getStackTrace();
359364
for (StackTraceElement stackTrace1 : stackTrace) {

0 commit comments

Comments
 (0)