From c308919de1c84a81d08b5bf4209af9ec2b0f18b3 Mon Sep 17 00:00:00 2001 From: Alexey Kremsa Date: Tue, 28 Apr 2020 00:29:18 +0300 Subject: [PATCH] applied review notes applied review notes --- pkg/apis/agones/v1/common.go | 2 +- pkg/apis/agones/v1/gameserver.go | 2 +- pkg/apis/agones/v1/gameserver_test.go | 29 +++++++++++++++++---------- test/e2e/fleet_test.go | 2 +- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/pkg/apis/agones/v1/common.go b/pkg/apis/agones/v1/common.go index b632f053eb..82f463fb68 100644 --- a/pkg/apis/agones/v1/common.go +++ b/pkg/apis/agones/v1/common.go @@ -28,7 +28,7 @@ import ( // Block of const Error messages const ( ErrContainerRequired = "Container is required when using multiple containers in the pod template" - ErrHostPortDynamic = "HostPort cannot be specified with a Dynamic or Passthrough PortPolicy" + ErrHostPort = "HostPort cannot be specified with a Dynamic or Passthrough PortPolicy" ErrPortPolicyStatic = "PortPolicy must be Static" ErrContainerPortRequired = "ContainerPort must be defined for Dynamic and Static PortPolicies" ErrContainerPortPassthrough = "ContainerPort cannot be specified with Passthrough PortPolicy" diff --git a/pkg/apis/agones/v1/gameserver.go b/pkg/apis/agones/v1/gameserver.go index a4c95aa552..ba97fae70e 100644 --- a/pkg/apis/agones/v1/gameserver.go +++ b/pkg/apis/agones/v1/gameserver.go @@ -447,7 +447,7 @@ func (gss *GameServerSpec) Validate(devAddress string) ([]metav1.StatusCause, bo causes = append(causes, metav1.StatusCause{ Type: metav1.CauseTypeFieldValueInvalid, Field: fmt.Sprintf("%s.hostPort", p.Name), - Message: ErrHostPortDynamic, + Message: ErrHostPort, }) } diff --git a/pkg/apis/agones/v1/gameserver_test.go b/pkg/apis/agones/v1/gameserver_test.go index d2fdea3011..4f203b4713 100644 --- a/pkg/apis/agones/v1/gameserver_test.go +++ b/pkg/apis/agones/v1/gameserver_test.go @@ -1036,7 +1036,7 @@ func TestGameServerValidate(t *testing.T) { } } -func TestGameServerValidate_Features(t *testing.T) { +func TestGameServerValidateFeatures(t *testing.T) { t.Parallel() runtime.FeatureTestMutex.Lock() defer runtime.FeatureTestMutex.Unlock() @@ -1177,7 +1177,7 @@ func TestGameServerValidate_Features(t *testing.T) { } } -func TestGameServerPod_NoErrors(t *testing.T) { +func TestGameServerPodNoErrors(t *testing.T) { t.Parallel() fixture := defaultGameServer() fixture.ApplyDefaults() @@ -1498,12 +1498,15 @@ func TestGameServerIsBeforeReady(t *testing.T) { func TestGameServerApplyToPodContainer(t *testing.T) { t.Parallel() + type expected struct { + err string + tty bool + } var testCases = []struct { description string gs *GameServer - errExpected string - ttyExpected bool + expected expected }{ { description: "OK, no error", @@ -1520,8 +1523,10 @@ func TestGameServerApplyToPodContainer(t *testing.T) { }, }, }, - errExpected: "", - ttyExpected: true, + expected: expected{ + err: "", + tty: true, + }, }, { description: "container not found, error is returned", @@ -1538,8 +1543,10 @@ func TestGameServerApplyToPodContainer(t *testing.T) { }, }, }, - errExpected: "failed to find container named mycontainer-WRONG-NAME in pod spec", - ttyExpected: false, + expected: expected{ + err: "failed to find container named mycontainer-WRONG-NAME in pod spec", + tty: false, + }, }, } @@ -1552,10 +1559,10 @@ func TestGameServerApplyToPodContainer(t *testing.T) { return c }) - if tc.errExpected != "" && assert.NotNil(t, result) { - assert.Equal(t, tc.errExpected, result.Error()) + if tc.expected.err != "" && assert.NotNil(t, result) { + assert.Equal(t, tc.expected.err, result.Error()) } - assert.Equal(t, tc.ttyExpected, pod.Spec.Containers[0].TTY) + assert.Equal(t, tc.expected.tty, pod.Spec.Containers[0].TTY) assert.False(t, pod.Spec.Containers[1].TTY) }) } diff --git a/test/e2e/fleet_test.go b/test/e2e/fleet_test.go index cfec5b0649..ec1eb3d3dd 100644 --- a/test/e2e/fleet_test.go +++ b/test/e2e/fleet_test.go @@ -627,7 +627,7 @@ func TestFleetGSSpecValidation(t *testing.T) { statusErr, ok = err.(*k8serrors.StatusError) assert.True(t, ok) assert.Len(t, statusErr.Status().Details.Causes, 1) - assert.Equal(t, agonesv1.ErrHostPortDynamic, statusErr.Status().Details.Causes[0].Message) + assert.Equal(t, agonesv1.ErrHostPort, statusErr.Status().Details.Causes[0].Message) fltPort.Spec.Template.Spec.Ports[0].PortPolicy = agonesv1.Static fltPort.Spec.Template.Spec.Ports[0].HostPort = 0