From 3de14c634275a2013227ae655ff8f3363c0ecf48 Mon Sep 17 00:00:00 2001
From: Chris Busbey <cbusbey@connamara.com>
Date: Wed, 10 Aug 2016 08:39:55 -0500
Subject: [PATCH] fixes some flakey session time behavior, better test coverage

---
 in_session_test.go       |  15 ----
 latent_state_test.go     |  11 ++-
 logon_state_test.go      |   4 -
 logout_state_test.go     |   4 -
 not_session_time.go      |  12 +++
 not_session_time_test.go |  31 +++++++
 pending_timeout_test.go  |  42 ++++-----
 quickfix_test.go         |   9 +-
 session_state.go         |  21 +++--
 session_test.go          | 189 ++++++++++++++++++++++++++++++++++-----
 10 files changed, 250 insertions(+), 88 deletions(-)
 create mode 100644 not_session_time_test.go

diff --git a/in_session_test.go b/in_session_test.go
index f505e64a..ccd30ef7 100644
--- a/in_session_test.go
+++ b/in_session_test.go
@@ -99,18 +99,3 @@ func (s *InSessionTestSuite) TestDisconnected() {
 	s.mockApp.AssertExpectations(s.T())
 	s.State(latentState{})
 }
-
-func (s *InSessionTestSuite) TestShutdownNow() {
-	s.mockApp.On("FromApp").Return(nil)
-	s.FixMsgIn(s.session, s.NewOrderSingle())
-	s.mockApp.AssertExpectations(s.T())
-	s.session.store.IncrNextSenderMsgSeqNum()
-
-	s.mockApp.On("ToAdmin").Return(nil)
-	s.session.State.ShutdownNow(s.session)
-
-	s.mockApp.AssertExpectations(s.T())
-	s.LastToAdminMessageSent()
-	s.MessageType("5", s.mockApp.lastToAdmin)
-	s.FieldEquals(tagMsgSeqNum, 2, s.mockApp.lastToAdmin.Header)
-}
diff --git a/latent_state_test.go b/latent_state_test.go
index 44d3de54..58cd9e9c 100644
--- a/latent_state_test.go
+++ b/latent_state_test.go
@@ -19,14 +19,13 @@ func (s *LatentStateTestSuite) SetupTest() {
 	s.session.State = latentState{}
 }
 
-func (s *LatentStateTestSuite) TestIsLoggedOn() {
+func (s *LatentStateTestSuite) TestPreliminary() {
 	s.False(s.session.IsLoggedOn())
-}
-
-func (s *LatentStateTestSuite) TestIsConnected() {
 	s.False(s.session.IsConnected())
+	s.True(s.session.IsSessionTime())
 }
 
-func (s *LatentStateTestSuite) TestIsSessionTime() {
-	s.True(s.session.IsSessionTime())
+func (s *LatentStateTestSuite) TestDisconnected() {
+	s.session.Disconnected(s.session)
+	s.State(latentState{})
 }
diff --git a/logon_state_test.go b/logon_state_test.go
index 5f8da833..b549ce1b 100644
--- a/logon_state_test.go
+++ b/logon_state_test.go
@@ -58,10 +58,6 @@ func (s *LogonStateTestSuite) TestTimeoutNotLogonTimeout() {
 	}
 }
 
-func (s *LogonStateTestSuite) TestShutdownNow() {
-	s.session.State.ShutdownNow(s.session)
-}
-
 func (s *LogonStateTestSuite) TestDisconnected() {
 	s.session.Disconnected(s.session)
 	s.State(latentState{})
diff --git a/logout_state_test.go b/logout_state_test.go
index 83eb0568..6557d02e 100644
--- a/logout_state_test.go
+++ b/logout_state_test.go
@@ -49,10 +49,6 @@ func (s *LogoutStateTestSuite) TestTimeoutNotLogoutTimeout() {
 	}
 }
 
-func (s *LogoutStateTestSuite) TestShutdownNow() {
-	s.session.State.ShutdownNow(s.session)
-}
-
 func (s *LogoutStateTestSuite) TestDisconnected() {
 	s.mockApp.On("OnLogout").Return(nil)
 	s.session.Disconnected(s.session)
diff --git a/not_session_time.go b/not_session_time.go
index d594222c..5fd464bf 100644
--- a/not_session_time.go
+++ b/not_session_time.go
@@ -1,5 +1,17 @@
 package quickfix
 
+import "github.com/quickfixgo/quickfix/internal"
+
 type notSessionTime struct{ latentState }
 
+func (notSessionTime) String() string      { return "Not session time" }
 func (notSessionTime) IsSessionTime() bool { return false }
+
+func (state notSessionTime) FixMsgIn(session *session, msg Message) (nextState sessionState) {
+	session.log.OnEventf("Invalid Session State: Unexpected Msg %v while in Latent state", msg)
+	return state
+}
+
+func (state notSessionTime) Timeout(*session, internal.Event) (nextState sessionState) {
+	return state
+}
diff --git a/not_session_time_test.go b/not_session_time_test.go
new file mode 100644
index 00000000..33b48786
--- /dev/null
+++ b/not_session_time_test.go
@@ -0,0 +1,31 @@
+package quickfix
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+)
+
+type NotSessionTimeTestSuite struct {
+	SessionSuite
+}
+
+func TestNotSessionTime(t *testing.T) {
+	suite.Run(t, new(NotSessionTimeTestSuite))
+}
+
+func (s *NotSessionTimeTestSuite) SetupTest() {
+	s.Init()
+	s.session.State = notSessionTime{}
+}
+
+func (s *NotSessionTimeTestSuite) TestPreliminary() {
+	s.False(s.session.IsLoggedOn())
+	s.False(s.session.IsConnected())
+	s.False(s.session.IsSessionTime())
+}
+
+func (s *NotSessionTimeTestSuite) TestDisconnected() {
+	s.session.Disconnected(s.session)
+	s.State(notSessionTime{})
+}
diff --git a/pending_timeout_test.go b/pending_timeout_test.go
index eeda54f4..0d61a67c 100644
--- a/pending_timeout_test.go
+++ b/pending_timeout_test.go
@@ -19,6 +19,20 @@ func (s *PendingTimeoutTestSuite) SetupTest() {
 	s.Init()
 }
 
+func (s *PendingTimeoutTestSuite) TestIsConnectedIsLoggedOn() {
+	tests := []pendingTimeout{
+		pendingTimeout{inSession{}},
+		pendingTimeout{resendState{}},
+	}
+
+	for _, state := range tests {
+		s.session.State = state
+
+		s.True(s.session.IsConnected())
+		s.True(s.session.IsLoggedOn())
+	}
+}
+
 func (s *PendingTimeoutTestSuite) TestSessionTimeout() {
 	tests := []pendingTimeout{
 		pendingTimeout{inSession{}},
@@ -68,32 +82,6 @@ func (s *PendingTimeoutTestSuite) TestDisconnected() {
 		s.session.Disconnected(s.session)
 
 		s.mockApp.AssertExpectations(s.T())
-	}
-}
-
-func (s *PendingTimeoutTestSuite) TestShutdownNow() {
-	tests := []pendingTimeout{
-		pendingTimeout{inSession{}},
-		pendingTimeout{resendState{}},
-	}
-
-	for _, state := range tests {
-		s.SetupTest()
-		s.session.State = inSession{}
-
-		s.mockApp.On("FromApp").Return(nil)
-		s.FixMsgIn(s.session, s.NewOrderSingle())
-		s.mockApp.AssertExpectations(s.T())
-		s.session.store.IncrNextSenderMsgSeqNum()
-
-		s.session.State = state
-
-		s.mockApp.On("ToAdmin").Return(nil)
-		s.session.State.ShutdownNow(s.session)
-
-		s.mockApp.AssertExpectations(s.T())
-		s.LastToAdminMessageSent()
-		s.MessageType("5", s.mockApp.lastToAdmin)
-		s.FieldEquals(tagMsgSeqNum, 2, s.mockApp.lastToAdmin.Header)
+		s.State(latentState{})
 	}
 }
diff --git a/quickfix_test.go b/quickfix_test.go
index ccfd0c34..8e205f56 100644
--- a/quickfix_test.go
+++ b/quickfix_test.go
@@ -86,12 +86,17 @@ func (s *SessionSuite) NoMessageQueued() {
 	s.Empty(s.session.toSend, "no messages should be queueud")
 }
 
+func (s *SessionSuite) ExpectStoreReset() {
+	s.NextSenderMsgSeqNum(1)
+	s.NextTargetMsgSeqNum(1)
+}
+
 func (s *SessionSuite) NextTargetMsgSeqNum(expected int) {
-	s.Equal(expected, s.session.store.NextTargetMsgSeqNum(), "NextTargetMsgSeqNum should be ", expected)
+	s.Equal(expected, s.session.store.NextTargetMsgSeqNum(), "NextTargetMsgSeqNum should be %v ", expected)
 }
 
 func (s *SessionSuite) NextSenderMsgSeqNum(expected int) {
-	s.Equal(expected, s.session.store.NextSenderMsgSeqNum(), "NextSenderMsgSeqNum should be ", expected)
+	s.Equal(expected, s.session.store.NextSenderMsgSeqNum(), "NextSenderMsgSeqNum should be %v", expected)
 }
 
 func (s *SessionSuite) NoMessagePersisted(seqNum int) {
diff --git a/session_state.go b/session_state.go
index 64dc3a84..06f79677 100644
--- a/session_state.go
+++ b/session_state.go
@@ -12,7 +12,9 @@ type stateMachine struct {
 }
 
 func (sm *stateMachine) Disconnected(session *session) {
-	sm.setState(session, latentState{})
+	if sm.IsConnected() {
+		sm.setState(session, latentState{})
+	}
 }
 
 func (sm *stateMachine) FixMsgIn(session *session, m Message) {
@@ -24,21 +26,22 @@ func (sm *stateMachine) Timeout(session *session, e internal.Event) {
 }
 
 func (sm *stateMachine) CheckSessionTime(session *session, now time.Time) {
+	if !session.sessionTime.IsInRange(now) {
+		sm.State.ShutdownNow(session)
+		sm.setState(session, notSessionTime{})
+		return
+	}
+
 	if !sm.IsSessionTime() {
-		if !session.sessionTime.IsInRange(now) {
-			return
-		}
 		sm.setState(session, latentState{})
 	}
 
 	if !session.sessionTime.IsInSameRange(session.store.CreationTime(), now) {
 		sm.State.ShutdownNow(session)
-
 		if err := session.dropAndReset(); err != nil {
 			session.logError(err)
 		}
-
-		sm.setState(session, notSessionTime{})
+		sm.setState(session, latentState{})
 	}
 }
 
@@ -55,10 +58,10 @@ func (sm *stateMachine) handleDisconnectState(s *session) {
 
 	switch s.State.(type) {
 	case logoutState:
-		s.application.OnLogout(s.sessionID)
+		doOnLogout = true
 	case logonState:
 		if s.initiateLogon {
-			s.application.OnLogout(s.sessionID)
+			doOnLogout = true
 		}
 	}
 
diff --git a/session_test.go b/session_test.go
index b42c2714..2d3db575 100644
--- a/session_test.go
+++ b/session_test.go
@@ -278,39 +278,186 @@ func (s *CheckSessionTimeTestSuite) SetupTest() {
 }
 
 func (s *CheckSessionTimeTestSuite) TestNoStartTimeEndTime() {
-	s.session.sessionTime = nil
-	s.session.CheckSessionTime(s.session, time.Now())
-	s.State(latentState{})
+	var tests = []struct {
+		before, after sessionState
+	}{
+		{before: latentState{}},
+		{before: logonState{}},
+		{before: logoutState{}},
+		{before: inSession{}},
+		{before: resendState{}},
+		{before: pendingTimeout{resendState{}}},
+		{before: pendingTimeout{inSession{}}},
+		{before: notSessionTime{}, after: latentState{}},
+	}
+
+	for _, test := range tests {
+		s.SetupTest()
+		s.session.sessionTime = nil
+		s.session.State = test.before
+
+		s.session.CheckSessionTime(s.session, time.Now())
+		if test.after != nil {
+			s.State(test.after)
+		} else {
+			s.State(test.before)
+		}
+	}
 }
 
 func (s *CheckSessionTimeTestSuite) TestInRange() {
-	now := time.Now()
-	s.session.sessionTime = &internal.TimeRange{
-		StartTime: internal.NewTimeOfDay(now.Add(time.Duration(-1) * time.Hour).UTC().Clock()),
-		EndTime:   internal.NewTimeOfDay(now.Add(time.Hour).UTC().Clock()),
+	var tests = []struct {
+		before, after sessionState
+		expectReset   bool
+	}{
+		{before: latentState{}},
+		{before: logonState{}},
+		{before: logoutState{}},
+		{before: inSession{}},
+		{before: resendState{}},
+		{before: pendingTimeout{resendState{}}},
+		{before: pendingTimeout{inSession{}}},
+		{before: notSessionTime{}, after: latentState{}, expectReset: true},
+	}
+
+	for _, test := range tests {
+		s.SetupTest()
+		s.session.State = test.before
+
+		now := time.Now()
+		store := new(memoryStore)
+		if test.before.IsSessionTime() {
+			store.Reset()
+		} else {
+			store.creationTime = now.Add(time.Duration(-1) * time.Minute)
+		}
+		s.session.store = store
+		s.Nil(s.session.store.IncrNextSenderMsgSeqNum())
+		s.Nil(s.session.store.IncrNextTargetMsgSeqNum())
+
+		s.session.sessionTime = &internal.TimeRange{
+			StartTime: internal.NewTimeOfDay(now.UTC().Clock()),
+			EndTime:   internal.NewTimeOfDay(now.Add(time.Hour).UTC().Clock()),
+		}
+
+		s.session.CheckSessionTime(s.session, now)
+		if test.after != nil {
+			s.State(test.after)
+		} else {
+			s.State(test.before)
+		}
+
+		if test.expectReset {
+			s.ExpectStoreReset()
+		} else {
+			s.NextSenderMsgSeqNum(2)
+			s.NextSenderMsgSeqNum(2)
+		}
 	}
-	s.session.CheckSessionTime(s.session, now)
-	s.State(latentState{})
 }
 
 func (s *CheckSessionTimeTestSuite) TestNotInRange() {
-	now := time.Now()
-	s.session.sessionTime = &internal.TimeRange{
-		StartTime: internal.NewTimeOfDay(now.Add(time.Hour).UTC().Clock()),
-		EndTime:   internal.NewTimeOfDay(now.Add(time.Duration(2) * time.Hour).UTC().Clock()),
+	var tests = []struct {
+		before           sessionState
+		initiateLogon    bool
+		expectOnLogout   bool
+		expectSendLogout bool
+	}{
+		{before: latentState{}},
+		{before: logonState{}},
+		{before: logonState{}, initiateLogon: true, expectOnLogout: true},
+		{before: logoutState{}, expectOnLogout: true},
+		{before: inSession{}, expectOnLogout: true, expectSendLogout: true},
+		{before: resendState{}, expectOnLogout: true, expectSendLogout: true},
+		{before: pendingTimeout{resendState{}}, expectOnLogout: true, expectSendLogout: true},
+		{before: pendingTimeout{inSession{}}, expectOnLogout: true, expectSendLogout: true},
+		{before: notSessionTime{}},
+	}
+
+	for _, test := range tests {
+		s.SetupTest()
+		s.session.State = test.before
+		s.session.initiateLogon = test.initiateLogon
+		s.Nil(s.session.store.IncrNextSenderMsgSeqNum())
+		s.Nil(s.session.store.IncrNextTargetMsgSeqNum())
+
+		now := time.Now()
+		s.session.sessionTime = &internal.TimeRange{
+			StartTime: internal.NewTimeOfDay(now.Add(time.Hour).UTC().Clock()),
+			EndTime:   internal.NewTimeOfDay(now.Add(time.Duration(2) * time.Hour).UTC().Clock()),
+		}
+
+		if test.expectOnLogout {
+			s.mockApp.On("OnLogout")
+		}
+		if test.expectSendLogout {
+			s.mockApp.On("ToAdmin")
+		}
+		s.session.CheckSessionTime(s.session, now)
+
+		s.mockApp.AssertExpectations(s.T())
+		s.State(notSessionTime{})
+
+		s.NextTargetMsgSeqNum(2)
+		if test.expectSendLogout {
+			s.LastToAdminMessageSent()
+			s.MessageType(enum.MsgType_LOGOUT, s.mockApp.lastToAdmin)
+			s.NextSenderMsgSeqNum(3)
+		} else {
+			s.NextSenderMsgSeqNum(2)
+		}
 	}
-	s.session.CheckSessionTime(s.session, now)
-	s.State(notSessionTime{})
 }
 
 func (s *CheckSessionTimeTestSuite) TestInRangeButNotSameRangeAsStore() {
-	now := time.Now()
-	s.session.sessionTime = &internal.TimeRange{
-		StartTime: internal.NewTimeOfDay(now.Add(time.Duration(-1) * time.Hour).UTC().Clock()),
-		EndTime:   internal.NewTimeOfDay(now.Add(time.Hour).UTC().Clock()),
+	var tests = []struct {
+		before           sessionState
+		initiateLogon    bool
+		expectOnLogout   bool
+		expectSendLogout bool
+	}{
+		{before: latentState{}},
+		{before: logonState{}},
+		{before: logonState{}, initiateLogon: true, expectOnLogout: true},
+		{before: logoutState{}, expectOnLogout: true},
+		{before: inSession{}, expectOnLogout: true, expectSendLogout: true},
+		{before: resendState{}, expectOnLogout: true, expectSendLogout: true},
+		{before: pendingTimeout{resendState{}}, expectOnLogout: true, expectSendLogout: true},
+		{before: pendingTimeout{inSession{}}, expectOnLogout: true, expectSendLogout: true},
+		{before: notSessionTime{}},
+	}
+
+	for _, test := range tests {
+		s.SetupTest()
+		s.session.State = test.before
+		s.session.initiateLogon = test.initiateLogon
+		s.store.Reset()
+		s.Nil(s.session.store.IncrNextSenderMsgSeqNum())
+		s.Nil(s.session.store.IncrNextTargetMsgSeqNum())
+
+		now := time.Now()
+		s.session.sessionTime = &internal.TimeRange{
+			StartTime: internal.NewTimeOfDay(now.Add(time.Duration(-1) * time.Hour).UTC().Clock()),
+			EndTime:   internal.NewTimeOfDay(now.Add(time.Hour).UTC().Clock()),
+		}
+
+		if test.expectOnLogout {
+			s.mockApp.On("OnLogout")
+		}
+		if test.expectSendLogout {
+			s.mockApp.On("ToAdmin")
+		}
+		s.session.CheckSessionTime(s.session, now.AddDate(0, 0, 1))
+
+		s.mockApp.AssertExpectations(s.T())
+		s.State(latentState{})
+		if test.expectSendLogout {
+			s.LastToAdminMessageSent()
+			s.MessageType(enum.MsgType_LOGOUT, s.mockApp.lastToAdmin)
+			s.FieldEquals(tagMsgSeqNum, 2, s.mockApp.lastToAdmin.Header)
+		}
+		s.ExpectStoreReset()
 	}
-	s.session.CheckSessionTime(s.session, now.AddDate(0, 0, 1))
-	s.State(notSessionTime{})
 }
 
 type SessionSendTestSuite struct {
-- 
GitLab