Player Tracking Go SDK Conformance Tests #1527
Conversation
markmandel
added
kind/feature
markmandel
force-pushed
the
feature/go-sdk-conformance
branch
from
8b36358
to
da9cbe3
Compare
|
Build Failed 😱 Build Id: f251bdf4-d765-48ac-93a3-f7ef644a43a5 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Build Succeeded 👏 Build Id: 4d454845-45f7-4a56-8506-6e1ff85145bb The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
markmandel
force-pushed
the
feature/go-sdk-conformance
branch
from
da9cbe3
to
2989697
Compare
|
Build Succeeded 👏 Build Id: 10da5f08-9019-4c9d-83fa-2bcaa2ca1fd0 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
| @@ -34,8 +34,7 @@ SDK_FOLDER ?= go | |||
| COMMAND ?= gen | |||
| SDK_IMAGE_TAG=$(build_sdk_prefix)$(SDK_FOLDER):$(build_sdk_version) | |||
| DEFAULT_CONFORMANCE_TESTS = ready,allocate,setlabel,setannotation,gameserver,health,shutdown,watch,reserve | |||
| ALPHA_CONFORMANCE_TESTS = getplayercapacity,setplayercapacity | |||
|
|
|||
| ALPHA_CONFORMANCE_TESTS = getplayercapacity,setplayercapacity,playerconnect,playerdisconnect,getplayercount,isplayerconnected,getconnectedplayers | |||
There was a problem hiding this comment.
it looks like these don't have to be in the same order as the code. do we just check that they were all called?
There was a problem hiding this comment.
You are right, it doesn't matter the order they are called in - it checks that SDK methods are called with the expected arguments.
| log.Fatalf("PlayerDisconnect returned false") | ||
| } | ||
|
|
||
| if c, err = sdk.Alpha().GetPlayerCount(); err != nil { |
There was a problem hiding this comment.
I don't know if it would work, but it would be nice to call this once with the player connected (and verify that result is 1) and then again here to check that it is back to 0.
Similarly, it would be nice to verify that IsPlayerConnected returns false for a non-existent player and also that it returns false for "1234" after the player disconnects.
I'm sure this is all covered by unit tests, but it would be nice to have it covered by the conformance tests as well.
There was a problem hiding this comment.
This is definitely covered by the unit tests, but I can't see how we could do this with the current structure of the conformance tests. The conformance tests check that a method has been called once with a specific value, so there is no concept for ordering.
@aLekSer unless you can think of anything I'm not thinking of?
There was a problem hiding this comment.
That's what I thought. I don't want to block this change, but let's file an issue so we can discuss if it would be worth modifying the conformance test infra.
|
/assign |
Conformance tests for the Go SDK. Work on googleforgames#1507
markmandel
force-pushed
the
feature/go-sdk-conformance
branch
from
2989697
to
158607f
Compare
|
Build Succeeded 👏 Build Id: bbd68abf-eadd-4945-b8fa-071b1bcdf9ea The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: markmandel, roberthbailey 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:
Approvers can indicate their approval by writing |
Conformance tests for the Go SDK. Work on googleforgames#1507
What type of PR is this?
/kind feature
What this PR does / Why we need it:
Conformance tests for the Go SDK.
Which issue(s) this PR fixes:
Work on #1507
Special notes for your reviewer:
None.