Skip to content

Commit

Permalink
Fix tests timeout, WatchGameServer recent feature (googleforgames#1674)
Browse files Browse the repository at this point in the history
* Fix tests timeout, WatchGameServer recent feature

Move GetGameServer() call out of streamMutex lock.
Add previously writtent tests to run without this feature, as they
timeouts.
  • Loading branch information
aLekSer authored and ilkercelikyilmaz committed Oct 23, 2020
1 parent 4b19091 commit 5587d99
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 4 deletions.
2 changes: 1 addition & 1 deletion pkg/sdkserver/sdkserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,6 @@ func (s *SDKServer) GetGameServer(context.Context, *sdk.Empty) (*sdk.GameServer,
// backing GameServer configuration / status
func (s *SDKServer) WatchGameServer(_ *sdk.Empty, stream sdk.SDK_WatchGameServerServer) error {
s.logger.Debug("Received WatchGameServer request, adding stream to connectedStreams")
s.streamMutex.Lock()

if runtime.FeatureEnabled(runtime.FeatureSDKWatchSendOnExecute) {
gs, err := s.GetGameServer(context.Background(), &sdk.Empty{})
Expand All @@ -511,6 +510,7 @@ func (s *SDKServer) WatchGameServer(_ *sdk.Empty, stream sdk.SDK_WatchGameServer
}
}

s.streamMutex.Lock()
s.connectedStreams = append(s.connectedStreams, stream)
s.streamMutex.Unlock()
// don't exit until we shutdown, because that will close the stream
Expand Down
34 changes: 31 additions & 3 deletions pkg/sdkserver/sdkserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,14 @@ func TestSDKServerGetGameServer(t *testing.T) {

func TestSDKServerWatchGameServer(t *testing.T) {
t.Parallel()

agruntime.FeatureTestMutex.Lock()
defer agruntime.FeatureTestMutex.Unlock()
err := agruntime.ParseFeatures(string(agruntime.FeatureSDKWatchSendOnExecute) + "=false")
if err != nil {
assert.FailNow(t, "Can not parse FeatureSDKWatchSendOnExecute")
}

m := agtesting.NewMocks()
sc, err := defaultSidecar(m)
assert.Nil(t, err)
Expand Down Expand Up @@ -686,8 +694,8 @@ func TestSDKServerWatchGameServerFeatureSDKWatchSendOnExecute(t *testing.T) {
m.AgonesClient.AddWatchReactor("gameservers", k8stesting.DefaultWatchReactor(fakeWatch, nil))

err := agruntime.ParseFeatures(string(agruntime.FeatureSDKWatchSendOnExecute) + "=true")
if !assert.NoError(t, err) {
t.Fatal("Can not parse FeatureSDKWatchSendOnExecute")
if err != nil {
assert.FailNow(t, "Can not parse FeatureSDKWatchSendOnExecute")
}

sc, err := defaultSidecar(m)
Expand Down Expand Up @@ -745,6 +753,14 @@ func TestSDKServerWatchGameServerFeatureSDKWatchSendOnExecute(t *testing.T) {

func TestSDKServerSendGameServerUpdate(t *testing.T) {
t.Parallel()

agruntime.FeatureTestMutex.Lock()
defer agruntime.FeatureTestMutex.Unlock()
err := agruntime.ParseFeatures(string(agruntime.FeatureSDKWatchSendOnExecute) + "=false")
if err != nil {
assert.FailNow(t, "Can not parse FeatureSDKWatchSendOnExecute")
}

m := agtesting.NewMocks()
sc, err := defaultSidecar(m)
assert.Nil(t, err)
Expand Down Expand Up @@ -774,8 +790,17 @@ func TestSDKServerSendGameServerUpdate(t *testing.T) {

func TestSDKServerUpdateEventHandler(t *testing.T) {
t.Parallel()
m := agtesting.NewMocks()

// Acquire lock in order to be sure that
// no other parallel test turn on FeatureSDKWatchSendOnExecute
agruntime.FeatureTestMutex.Lock()
defer agruntime.FeatureTestMutex.Unlock()
err := agruntime.ParseFeatures(string(agruntime.FeatureSDKWatchSendOnExecute) + "=false")
if err != nil {
assert.FailNow(t, "Can not parse FeatureSDKWatchSendOnExecute")
}

m := agtesting.NewMocks()
fakeWatch := watch.NewFake()
m.AgonesClient.AddWatchReactor("gameservers", k8stesting.DefaultWatchReactor(fakeWatch, nil))

Expand Down Expand Up @@ -1271,6 +1296,9 @@ func waitConnectedStreamCount(sc *SDKServer, count int) error {
}

func asyncWatchGameServer(t *testing.T, sc *SDKServer, stream sdk.SDK_WatchGameServerServer) {
// Note that new FeatureSDKWatchSendOnExecute feature gate
// uses getGameServer() function and therefore WatchGameServer()
// would block if gsWaitForSync is not Done().
go func() {
err := sc.WatchGameServer(&sdk.Empty{}, stream)
assert.Nil(t, err)
Expand Down

0 comments on commit 5587d99

Please sign in to comment.