Fix flaky Local SDK test #1586

aLekSer · 2020-05-26T17:08:39Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking
/kind bug
/kind cleanup
/kind documentation
/kind feature

/kind hotfix

What this PR does / Why we need it:
Fix LocalSDK test which is flaky.

Which issue(s) this PR fixes:

Special notes for your reviewer:

There is still a timeout possible here:

agones/pkg/sdkserver/localsdk_test.go

Line 281 in d30668c

assertWatchUpdate(t, stream, "bar", func(gs *sdk.GameServer) interface{} {

markmandel · 2020-05-26T17:10:25Z

pkg/sdkserver/localsdk_test.go

@@ -125,6 +125,8 @@ func TestLocalSDKServerSetLabel(t *testing.T) {
 	}

 	for k, v := range fixtures {
+		k := k


I've seen this before, and I feel like I've done similar to fix previous issues and I forget the reason for it. Can we add a note explaining?

yes, I will do (there is a gocritic linter article about this), btw on the multiple runs still get some errors. Will update the test one more time

It was not gocritic, but https://github.com/kyoh86/scopelint and it is obsolete, with two successors.

aLekSer · 2020-05-26T17:17:32Z

Strange enough I have received error here, something wrong with runtime.Features:

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1e977ec]

goroutine 541 [running]:
testing.tRunner.func1(0xc00040b300)
	/usr/local/Cellar/go/1.13.1/libexec/src/testing/testing.go:874 +0x3a3
panic(0x1ffa7e0, 0x2e23980)
	/usr/local/Cellar/go/1.13.1/libexec/src/runtime/panic.go:679 +0x1b2
agones.dev/agones/pkg/sdkserver.TestLocalSDKServerPlayerConnectAndDisconnect.func2(0xc00040b300)
	/Users/alexander.apalikov/go/src/github.com/agones/agones/pkg/sdkserver/localsdk_test.go:460 +0x61c
testing.tRunner(0xc00040b300, 0xc0008ea210)
	/usr/local/Cellar/go/1.13.1/libexec/src/testing/testing.go:909 +0xc9
created by testing.(*T).Run
	/usr/local/Cellar/go/1.13.1/libexec/src/testing/testing.go:960 +0x350
exit status 2
FAIL	agones.dev/agones/pkg/sdkserver	13.300s

markmandel · 2020-05-26T17:23:38Z

Strange enough I have received error here, something wrong with runtime.Features:

Seems like the feature flag got flipped mid test? Is there a test somewhere that's not been locked appropriately I wonder.

I wonder if we want to add some debug logging here and here to help with picking this up?

agones-bot · 2020-05-26T17:25:51Z

Build Succeeded 👏

Build Id: 35ca3c2b-c34d-4eca-9e42-c5600b8fbe21

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.6.0-d30668c
image: gcr.io/agones-images/agones-sdk:1.6.0-d30668c
image: gcr.io/agones-images/agones-ping:1.6.0-d30668c
Linux C++ SDK (build): agonessdk-1.6.0-d30668c-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.6.0-d30668c.zip

A preview of the website (the last 30 builds are retained):

https://d30668c-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1586/head:pr_1586 && git checkout pr_1586
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.6.0-d30668c

agones-bot · 2020-05-26T17:31:08Z

Build Succeeded 👏

Build Id: 050b18ef-9013-46e9-b506-9c43c8a8c039

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.6.0-76b69bc
image: gcr.io/agones-images/agones-sdk:1.6.0-76b69bc
image: gcr.io/agones-images/agones-ping:1.6.0-76b69bc
Linux C++ SDK (build): agonessdk-1.6.0-76b69bc-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.6.0-76b69bc.zip

A preview of the website (the last 30 builds are retained):

https://76b69bc-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1586/head:pr_1586 && git checkout pr_1586
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.6.0-76b69bc

aLekSer · 2020-05-26T17:56:32Z

Will add this comment here:

	runtime.FeatureTestMutex.Lock()
	defer runtime.FeatureTestMutex.Unlock()
	for k, v := range fixtures {
		k := k
		v := v
		t.Run(k, func(t *testing.T) {
			t.Parallel()
			runtime.FeatureTestMutex.Lock()
			defer runtime.FeatureTestMutex.Unlock()
...

This one above would not have a deadlock and without t.Parallel() it has. So nested parallel tests could use different variables set.

Line 281 is flaky.

aLekSer · 2020-05-26T18:11:25Z

I am running all tests in pkg/sdkserver/localsdk_test.go for 2k times.

agones-bot · 2020-05-26T18:18:15Z

Build Succeeded 👏

Build Id: a621a926-f4e1-4968-94cd-c27899d6a23a

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.6.0-0cc6037
image: gcr.io/agones-images/agones-sdk:1.6.0-0cc6037
image: gcr.io/agones-images/agones-ping:1.6.0-0cc6037
Linux C++ SDK (build): agonessdk-1.6.0-0cc6037-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.6.0-0cc6037.zip

A preview of the website (the last 30 builds are retained):

https://0cc6037-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1586/head:pr_1586 && git checkout pr_1586
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.6.0-0cc6037

aLekSer · 2020-05-26T18:19:08Z

pkg/sdkserver/localsdk_test.go

 		t.Run(k, func(t *testing.T) {
-			t.Parallel()


This line leads to unpredictable behaviour. I assume we can add this back in a separate PR.

agones-bot · 2020-05-26T18:25:34Z

Build Succeeded 👏

Build Id: 3221cbdb-084d-4482-b63b-3d0cd8963e10

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.6.0-095205f
image: gcr.io/agones-images/agones-sdk:1.6.0-095205f
image: gcr.io/agones-images/agones-ping:1.6.0-095205f
Linux C++ SDK (build): agonessdk-1.6.0-095205f-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.6.0-095205f.zip

A preview of the website (the last 30 builds are retained):

https://095205f-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1586/head:pr_1586 && git checkout pr_1586
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.6.0-095205f

markmandel

Nice cleanup!

google-oss-robot · 2020-05-26T18:54:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aLekSer, markmandel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [markmandel]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

aLekSer · 2020-05-26T19:41:28Z

One rare failed test still exist (1 out of 144 tests):

--- FAIL: TestLocal (0.00s)
    localsdk_test.go:67: 
        	Error Trace:	localsdk_test.go:67
        	Error:      	Not equal: 
        	            	expected: &sdk.GameServer_ObjectMeta{Name:"local", Namespace:"default", Uid:"1234", ResourceVersion:"v1", Generation:1, CreationTimestamp:1590521272, DeletionTimestamp:0, Annotations:map[string]string{"annotation":"true"}, Labels:map[string]string{"islocal":"true"}, XXX_NoUnkeyedLiteral:struct {}{}, XXX_unrecognized:[]uint8(nil), XXX_sizecache:0}
        	            	actual  : &sdk.GameServer_ObjectMeta{Name:"local", Namespace:"default", Uid:"1234", ResourceVersion:"v1", Generation:1, CreationTimestamp:1590521271, DeletionTimestamp:0, Annotations:map[string]string{"annotation":"true"}, Labels:map[string]string{"islocal":"true"}, XXX_NoUnkeyedLiteral:struct {}{}, XXX_unrecognized:[]uint8(nil), XXX_sizecache:0}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,2 +1,2 @@
        	            	-(*sdk.GameServer_ObjectMeta)(name:"local" namespace:"default" uid:"1234" resource_version:"v1" generation:1 creation_timestamp:1590521272 annotations:<key:"annotation" value:"true" > labels:<key:"islocal" value:"true" > )
        	            	+(*sdk.GameServer_ObjectMeta)(name:"local" namespace:"default" uid:"1234" resource_version:"v1" generation:1 creation_timestamp:1590521271 annotations:<key:"annotation" value:"true" > labels:<key:"islocal" value:"true" > )
        	            	 
        	Test:       	TestLocal

And localsdk_test.go:282 could timeout locally at least.

* Fix flaky Local SDK test * Extend Watch update to wait for file update Line 281 is flaky.

aLekSer added the area/tests Unit tests, e2e tests, anything to make sure things don't break label May 26, 2020

google-oss-robot requested review from EricFortin and roberthbailey May 26, 2020 17:08

google-oss-robot added the size/S label May 26, 2020

aLekSer force-pushed the fix/flaky-local-sdk-test branch from d30668c to 76b69bc Compare May 26, 2020 17:09

markmandel reviewed May 26, 2020

View reviewed changes

Fix flaky Local SDK test

0cc6037

aLekSer force-pushed the fix/flaky-local-sdk-test branch from 76b69bc to 0cc6037 Compare May 26, 2020 17:58

Extend Watch update to wait for file update

095205f

Line 281 is flaky.

aLekSer commented May 26, 2020

View reviewed changes

markmandel approved these changes May 26, 2020

View reviewed changes

google-oss-robot assigned markmandel May 26, 2020

google-oss-robot added the lgtm label May 26, 2020

google-oss-robot added the approved label May 26, 2020

markmandel merged commit 2f90dda into googleforgames:master May 26, 2020

markmandel added this to the 1.6.0 milestone May 26, 2020

ilkercelikyilmaz pushed a commit to ilkercelikyilmaz/agones that referenced this pull request Oct 23, 2020

Fix flaky Local SDK test (googleforgames#1586)

66fc12e

* Fix flaky Local SDK test * Extend Watch update to wait for file update Line 281 is flaky.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix flaky Local SDK test #1586

Fix flaky Local SDK test #1586

aLekSer commented May 26, 2020

markmandel May 26, 2020

aLekSer May 26, 2020

aLekSer May 26, 2020

aLekSer commented May 26, 2020

markmandel commented May 26, 2020

agones-bot commented May 26, 2020

agones-bot commented May 26, 2020

aLekSer commented May 26, 2020

aLekSer commented May 26, 2020 •

edited

Loading

agones-bot commented May 26, 2020

aLekSer May 26, 2020 •

edited

Loading

agones-bot commented May 26, 2020

markmandel left a comment

google-oss-robot commented May 26, 2020

aLekSer commented May 26, 2020

Fix flaky Local SDK test #1586

Fix flaky Local SDK test #1586

Conversation

aLekSer commented May 26, 2020

markmandel May 26, 2020

Choose a reason for hiding this comment

aLekSer May 26, 2020

Choose a reason for hiding this comment

aLekSer May 26, 2020

Choose a reason for hiding this comment

aLekSer commented May 26, 2020

markmandel commented May 26, 2020

agones-bot commented May 26, 2020

agones-bot commented May 26, 2020

aLekSer commented May 26, 2020

aLekSer commented May 26, 2020 • edited Loading

agones-bot commented May 26, 2020

aLekSer May 26, 2020 • edited Loading

Choose a reason for hiding this comment

agones-bot commented May 26, 2020

markmandel left a comment

Choose a reason for hiding this comment

google-oss-robot commented May 26, 2020

aLekSer commented May 26, 2020

aLekSer commented May 26, 2020 •

edited

Loading

aLekSer May 26, 2020 •

edited

Loading