Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Player Tracking Go SDK Conformance Tests #1527

Merged
merged 1 commit into from May 10, 2020
Merged

Player Tracking Go SDK Conformance Tests #1527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions build/includes/sdk.mk
View file
Open in desktop
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, it doesn't matter the order they are called in - it checks that SDK methods are called with the expected arguments.


.PHONY: test-sdks test-sdk build-sdks build-sdk gen-all-sdk-grpc gen-sdk-grpc run-all-sdk-command run-sdk-command build-example

Expand Down
31 changes: 31 additions & 0 deletions test/sdk/go/sdk-client-test.go
View file
Open in desktop
Expand Up @@ -106,6 +106,37 @@ func main() {
if c != capacity {
log.Fatalf("Player Capacity should be %d, but is %d", capacity, c)
}

playerID := "1234"
if ok, err := sdk.Alpha().PlayerConnect(playerID); err != nil {
log.Fatalf("Error registering player as connected: %s", err)
} else if !ok {
log.Fatalf("PlayerConnect returned false")
}

if ok, err := sdk.Alpha().IsPlayerConnected(playerID); err != nil {
log.Fatalf("Error checking if player is connected: %s", err)
} else if !ok {
log.Fatalf("IsPlayerConnected returned false")
}

if list, err := sdk.Alpha().GetConnectedPlayers(); err != nil {
log.Fatalf("Error getting connected player: %s", err)
} else if len(list) == 0 {
log.Fatalf("No connected players returned")
}

if ok, err := sdk.Alpha().PlayerDisconnect(playerID); err != nil {
log.Fatalf("Error registering player as disconnected: %s", err)
} else if !ok {
log.Fatalf("PlayerDisconnect returned false")
}

if c, err = sdk.Alpha().GetPlayerCount(); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

log.Fatalf("Error retrieving player count: %s", err)
} else if c != int64(0) {
log.Fatalf("Player Count should be 0, but is %d", c)
}
}

err = sdk.Shutdown()
Expand Down