Skip to content

Commit abbc949

Browse files
andrskomandrskom
authored
Merge pull request #5 from qa-dev/bugfix
refactoring mistake
2 parents d606558 + a6de600 commit abbc949

File tree

6 files changed

+178
-20
lines changed

6 files changed

+178
-20
lines changed

‎handlers/createSession.go‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,7 @@ func (h *CreateSession) tryCreateSession(r *http.Request, capabilities *capabili
7979
if err != nil {
8080
return nil, errors.New("reserve node error: " + err.Error())
8181
}
82-
//todo: посылать в мониторинг событие, если вернулся не 0
83-
seleniumClient := h.ClientFactory.Create(node.Address)
84-
seleniumNode := jsonwire.NewNode(seleniumClient)
85-
_, err = seleniumNode.RemoveAllSessions()
86-
if err != nil {
87-
return nil, errors.New("Can't remove all sessions from node: " + err.Error() + ", go to next available node: " + node.String())
88-
}
82+
8983
reverseProxy := httputil.NewSingleHostReverseProxy(&url.URL{
9084
Scheme: "http",
9185
Host: node.Address,

‎pool/strategy/persistent/factory.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,10 @@ func (f *StrategyFactory) Create(
1313
capsComparator capabilities.ComparatorInterface,
1414
clientFactory jsonwire.ClientFactoryInterface,
1515
) (pool.StrategyInterface, error) {
16-
return &Strategy{storage, capsComparator, clientFactory}, nil
16+
return &Strategy{
17+
storage,
18+
capsComparator,
19+
clientFactory,
20+
new(nodeHelperFactory),
21+
}, nil
1722
}

‎jsonwire/node.go‎ renamed to ‎pool/strategy/persistent/nodeHelper.go‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
1-
package jsonwire
1+
package persistent
22

33
import (
44
"fmt"
5+
"github.com/qa-dev/jsonwire-grid/jsonwire"
56
)
67

7-
type Node struct {
8-
client ClientInterface
8+
type nodeHelperFactory struct{}
9+
10+
func (f *nodeHelperFactory) create(abstractClient jsonwire.ClientInterface) sessionsRemover {
11+
return &nodeHelper{client: abstractClient}
912
}
1013

11-
func NewNode(abstractClient ClientInterface) *Node {
12-
return &Node{client: abstractClient}
14+
type nodeHelper struct {
15+
client jsonwire.ClientInterface
1316
}
1417

15-
func (n *Node) RemoveAllSessions() (int, error) {
18+
func (n *nodeHelper) removeAllSessions() (int, error) {
1619
message, err := n.client.Sessions()
1720
if err != nil {
1821
return 0, fmt.Errorf("Can't get sessions, %s", err.Error())

‎pool/strategy/persistent/nodeHelper_test.go‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package persistent
2+
3+
import (
4+
"testing"
5+
"github.com/stretchr/testify/assert"
6+
"github.com/qa-dev/jsonwire-grid/jsonwire"
7+
"encoding/json"
8+
"github.com/stretchr/testify/mock"
9+
"errors"
10+
)
11+
12+
func TestNodeHelperFactory_create(t *testing.T) {
13+
nhf := new(nodeHelperFactory)
14+
nodeHelper := nhf.create(nil)
15+
assert.NotNil(t, nodeHelper)
16+
}
17+
18+
func TestNodeHelper_removeAllSessions_Positive_NothingToRemove(t *testing.T) {
19+
cm := new(jsonwire.ClientMock)
20+
nodeHelper := &nodeHelper{cm}
21+
message := new(jsonwire.Sessions)
22+
cm.On("Sessions").Return(message, nil)
23+
_, err := nodeHelper.removeAllSessions()
24+
assert.Nil(t, err)
25+
}
26+
27+
func TestNodeHelper_removeAllSessions_Positive_SuccessRemove(t *testing.T) {
28+
cm := new(jsonwire.ClientMock)
29+
nodeHelper := &nodeHelper{cm}
30+
sessions := new(jsonwire.Sessions)
31+
sessions.Value = []struct {
32+
ID string `json:"id"`
33+
Capabilities json.RawMessage `json:"capabilities"`
34+
}{
35+
{ID: "lrololo", Capabilities: nil},
36+
}
37+
cm.On("Sessions").Return(sessions, nil)
38+
message := new(jsonwire.Message)
39+
cm.On("CloseSession", mock.AnythingOfType("string")).Return(message, nil)
40+
_, err := nodeHelper.removeAllSessions()
41+
assert.Nil(t, err)
42+
}
43+
44+
func TestNodeHelper_removeAllSessions_Negative_Sessions_Error(t *testing.T) {
45+
cm := new(jsonwire.ClientMock)
46+
nodeHelper := &nodeHelper{cm}
47+
cm.On("Sessions").Return(new(jsonwire.Sessions), errors.New("Err"))
48+
_, err := nodeHelper.removeAllSessions()
49+
assert.NotNil(t, err)
50+
}
51+
52+
func TestNodeHelper_removeAllSessions_Negative_Sessions_MessageStatusNotOk(t *testing.T) {
53+
cm := new(jsonwire.ClientMock)
54+
nodeHelper := &nodeHelper{cm}
55+
sessions := new(jsonwire.Sessions)
56+
sessions.Status = 99999
57+
cm.On("Sessions").Return(new(jsonwire.Sessions), errors.New("Err"))
58+
_, err := nodeHelper.removeAllSessions()
59+
assert.NotNil(t, err)
60+
}
61+
62+
func TestNodeHelper_removeAllSessions_Negative_CloseSession_Error(t *testing.T) {
63+
cm := new(jsonwire.ClientMock)
64+
nodeHelper := &nodeHelper{cm}
65+
sessions := new(jsonwire.Sessions)
66+
sessions.Value = []struct {
67+
ID string `json:"id"`
68+
Capabilities json.RawMessage `json:"capabilities"`
69+
}{
70+
{ID: "lrololo", Capabilities: nil},
71+
}
72+
cm.On("Sessions").Return(sessions, nil)
73+
message := new(jsonwire.Message)
74+
cm.On("CloseSession", mock.AnythingOfType("string")).Return(message, errors.New("Err"))
75+
_, err := nodeHelper.removeAllSessions()
76+
assert.NotNil(t, err)
77+
}
78+
79+
func TestNodeHelper_removeAllSessions_Negative_CloseSession_MessageStatusNotOk(t *testing.T) {
80+
cm := new(jsonwire.ClientMock)
81+
nodeHelper := &nodeHelper{cm}
82+
sessions := new(jsonwire.Sessions)
83+
sessions.Value = []struct {
84+
ID string `json:"id"`
85+
Capabilities json.RawMessage `json:"capabilities"`
86+
}{
87+
{ID: "lrololo", Capabilities: nil},
88+
}
89+
cm.On("Sessions").Return(sessions, nil)
90+
message := new(jsonwire.Message)
91+
message.Status = 999999
92+
cm.On("CloseSession", mock.AnythingOfType("string")).Return(message, errors.New("Err"))
93+
_, err := nodeHelper.removeAllSessions()
94+
assert.NotNil(t, err)
95+
}
96+

‎pool/strategy/persistent/strategy.go‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,19 @@ import (
99
"github.com/qa-dev/jsonwire-grid/pool/strategy"
1010
)
1111

12+
type sessionsRemoverFactory interface {
13+
create(abstractClient jsonwire.ClientInterface) sessionsRemover
14+
}
15+
16+
type sessionsRemover interface {
17+
removeAllSessions() (int, error)
18+
}
19+
1220
type Strategy struct {
13-
storage pool.StorageInterface
14-
capsComparator capabilities.ComparatorInterface
15-
clientFactory jsonwire.ClientFactoryInterface
21+
storage pool.StorageInterface
22+
capsComparator capabilities.ComparatorInterface
23+
clientFactory jsonwire.ClientFactoryInterface
24+
sessionsRemoverFactory sessionsRemoverFactory
1625
}
1726

1827
func (s *Strategy) Reserve(desiredCaps capabilities.Capabilities) (pool.Node, error) {
@@ -43,10 +52,20 @@ func (s *Strategy) Reserve(desiredCaps capabilities.Capabilities) (pool.Node, er
4352
}
4453
continue
4554
}
55+
4656
//todo: заменить магические числа на константы статусов
47-
if message.Status == 0 { // status == ok
48-
return node, nil
57+
if message.Status != 0 { // status != ok
58+
continue
4959
}
60+
61+
seleniumNode := s.sessionsRemoverFactory.create(client)
62+
//todo: посылать в мониторинг событие, если вернулся не 0
63+
_, err = seleniumNode.removeAllSessions()
64+
if err != nil {
65+
return pool.Node{}, errors.New("remove all existing sessions from node, " + err.Error())
66+
}
67+
68+
return node, nil
5069
}
5170

5271
return pool.Node{}, strategy.ErrNotFound

‎pool/strategy/persistent/strategy_test.go‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,23 @@ import (
1111
"testing"
1212
)
1313

14+
type sessionsRemoverMockFactory struct {
15+
sessionsRemover sessionsRemover
16+
}
17+
18+
func (f *sessionsRemoverMockFactory) create(abstractClient jsonwire.ClientInterface) sessionsRemover {
19+
return f.sessionsRemover
20+
}
21+
22+
type sessionsRemoverMock struct {
23+
mock.Mock
24+
}
25+
26+
func (r *sessionsRemoverMock) removeAllSessions() (int, error) {
27+
args := r.Called()
28+
return args.Int(0), args.Error(1)
29+
}
30+
1431
func TestStrategy_Reserve_Positive(t *testing.T) {
1532
sm := new(pool.StorageMock)
1633
expectedNode := pool.Node{CapabilitiesList: []capabilities.Capabilities{{"cap1": "cal1"}}}
@@ -24,7 +41,10 @@ func TestStrategy_Reserve_Positive(t *testing.T) {
2441
cfm.On("Create", mock.AnythingOfType("string")).Return(clm)
2542
message := new(jsonwire.Message)
2643
clm.On("Status").Return(message, nil)
27-
s := Strategy{storage: sm, capsComparator: cm, clientFactory: cfm}
44+
srm := new(sessionsRemoverMock)
45+
srm.On("removeAllSessions").Return(0, nil)
46+
srfm := &sessionsRemoverMockFactory{srm}
47+
s := Strategy{storage: sm, capsComparator: cm, clientFactory: cfm, sessionsRemoverFactory: srfm}
2848
node, err := s.Reserve(capabilities.Capabilities{})
2949
assert.Nil(t, err)
3050
assert.Equal(t, expectedNode, node)
@@ -95,6 +115,27 @@ func TestStrategy_Reserve_Negative_Client_Status_NotOk(t *testing.T) {
95115
assert.NotNil(t, err)
96116
}
97117

118+
func TestStrategy_Reserve_Negative_removeAllSessions_Error(t *testing.T) {
119+
sm := new(pool.StorageMock)
120+
sm.On("GetAll").Return([]pool.Node{{}}, nil)
121+
cm := new(capabilities.ComparatorMock)
122+
cm.On("Compare", mock.AnythingOfType("capabilities.Capabilities"), mock.AnythingOfType("capabilities.Capabilities")).Return(true)
123+
expectedNode := pool.Node{CapabilitiesList: []capabilities.Capabilities{{"cap1": "cal1"}}}
124+
sm.On("ReserveAvailable", mock.AnythingOfType("[]pool.Node")).Return([]pool.Node{expectedNode}, nil)
125+
cfm := new(jsonwire.ClientFactoryMock)
126+
clm := new(jsonwire.ClientMock)
127+
cfm.On("Create", mock.AnythingOfType("string")).Return(clm)
128+
message := new(jsonwire.Message)
129+
clm.On("Status").Return(message, nil)
130+
eError := errors.New("Error")
131+
srm := new(sessionsRemoverMock)
132+
srm.On("removeAllSessions").Return(0, eError)
133+
srfm := &sessionsRemoverMockFactory{srm}
134+
s := Strategy{storage: sm, capsComparator: cm, clientFactory: cfm, sessionsRemoverFactory: srfm}
135+
_, err := s.Reserve(capabilities.Capabilities{})
136+
assert.NotNil(t, err)
137+
}
138+
98139
func TestStrategy_CleanUp_Positive(t *testing.T) {
99140
sm := new(pool.StorageMock)
100141
sm.On("SetAvailable", mock.AnythingOfType("pool.Node")).Return(nil)

0 commit comments

Comments
 (0)