Skip to content

Commit 463cc00

Browse files
SoManyHsSoManyHs
committed
Refactor service_test
Refactored `upServiceWithCurrentTaskDefTest` and `upServiceWithNewTaskDefTest` to simply `updateServiceTest` to more closely mirror `createServiceTest`. Note: updateServiceTest only accounts for updating existing services. Does not test code path in which `compose service up` is used to create a new service.
1 parent 2620dbc commit 463cc00

File tree

1 file changed

+33
-103
lines changed

1 file changed

+33
-103
lines changed

‎ecs-cli/modules/cli/compose/entity/service/service_test.go

Lines changed: 33 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ import (
3131
"github.com/urfave/cli"
3232
)
3333

34+
const (
35+
arnPrefix = "arn:aws:ecs:us-west-2:accountId:task-definition/"
36+
)
37+
3438
//////////////////////////
3539
// Create Service tests //
3640
/////////////////////////
@@ -483,7 +487,7 @@ func TestCreateWithServiceDiscoveryWithContainerNameAndPort(t *testing.T) {
483487

484488
//////////////////////////////////////
485489
// Helpers for CreateService tests //
486-
//////////////////////////////////////
490+
/////////////////////////////////////
487491
type validateCreateServiceInputField func(*ecs.CreateServiceInput)
488492

489493
func createServiceTest(t *testing.T,
@@ -498,6 +502,7 @@ func createServiceTest(t *testing.T,
498502
taskDefID := "taskDefinitionId"
499503
taskDefArn, taskDefinition, registerTaskDefResponse := getTestTaskDef(taskDefID)
500504

505+
// Mock ECS calls
501506
mockEcs := mock_ecs.NewMockECSClient(ctrl)
502507
gomock.InOrder(
503508
mockEcs.EXPECT().RegisterTaskDefinitionIfNeeded(
@@ -516,7 +521,6 @@ func createServiceTest(t *testing.T,
516521
)
517522

518523
cliContext := cli.NewContext(nil, flagSet, nil)
519-
520524
context := &context.ECSContext{
521525
ECSClient: mockEcs,
522526
CommandConfig: commandConfig,
@@ -556,7 +560,6 @@ func getCreateServiceWithDelayMockClient(t *testing.T,
556560
gomock.InOrder(
557561
mockEcs.EXPECT().DescribeService(gomock.Any()).Return(getDescribeServiceTestResponse(nil), nil),
558562

559-
560563
mockEcs.EXPECT().RegisterTaskDefinitionIfNeeded(
561564
gomock.Any(), // RegisterTaskDefinitionInput
562565
gomock.Any(), // taskDefinitionCache
@@ -637,7 +640,6 @@ func TestLoadContext(t *testing.T) {
637640
"DeploymentConfig.MaxPercent should match")
638641
assert.Nil(t, observedDeploymentConfig.MinimumHealthyPercent,
639642
"DeploymentConfig.MinimumHealthyPercent should be nil")
640-
641643
}
642644

643645
func TestLoadContextForIncorrectInput(t *testing.T) {
@@ -683,7 +685,6 @@ func TestServiceInfo(t *testing.T) {
683685
}, t, true)
684686
}
685687

686-
687688
////////////////
688689
// Run tests //
689690
///////////////
@@ -715,7 +716,6 @@ func TestUpdateExistingServiceWithForceFlag(t *testing.T) {
715716

716717
flagSet := flag.NewFlagSet("ecs-cli-up", 0)
717718
flagSet.Bool(flags.ForceDeploymentFlag, forceFlagValue, "")
718-
cliContext := cli.NewContext(nil, flagSet, nil)
719719

720720
// define existing service
721721
serviceName := "test-service"
@@ -732,8 +732,7 @@ func TestUpdateExistingServiceWithForceFlag(t *testing.T) {
732732
expectedInput.forceDeployment = forceFlagValue
733733

734734
// call tests
735-
upServiceWithCurrentTaskDefTest(t, cliContext, &config.CommandConfig{}, &utils.ECSParams{}, expectedInput, existingService)
736-
upServiceWithNewTaskDefTest(t, cliContext, &config.CommandConfig{}, &utils.ECSParams{}, expectedInput, existingService)
735+
updateServiceTest(t, flagSet, &config.CommandConfig{}, &utils.ECSParams{}, expectedInput, existingService)
737736
}
738737

739738
func TestUpdateExistingServiceWithNewDeploymentConfig(t *testing.T) {
@@ -744,7 +743,6 @@ func TestUpdateExistingServiceWithNewDeploymentConfig(t *testing.T) {
744743
flagSet := flag.NewFlagSet("ecs-cli-up", 0)
745744
flagSet.String(flags.DeploymentMaxPercentFlag, strconv.Itoa(deploymentMaxPercent), "")
746745
flagSet.String(flags.DeploymentMinHealthyPercentFlag, strconv.Itoa(deploymentMinPercent), "")
747-
cliContext := cli.NewContext(nil, flagSet, nil)
748746

749747
// define existing service
750748
serviceName := "test-service"
@@ -764,8 +762,7 @@ func TestUpdateExistingServiceWithNewDeploymentConfig(t *testing.T) {
764762
}
765763

766764
// call tests
767-
upServiceWithCurrentTaskDefTest(t, cliContext, &config.CommandConfig{}, &utils.ECSParams{}, expectedInput, existingService)
768-
upServiceWithNewTaskDefTest(t, cliContext, &config.CommandConfig{}, &utils.ECSParams{}, expectedInput, existingService)
765+
updateServiceTest(t, flagSet, &config.CommandConfig{}, &utils.ECSParams{}, expectedInput, existingService)
769766
}
770767

771768
func TestUpdateExistingServiceWithNewHCGP(t *testing.T) {
@@ -774,7 +771,6 @@ func TestUpdateExistingServiceWithNewHCGP(t *testing.T) {
774771

775772
flagSet := flag.NewFlagSet("ecs-cli-up", 0)
776773
flagSet.String(flags.HealthCheckGracePeriodFlag, strconv.Itoa(healthCheckGracePeriod), "")
777-
cliContext := cli.NewContext(nil, flagSet, nil)
778774

779775
// define existing service
780776
serviceName := "test-service"
@@ -792,16 +788,14 @@ func TestUpdateExistingServiceWithNewHCGP(t *testing.T) {
792788
expectedInput.healthCheckGracePeriod = aws.Int64(int64(healthCheckGracePeriod))
793789

794790
// call tests
795-
upServiceWithCurrentTaskDefTest(t, cliContext, &config.CommandConfig{}, &utils.ECSParams{}, expectedInput, existingService)
796-
upServiceWithNewTaskDefTest(t, cliContext, &config.CommandConfig{}, &utils.ECSParams{}, expectedInput, existingService)
791+
updateServiceTest(t, flagSet, &config.CommandConfig{}, &utils.ECSParams{}, expectedInput, existingService)
797792
}
798793

799794
func TestUpdateExistingServiceWithDesiredCountOverOne(t *testing.T) {
800795
// define test values
801796
existingDesiredCount := 2
802797

803798
flagSet := flag.NewFlagSet("ecs-cli-up", 0)
804-
cliContext := cli.NewContext(nil, flagSet, nil)
805799

806800
// define existing service
807801
serviceName := "test-service"
@@ -818,8 +812,7 @@ func TestUpdateExistingServiceWithDesiredCountOverOne(t *testing.T) {
818812
expectedInput.count = *aws.Int64(int64(existingDesiredCount))
819813

820814
// call tests
821-
upServiceWithCurrentTaskDefTest(t, cliContext, &config.CommandConfig{}, &utils.ECSParams{}, expectedInput, existingService)
822-
upServiceWithNewTaskDefTest(t, cliContext, &config.CommandConfig{}, &utils.ECSParams{}, expectedInput, existingService)
815+
updateServiceTest(t, flagSet, &config.CommandConfig{}, &utils.ECSParams{}, expectedInput, existingService)
823816
}
824817

825818
///////////////////////////////////////
@@ -833,15 +826,25 @@ func getDefaultUpdateInput() UpdateServiceParams {
833826
}
834827
}
835828

836-
// helper for upServiceWithNewTaskDefTest and upServiceWithCurrentTaskDefTest
837-
func getUpdateServiceMockClient(t *testing.T,
838-
ctrl *gomock.Controller,
839-
describeServiceResponse *ecs.DescribeServicesOutput,
840-
taskDefinition ecs.TaskDefinition,
841-
registerTaskDefResponse ecs.TaskDefinition,
842-
expectedInput UpdateServiceParams) *mock_ecs.MockECSClient {
829+
// Tests only existing services, e.g. updateService private method rather than
830+
// the public Up method, which would potentially create a new service if it
831+
// does not already exist.
832+
func updateServiceTest(t *testing.T,
833+
flagSet *flag.FlagSet,
834+
commandConfig *config.CommandConfig,
835+
ecsParams *utils.ECSParams,
836+
expectedInput UpdateServiceParams,
837+
existingService *ecs.Service) {
838+
839+
ctrl := gomock.NewController(t)
840+
defer ctrl.Finish()
841+
842+
taskDefID := entity.GetIdFromArn(existingService.TaskDefinition)
843+
taskDefArn, taskDefinition, registerTaskDefResponse := getTestTaskDef(taskDefID)
843844

845+
// Mock ECS calls
844846
mockEcs := mock_ecs.NewMockECSClient(ctrl)
847+
describeServiceResponse := getDescribeServiceTestResponse(existingService)
845848
gomock.InOrder(
846849
mockEcs.EXPECT().DescribeService(gomock.Any()).Return(describeServiceResponse, nil),
847850

@@ -869,43 +872,16 @@ func getUpdateServiceMockClient(t *testing.T,
869872

870873
}).Return(nil),
871874
)
872-
return mockEcs
873-
}
874-
875-
func upServiceWithCurrentTaskDefTest(t *testing.T,
876-
cliContext *cli.Context,
877-
commandConfig *config.CommandConfig,
878-
ecsParams *utils.ECSParams,
879-
expectedInput UpdateServiceParams,
880-
existingService *ecs.Service) {
881-
882-
ctrl := gomock.NewController(t)
883-
defer ctrl.Finish()
884-
885-
// set up expected task def
886-
taskDefID := "taskDefinitionId"
887-
if existingService != nil {
888-
// set to existing if provided
889-
taskDefID = entity.GetIdFromArn(existingService.TaskDefinition)
890-
}
891-
taskDefArn, taskDefinition, registerTaskDefResponse := getTestTaskDef(taskDefID)
892-
893-
// set up DescribeService() response
894-
describeServiceResponse := getDescribeServiceTestResponse(existingService)
895-
896-
mockEcs := getUpdateServiceMockClient(t, ctrl, describeServiceResponse,
897-
taskDefinition, registerTaskDefResponse, expectedInput)
898875

876+
cliContext := cli.NewContext(nil, flagSet, nil)
899877
ecsContext := &context.ECSContext{
900878
ECSClient: mockEcs,
901879
CommandConfig: commandConfig,
902880
CLIContext: cliContext,
903881
ECSParams: ecsParams,
904882
}
905-
// if taskDef is unchanged, serviceName is taken from current context
906-
if existingService != nil {
907-
ecsContext.ProjectName = *existingService.ServiceName
908-
}
883+
884+
ecsContext.ProjectName = *existingService.ServiceName
909885
service := NewService(ecsContext)
910886
err := service.LoadContext()
911887
assert.NoError(t, err, "Unexpected error while loading context in update service with current task def test")
@@ -918,52 +894,6 @@ func upServiceWithCurrentTaskDefTest(t *testing.T,
918894
assert.Equal(t, taskDefArn, aws.StringValue(service.TaskDefinition().TaskDefinitionArn), "TaskDefArn should match")
919895
}
920896

921-
func upServiceWithNewTaskDefTest(t *testing.T,
922-
cliContext *cli.Context,
923-
commandConfig *config.CommandConfig,
924-
ecsParams *utils.ECSParams,
925-
expectedInput UpdateServiceParams,
926-
existingService *ecs.Service) {
927-
928-
ctrl := gomock.NewController(t)
929-
defer ctrl.Finish()
930-
931-
// set up expected (new) task def
932-
taskDefID := "newTaskDefinitionId"
933-
taskDefArn, taskDefinition, registerTaskDefResponse := getTestTaskDef(taskDefID)
934-
935-
// expect input to include new task def
936-
expectedInput.taskDefinition = taskDefID
937-
938-
// set up DescribeService() response
939-
describeServiceResponse := getDescribeServiceTestResponse(existingService)
940-
941-
mockEcs := getUpdateServiceMockClient(t, ctrl, describeServiceResponse,
942-
taskDefinition, registerTaskDefResponse, expectedInput)
943-
944-
ecsContext := &context.ECSContext{
945-
ECSClient: mockEcs,
946-
CommandConfig: commandConfig,
947-
CLIContext: cliContext,
948-
ECSParams: ecsParams,
949-
}
950-
951-
952-
953-
954-
955-
service := NewService(ecsContext)
956-
err := service.LoadContext()
957-
assert.NoError(t, err, "Unexpected error while loading context in update service with new task def test")
958-
959-
service.SetTaskDefinition(&taskDefinition)
960-
err = service.Up()
961-
assert.NoError(t, err, "Unexpected error on service up with new task def")
962-
963-
// task definition should be set
964-
assert.Equal(t, taskDefArn, aws.StringValue(service.TaskDefinition().TaskDefinitionArn), "TaskDefArn should match")
965-
}
966-
967897
func getDescribeServiceTestResponse(existingService *ecs.Service) *ecs.DescribeServicesOutput {
968898
if existingService != nil {
969899
return &ecs.DescribeServicesOutput{
@@ -985,16 +915,16 @@ func getDescribeServiceTestResponse(existingService *ecs.Service) *ecs.DescribeS
985915
}
986916

987917
func getTestTaskDef(taskDefID string) (taskDefArn string, taskDefinition, registerTaskDefResponse ecs.TaskDefinition) {
988-
taskDefArn = "arn/" + taskDefID
918+
taskDefArn = arnPrefix + taskDefID
989919
taskDefinition = ecs.TaskDefinition{
990-
Family: aws.String("family"),
920+
Family: aws.String(taskDefID),
991921
ContainerDefinitions: []*ecs.ContainerDefinition{},
992922
Volumes: []*ecs.Volume{},
993923
}
994924
registerTaskDefResponse = taskDefinition
995925
registerTaskDefResponse.TaskDefinitionArn = aws.String(taskDefArn)
996926

997-
return taskDefArn, taskDefinition, registerTaskDefResponse
927+
return
998928
}
999929

1000930
func verifyTaskDefinitionInput(t *testing.T,

0 commit comments

Comments
 (0)