googleforgames / agones

Dedicated Game Server Hosting and Scaling for Multiplayer Games on Kubernetes
https://agones.dev
Apache License 2.0
6.06k stars 805 forks source link

Flaky: SDK conformance tests #1452

Closed aLekSer closed 4 years ago

aLekSer commented 4 years ago

make run-sdk-conformance-test-go is failing pretty often on CI builds.

What happened:

One of Sidecar methods (Ready in the logs below) call was not counted to the result.

What you expected to happen:

No errors on SDK tests. Add readability to test failure output.

How to reproduce it (as minimally and precisely as possible): make run-sdk-conformance-test-go

Anything else we need to know?:

Logs from https://console.cloud.google.com/cloud-build/builds/309432a8-a2c6-4f45-95f3-8801af9f04da;step=23?project=agones-images

includes/sdk.mk:162: recipe for target 'run-sdk-conformance-test-go' failed
includes/sdk.mk:152: recipe for target 'run-sdk-conformance-test' failed
make[1]: *** [run-sdk-conformance-test] Error 2
includes/sdk.mk:145: recipe for target 'run-sdk-conformance-no-build' failed
make[2]: *** [run-sdk-conformance-no-build] Error 1
{"message":"Testing Failed [ready allocate setlabel setannotation gameserver health shutdown watch reserve] [watch reserve allocate gameserver health setlabel setannotation shutdown]","severity":"info","time":"2020-04-06T19:33:00.434023752Z"}
{"message":"Compare","severity":"info","time":"2020-04-06T19:33:00.433934282Z"}
{"message":"shutting down sdk server","severity":"info","source":"main","time":"2020-04-06T19:33:00.433799498Z"}

Environment:

markmandel commented 4 years ago

As part of this work, we might want to enhance the conformance tests to make it easier to work out which one is failing. Maybe we pass a name through to the local sdk binary, so it can output that at the same time a pass/fail occurs?

aLekSer commented 4 years ago

Yes, I was also thinking to pass some name and it would also adds up to the logging strings. docker run .... | sed -e 's/^/CPP conformance test: /;' this easier injecting log prefix does not work.

aLekSer commented 4 years ago

I think the failure with missing one of method calls caused by concurrent use of RecordRequest in localsdk.go:

func (l *LocalSDKServer) recordRequest(request string) {
    if l.testMode {
        l.requestSequence = append(l.requestSequence, request)

So there is an option to reuse gsMutex or add a new one. It seems that we lack some test which will detect this as a race condition. (Is there any thread safe slice available? )