From 1551ca3975ed50a5af3946bb8847e908888ce27e Mon Sep 17 00:00:00 2001 From: Tariq Hasan Date: Tue, 30 Apr 2024 10:38:50 -0400 Subject: [PATCH] Make test fields private in Go unit tests (#2316) Signed-off-by: tariq-hasan --- .../experiment/manifest/generator_test.go | 72 ++-- .../suggestionclient/suggestionclient_test.go | 328 ++++++++-------- .../experiment/validator/validator_test.go | 310 +++++++-------- .../v1beta1/pod/inject_webhook_test.go | 352 +++++++++--------- 4 files changed, 531 insertions(+), 531 deletions(-) diff --git a/pkg/controller.v1beta1/experiment/manifest/generator_test.go b/pkg/controller.v1beta1/experiment/manifest/generator_test.go index 3adeb017f74..c6ca427a3bc 100644 --- a/pkg/controller.v1beta1/experiment/manifest/generator_test.go +++ b/pkg/controller.v1beta1/experiment/manifest/generator_test.go @@ -89,23 +89,23 @@ func TestGetRunSpecWithHP(t *testing.T) { } tcs := []struct { - Instance *experimentsv1beta1.Experiment - ParameterAssignments []commonapiv1beta1.ParameterAssignment + instance *experimentsv1beta1.Experiment + parameterAssignments []commonapiv1beta1.ParameterAssignment expectedRunSpec *unstructured.Unstructured - Err bool + err bool testDescription string }{ // Valid run { - Instance: newFakeInstance(), - ParameterAssignments: newFakeParameterAssignment(), + instance: newFakeInstance(), + parameterAssignments: newFakeParameterAssignment(), expectedRunSpec: expectedRunSpec, - Err: false, + err: false, testDescription: "Run with valid parameters", }, // Invalid JSON in unstructured { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() trialSpec := i.Spec.TrialTemplate.TrialSource.TrialSpec trialSpec.Object = map[string]interface{}{ @@ -113,25 +113,25 @@ func TestGetRunSpecWithHP(t *testing.T) { } return i }(), - ParameterAssignments: newFakeParameterAssignment(), - Err: true, + parameterAssignments: newFakeParameterAssignment(), + err: true, testDescription: "Invalid JSON in Trial template", }, // len(parameterAssignment) != len(trialParameters) { - Instance: newFakeInstance(), - ParameterAssignments: func() []commonapiv1beta1.ParameterAssignment { + instance: newFakeInstance(), + parameterAssignments: func() []commonapiv1beta1.ParameterAssignment { pa := newFakeParameterAssignment() pa = pa[1:] return pa }(), - Err: true, + err: true, testDescription: "Number of parameter assignments is not equal to number of Trial parameters", }, // Parameter from assignments not found in Trial parameters { - Instance: newFakeInstance(), - ParameterAssignments: func() []commonapiv1beta1.ParameterAssignment { + instance: newFakeInstance(), + parameterAssignments: func() []commonapiv1beta1.ParameterAssignment { pa := newFakeParameterAssignment() pa[0] = commonapiv1beta1.ParameterAssignment{ Name: "invalid-name", @@ -139,17 +139,17 @@ func TestGetRunSpecWithHP(t *testing.T) { } return pa }(), - Err: true, + err: true, testDescription: "Trial parameters don't have parameter from assignments", }, } for _, tc := range tcs { - actualRunSpec, err := p.GetRunSpecWithHyperParameters(tc.Instance, "trial-name", "trial-namespace", tc.ParameterAssignments) + actualRunSpec, err := p.GetRunSpecWithHyperParameters(tc.instance, "trial-name", "trial-namespace", tc.parameterAssignments) - if tc.Err && err == nil { + if tc.err && err == nil { t.Errorf("Case: %v failed. Expected err, got nil", tc.testDescription) - } else if !tc.Err { + } else if !tc.err { if err != nil { t.Errorf("Case: %v failed. Expected nil, got %v", tc.testDescription, err) } else if !reflect.DeepEqual(tc.expectedRunSpec, actualRunSpec) { @@ -249,15 +249,15 @@ spec: } tcs := []struct { - Instance *experimentsv1beta1.Experiment - ParameterAssignments []commonapiv1beta1.ParameterAssignment - Err bool + instance *experimentsv1beta1.Experiment + parameterAssignments []commonapiv1beta1.ParameterAssignment + err bool testDescription string }{ // Valid run // validGetConfigMap1 case { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialSource = experimentsv1beta1.TrialSource{ ConfigMap: &experimentsv1beta1.ConfigMapSource{ @@ -268,14 +268,14 @@ spec: } return i }(), - ParameterAssignments: newFakeParameterAssignment(), - Err: false, + parameterAssignments: newFakeParameterAssignment(), + err: false, testDescription: "Run with valid parameters", }, // Invalid ConfigMap name // invalidConfigMapName case { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialSource = experimentsv1beta1.TrialSource{ ConfigMap: &experimentsv1beta1.ConfigMapSource{ @@ -284,14 +284,14 @@ spec: } return i }(), - ParameterAssignments: newFakeParameterAssignment(), - Err: true, + parameterAssignments: newFakeParameterAssignment(), + err: true, testDescription: "Invalid ConfigMap name", }, // Invalid template path in ConfigMap name // validGetConfigMap3 case { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialSource = experimentsv1beta1.TrialSource{ ConfigMap: &experimentsv1beta1.ConfigMapSource{ @@ -302,8 +302,8 @@ spec: } return i }(), - ParameterAssignments: newFakeParameterAssignment(), - Err: true, + parameterAssignments: newFakeParameterAssignment(), + err: true, testDescription: "Invalid template path in ConfigMap", }, // Invalid Trial template spec in ConfigMap @@ -311,7 +311,7 @@ spec: // Because of that, user can specify not valid unstructured template // invalidTemplate case { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialSource = experimentsv1beta1.TrialSource{ ConfigMap: &experimentsv1beta1.ConfigMapSource{ @@ -322,17 +322,17 @@ spec: } return i }(), - ParameterAssignments: newFakeParameterAssignment(), - Err: true, + parameterAssignments: newFakeParameterAssignment(), + err: true, testDescription: "Invalid Trial spec in ConfigMap", }, } for _, tc := range tcs { - actualRunSpec, err := p.GetRunSpecWithHyperParameters(tc.Instance, "trial-name", "trial-namespace", tc.ParameterAssignments) - if tc.Err && err == nil { + actualRunSpec, err := p.GetRunSpecWithHyperParameters(tc.instance, "trial-name", "trial-namespace", tc.parameterAssignments) + if tc.err && err == nil { t.Errorf("Case: %v failed. Expected err, got nil", tc.testDescription) - } else if !tc.Err { + } else if !tc.err { if err != nil { t.Errorf("Case: %v failed. Expected nil, got %v", tc.testDescription, err) } else if !reflect.DeepEqual(expectedRunSpec, actualRunSpec) { diff --git a/pkg/controller.v1beta1/suggestion/suggestionclient/suggestionclient_test.go b/pkg/controller.v1beta1/suggestion/suggestionclient/suggestionclient_test.go index 1015e5ed157..26fa209a32b 100644 --- a/pkg/controller.v1beta1/suggestion/suggestionclient/suggestionclient_test.go +++ b/pkg/controller.v1beta1/suggestion/suggestionclient/suggestionclient_test.go @@ -185,62 +185,62 @@ func TestSyncAssignments(t *testing.T) { ) tcs := []struct { - Experiment *experimentsv1beta1.Experiment - Suggestion *suggestionsv1beta1.Suggestion - Trials []trialsv1beta1.Trial - Err bool - TestDescription string + experiment *experimentsv1beta1.Experiment + suggestion *suggestionsv1beta1.Suggestion + trials []trialsv1beta1.Trial + err bool + testDescription string }{ // Experiment contains HP and NAS config just for the test purpose // validRunGetSuggestions + validRunGetEarlyStopRules case { - Experiment: newFakeExperiment(), - Suggestion: newFakeSuggestion(), - Trials: newFakeTrials(), - Err: false, - TestDescription: "SyncAssignments valid run", + experiment: newFakeExperiment(), + suggestion: newFakeSuggestion(), + trials: newFakeTrials(), + err: false, + testDescription: "SyncAssignments valid run", }, { - Suggestion: func() *suggestionsv1beta1.Suggestion { + suggestion: func() *suggestionsv1beta1.Suggestion { s := newFakeSuggestion() s.Spec.Requests = 4 s.Status.SuggestionCount = 6 return s }(), - Err: false, - TestDescription: "Negative request number", + err: false, + testDescription: "Negative request number", }, // getSuggestionsFail case { - Experiment: newFakeExperiment(), - Suggestion: newFakeSuggestion(), - Trials: newFakeTrials(), - Err: true, - TestDescription: "Unable to execute GetSuggestions", + experiment: newFakeExperiment(), + suggestion: newFakeSuggestion(), + trials: newFakeTrials(), + err: true, + testDescription: "Unable to execute GetSuggestions", }, // invalidAssignmentsCount case { - Experiment: newFakeExperiment(), - Suggestion: newFakeSuggestion(), - Trials: newFakeTrials(), - Err: true, - TestDescription: "ParameterAssignments from response != request number", + experiment: newFakeExperiment(), + suggestion: newFakeSuggestion(), + trials: newFakeTrials(), + err: true, + testDescription: "ParameterAssignments from response != request number", }, // validRunGetSuggestions2 + getEarlyStopRulesFail case { - Experiment: newFakeExperiment(), - Suggestion: newFakeSuggestion(), - Trials: newFakeTrials(), - Err: true, - TestDescription: "Unable to execute GetEarlyStoppingRules", + experiment: newFakeExperiment(), + suggestion: newFakeSuggestion(), + trials: newFakeTrials(), + err: true, + testDescription: "Unable to execute GetEarlyStoppingRules", }, } for _, tc := range tcs { - err := suggestionClient.SyncAssignments(tc.Suggestion, tc.Experiment, tc.Trials) - if !tc.Err && err != nil { - t.Errorf("Case: %v failed. Expected nil, got %v", tc.TestDescription, err) - } else if tc.Err && err == nil { - t.Errorf("Case: %v failed. Expected err, got nil", tc.TestDescription) + err := suggestionClient.SyncAssignments(tc.suggestion, tc.experiment, tc.trials) + if !tc.err && err != nil { + t.Errorf("Case: %v failed. Expected nil, got %v", tc.testDescription, err) + } else if tc.err && err == nil { + t.Errorf("Case: %v failed. Expected err, got nil", tc.testDescription) } } } @@ -286,46 +286,46 @@ func TestValidateAlgorithmSettings(t *testing.T) { unimplementedMethod) tcs := []struct { - Experiment *experimentsv1beta1.Experiment - Suggestion *suggestionsv1beta1.Suggestion - Err bool - TestDescription string + experiment *experimentsv1beta1.Experiment + suggestion *suggestionsv1beta1.Suggestion + err bool + testDescription string }{ // validRun case { - Experiment: exp, - Suggestion: sug, - Err: false, - TestDescription: "ValidateAlgorithmSettings valid run", + experiment: exp, + suggestion: sug, + err: false, + testDescription: "ValidateAlgorithmSettings valid run", }, // invalidExperiment case { - Experiment: exp, - Suggestion: sug, - Err: true, - TestDescription: "Invalid argument return in Experiment validation", + experiment: exp, + suggestion: sug, + err: true, + testDescription: "Invalid argument return in Experiment validation", }, // connectionError case { - Experiment: exp, - Suggestion: sug, - Err: true, - TestDescription: "Connection to suggestion service error", + experiment: exp, + suggestion: sug, + err: true, + testDescription: "Connection to suggestion service error", }, // unimplementedMethod case { - Experiment: exp, - Suggestion: sug, - Err: false, - TestDescription: "Unimplemented ValidateAlgorithmSettings method", + experiment: exp, + suggestion: sug, + err: false, + testDescription: "Unimplemented ValidateAlgorithmSettings method", }, } for _, tc := range tcs { - err := suggestionClient.ValidateAlgorithmSettings(tc.Suggestion, tc.Experiment) - if !tc.Err && err != nil { - t.Errorf("Case: %v failed. Expected nil, got %v", tc.TestDescription, err) - } else if tc.Err && err == nil { - t.Errorf("Case: %v failed. Expected err, got nil", tc.TestDescription) + err := suggestionClient.ValidateAlgorithmSettings(tc.suggestion, tc.experiment) + if !tc.err && err != nil { + t.Errorf("Case: %v failed. Expected nil, got %v", tc.testDescription, err) + } else if tc.err && err == nil { + t.Errorf("Case: %v failed. Expected err, got nil", tc.testDescription) } } } @@ -365,46 +365,46 @@ func TestValidateEarlyStoppingSettings(t *testing.T) { unimplementedMethod) tcs := []struct { - Experiment *experimentsv1beta1.Experiment - Suggestion *suggestionsv1beta1.Suggestion - Err bool - TestDescription string + experiment *experimentsv1beta1.Experiment + suggestion *suggestionsv1beta1.Suggestion + err bool + testDescription string }{ // validRun case { - Experiment: exp, - Suggestion: sug, - Err: false, - TestDescription: "ValidateEarlyStoppingSettings valid run", + experiment: exp, + suggestion: sug, + err: false, + testDescription: "ValidateEarlyStoppingSettings valid run", }, // invalidExperiment case { - Experiment: exp, - Suggestion: sug, - Err: true, - TestDescription: "Invalid argument return in Experiment validation", + experiment: exp, + suggestion: sug, + err: true, + testDescription: "Invalid argument return in Experiment validation", }, // connectionError case { - Experiment: exp, - Suggestion: sug, - Err: true, - TestDescription: "Connection to early stopping service error", + experiment: exp, + suggestion: sug, + err: true, + testDescription: "Connection to early stopping service error", }, // unimplementedMethod case { - Experiment: exp, - Suggestion: sug, - Err: false, - TestDescription: "Unimplemented ValidateEarlyStoppingSettings method", + experiment: exp, + suggestion: sug, + err: false, + testDescription: "Unimplemented ValidateEarlyStoppingSettings method", }, } for _, tc := range tcs { - err := suggestionClient.ValidateEarlyStoppingSettings(tc.Suggestion, tc.Experiment) - if !tc.Err && err != nil { - t.Errorf("Case: %v failed. Expected nil, got %v", tc.TestDescription, err) - } else if tc.Err && err == nil { - t.Errorf("Case: %v failed. Expected err, got nil", tc.TestDescription) + err := suggestionClient.ValidateEarlyStoppingSettings(tc.suggestion, tc.experiment) + if !tc.err && err != nil { + t.Errorf("Case: %v failed. Expected nil, got %v", tc.testDescription, err) + } else if tc.err && err == nil { + t.Errorf("Case: %v failed. Expected err, got nil", tc.testDescription) } } } @@ -412,50 +412,50 @@ func TestValidateEarlyStoppingSettings(t *testing.T) { func TestConvertTrialConditionType(t *testing.T) { tcs := []struct { - InCondition trialsv1beta1.TrialConditionType - ExpectedCondition suggestionapi.TrialStatus_TrialConditionType - TestDescription string + inCondition trialsv1beta1.TrialConditionType + expectedCondition suggestionapi.TrialStatus_TrialConditionType + testDescription string }{ { - InCondition: trialsv1beta1.TrialCreated, - ExpectedCondition: suggestionapi.TrialStatus_CREATED, - TestDescription: "Convert created Trial condition", + inCondition: trialsv1beta1.TrialCreated, + expectedCondition: suggestionapi.TrialStatus_CREATED, + testDescription: "Convert created Trial condition", }, { - InCondition: trialsv1beta1.TrialRunning, - ExpectedCondition: suggestionapi.TrialStatus_RUNNING, - TestDescription: "Convert running Trial condition", + inCondition: trialsv1beta1.TrialRunning, + expectedCondition: suggestionapi.TrialStatus_RUNNING, + testDescription: "Convert running Trial condition", }, { - InCondition: trialsv1beta1.TrialSucceeded, - ExpectedCondition: suggestionapi.TrialStatus_SUCCEEDED, - TestDescription: "Convert succeeded Trial condition", + inCondition: trialsv1beta1.TrialSucceeded, + expectedCondition: suggestionapi.TrialStatus_SUCCEEDED, + testDescription: "Convert succeeded Trial condition", }, { - InCondition: trialsv1beta1.TrialKilled, - ExpectedCondition: suggestionapi.TrialStatus_KILLED, - TestDescription: "Convert killed Trial condition", + inCondition: trialsv1beta1.TrialKilled, + expectedCondition: suggestionapi.TrialStatus_KILLED, + testDescription: "Convert killed Trial condition", }, { - InCondition: trialsv1beta1.TrialFailed, - ExpectedCondition: suggestionapi.TrialStatus_FAILED, - TestDescription: "Convert failed Trial condition", + inCondition: trialsv1beta1.TrialFailed, + expectedCondition: suggestionapi.TrialStatus_FAILED, + testDescription: "Convert failed Trial condition", }, { - InCondition: trialsv1beta1.TrialEarlyStopped, - ExpectedCondition: suggestionapi.TrialStatus_EARLYSTOPPED, - TestDescription: "Convert early stopped Trial condition", + inCondition: trialsv1beta1.TrialEarlyStopped, + expectedCondition: suggestionapi.TrialStatus_EARLYSTOPPED, + testDescription: "Convert early stopped Trial condition", }, { - InCondition: "Unknown", - ExpectedCondition: suggestionapi.TrialStatus_UNKNOWN, - TestDescription: "Convert unknown Trial condition", + inCondition: "Unknown", + expectedCondition: suggestionapi.TrialStatus_UNKNOWN, + testDescription: "Convert unknown Trial condition", }, } for _, tc := range tcs { - actualCondition := convertTrialConditionType(tc.InCondition) - if actualCondition != tc.ExpectedCondition { - t.Errorf("Case: %v failed. Expected Trial condition %v, got %v", tc.TestDescription, tc.ExpectedCondition, actualCondition) + actualCondition := convertTrialConditionType(tc.inCondition) + if actualCondition != tc.expectedCondition { + t.Errorf("Case: %v failed. Expected Trial condition %v, got %v", tc.testDescription, tc.expectedCondition, actualCondition) } } } @@ -463,31 +463,31 @@ func TestConvertTrialConditionType(t *testing.T) { func TestConvertObjectiveType(t *testing.T) { tcs := []struct { - InType commonapiv1beta1.ObjectiveType - ExpectedType suggestionapi.ObjectiveType - TestDescription string + inType commonapiv1beta1.ObjectiveType + expectedType suggestionapi.ObjectiveType + testDescription string }{ { - InType: commonv1beta1.ObjectiveTypeMaximize, - ExpectedType: suggestionapi.ObjectiveType_MAXIMIZE, - TestDescription: "Convert maximize objective type", + inType: commonv1beta1.ObjectiveTypeMaximize, + expectedType: suggestionapi.ObjectiveType_MAXIMIZE, + testDescription: "Convert maximize objective type", }, { - InType: commonv1beta1.ObjectiveTypeMinimize, - ExpectedType: suggestionapi.ObjectiveType_MINIMIZE, - TestDescription: "Convert minimize objective type", + inType: commonv1beta1.ObjectiveTypeMinimize, + expectedType: suggestionapi.ObjectiveType_MINIMIZE, + testDescription: "Convert minimize objective type", }, { - InType: commonv1beta1.ObjectiveTypeUnknown, - ExpectedType: suggestionapi.ObjectiveType_UNKNOWN, - TestDescription: "Convert unknown objective type", + inType: commonv1beta1.ObjectiveTypeUnknown, + expectedType: suggestionapi.ObjectiveType_UNKNOWN, + testDescription: "Convert unknown objective type", }, } for _, tc := range tcs { - actualType := convertObjectiveType(tc.InType) - if actualType != tc.ExpectedType { - t.Errorf("Case: %v failed. Expected objective type %v, got %v", tc.TestDescription, tc.ExpectedType, actualType) + actualType := convertObjectiveType(tc.inType) + if actualType != tc.expectedType { + t.Errorf("Case: %v failed. Expected objective type %v, got %v", tc.testDescription, tc.expectedType, actualType) } } } @@ -495,40 +495,40 @@ func TestConvertObjectiveType(t *testing.T) { func TestConvertParameterType(t *testing.T) { tcs := []struct { - InType experimentsv1beta1.ParameterType - ExpectedType suggestionapi.ParameterType - TestDescription string + inType experimentsv1beta1.ParameterType + expectedType suggestionapi.ParameterType + testDescription string }{ { - InType: experimentsv1beta1.ParameterTypeDiscrete, - ExpectedType: suggestionapi.ParameterType_DISCRETE, - TestDescription: "Convert discrete parameter type", + inType: experimentsv1beta1.ParameterTypeDiscrete, + expectedType: suggestionapi.ParameterType_DISCRETE, + testDescription: "Convert discrete parameter type", }, { - InType: experimentsv1beta1.ParameterTypeCategorical, - ExpectedType: suggestionapi.ParameterType_CATEGORICAL, - TestDescription: "Convert categorical parameter type", + inType: experimentsv1beta1.ParameterTypeCategorical, + expectedType: suggestionapi.ParameterType_CATEGORICAL, + testDescription: "Convert categorical parameter type", }, { - InType: experimentsv1beta1.ParameterTypeDouble, - ExpectedType: suggestionapi.ParameterType_DOUBLE, - TestDescription: "Convert double parameter type", + inType: experimentsv1beta1.ParameterTypeDouble, + expectedType: suggestionapi.ParameterType_DOUBLE, + testDescription: "Convert double parameter type", }, { - InType: experimentsv1beta1.ParameterTypeInt, - ExpectedType: suggestionapi.ParameterType_INT, - TestDescription: "Convert int parameter type", + inType: experimentsv1beta1.ParameterTypeInt, + expectedType: suggestionapi.ParameterType_INT, + testDescription: "Convert int parameter type", }, { - InType: experimentsv1beta1.ParameterTypeUnknown, - ExpectedType: suggestionapi.ParameterType_UNKNOWN_TYPE, - TestDescription: "Convert unknown parameter type", + inType: experimentsv1beta1.ParameterTypeUnknown, + expectedType: suggestionapi.ParameterType_UNKNOWN_TYPE, + testDescription: "Convert unknown parameter type", }, } for _, tc := range tcs { - actualType := convertParameterType(tc.InType) - if actualType != tc.ExpectedType { - t.Errorf("Case: %v failed. Expected parameter type %v, got %v", tc.TestDescription, tc.ExpectedType, actualType) + actualType := convertParameterType(tc.inType) + if actualType != tc.expectedType { + t.Errorf("Case: %v failed. Expected parameter type %v, got %v", tc.testDescription, tc.expectedType, actualType) } } } @@ -586,35 +586,35 @@ func TestConvertTrialObservation(t *testing.T) { func TestConvertComparison(t *testing.T) { tcs := []struct { - InComparison suggestionapi.ComparisonType - ExpectedComparison commonapiv1beta1.ComparisonType - TestDescription string + inComparison suggestionapi.ComparisonType + expectedComparison commonapiv1beta1.ComparisonType + testDescription string }{ { - InComparison: suggestionapi.ComparisonType_EQUAL, - ExpectedComparison: commonapiv1beta1.ComparisonTypeEqual, - TestDescription: "Convert equal comparison type", + inComparison: suggestionapi.ComparisonType_EQUAL, + expectedComparison: commonapiv1beta1.ComparisonTypeEqual, + testDescription: "Convert equal comparison type", }, { - InComparison: suggestionapi.ComparisonType_LESS, - ExpectedComparison: commonapiv1beta1.ComparisonTypeLess, - TestDescription: "Convert less comparison type", + inComparison: suggestionapi.ComparisonType_LESS, + expectedComparison: commonapiv1beta1.ComparisonTypeLess, + testDescription: "Convert less comparison type", }, { - InComparison: suggestionapi.ComparisonType_GREATER, - ExpectedComparison: commonapiv1beta1.ComparisonTypeGreater, - TestDescription: "Convert greater comparison type", + inComparison: suggestionapi.ComparisonType_GREATER, + expectedComparison: commonapiv1beta1.ComparisonTypeGreater, + testDescription: "Convert greater comparison type", }, { - InComparison: suggestionapi.ComparisonType_UNKNOWN_COMPARISON, - ExpectedComparison: commonapiv1beta1.ComparisonTypeEqual, - TestDescription: "Convert unknown comparison type", + inComparison: suggestionapi.ComparisonType_UNKNOWN_COMPARISON, + expectedComparison: commonapiv1beta1.ComparisonTypeEqual, + testDescription: "Convert unknown comparison type", }, } for _, tc := range tcs { - actualComparison := convertComparison(tc.InComparison) - if actualComparison != tc.ExpectedComparison { - t.Errorf("Case: %v failed. Expected comparison type %v, got %v", tc.TestDescription, tc.ExpectedComparison, actualComparison) + actualComparison := convertComparison(tc.inComparison) + if actualComparison != tc.expectedComparison { + t.Errorf("Case: %v failed. Expected comparison type %v, got %v", tc.testDescription, tc.expectedComparison, actualComparison) } } } diff --git a/pkg/webhook/v1beta1/experiment/validator/validator_test.go b/pkg/webhook/v1beta1/experiment/validator/validator_test.go index f17b886abe6..0bc33acbb8d 100644 --- a/pkg/webhook/v1beta1/experiment/validator/validator_test.go +++ b/pkg/webhook/v1beta1/experiment/validator/validator_test.go @@ -65,140 +65,140 @@ func TestValidateExperiment(t *testing.T) { fakeNegativeInt := int32(-1) tcs := []struct { - Instance *experimentsv1beta1.Experiment - Err bool + instance *experimentsv1beta1.Experiment + err bool oldInstance *experimentsv1beta1.Experiment testDescription string }{ // Name { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Name = "1234-test" return i }(), - Err: true, + err: true, testDescription: "Name is invalid", }, // Objective { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.Objective = nil return i }(), - Err: true, + err: true, testDescription: "Objective is nil", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.Objective.Type = commonv1beta1.ObjectiveTypeUnknown return i }(), - Err: true, + err: true, testDescription: "Objective type is unknown", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.Objective.ObjectiveMetricName = "" return i }(), - Err: true, + err: true, testDescription: "Objective metric name is empty", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.Objective.ObjectiveMetricName = "objective" i.Spec.Objective.AdditionalMetricNames = []string{"objective", "objective-1"} return i }(), - Err: true, + err: true, testDescription: "additionalMetricNames should not contain objective metric name", }, // Algorithm { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.Algorithm = nil return i }(), - Err: true, + err: true, testDescription: "Algorithm is nil", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.Algorithm.AlgorithmName = "" return i }(), - Err: true, + err: true, testDescription: "Algorithm name is empty", }, // EarlyStopping { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.EarlyStopping = nil return i }(), - Err: false, + err: false, testDescription: "EarlyStopping is nil", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.EarlyStopping.AlgorithmName = "" return i }(), - Err: true, + err: true, testDescription: "EarlyStopping AlgorithmName is empty", }, // Valid Experiment { - Instance: newFakeInstance(), - Err: false, + instance: newFakeInstance(), + err: false, testDescription: "Run validator for correct experiment", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MaxFailedTrialCount = &fakeNegativeInt return i }(), - Err: true, + err: true, testDescription: "Max failed trial count is negative", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MaxTrialCount = &fakeNegativeInt return i }(), - Err: true, + err: true, testDescription: "Max trial count is negative", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.ParallelTrialCount = &fakeNegativeInt return i }(), - Err: true, + err: true, testDescription: "Parallel trial count is negative", }, // Validate Resume Experiment { - Instance: newFakeInstance(), - Err: false, + instance: newFakeInstance(), + err: false, oldInstance: newFakeInstance(), testDescription: "Run validator to correct resume experiment", }, { - Instance: newFakeInstance(), - Err: true, + instance: newFakeInstance(), + err: true, oldInstance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.MarkExperimentStatusSucceeded(experimentutil.ExperimentMaxTrialsReachedReason, "Experiment is succeeded") @@ -208,8 +208,8 @@ func TestValidateExperiment(t *testing.T) { testDescription: "Resume succeeded experiment with ResumePolicy = NeverResume", }, { - Instance: newFakeInstance(), - Err: true, + instance: newFakeInstance(), + err: true, oldInstance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Status = experimentsv1beta1.ExperimentStatus{ @@ -222,8 +222,8 @@ func TestValidateExperiment(t *testing.T) { testDescription: "Resume experiment with MaxTrialCount <= Status.Trials", }, { - Instance: newFakeInstance(), - Err: true, + instance: newFakeInstance(), + err: true, oldInstance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.Algorithm.AlgorithmName = "not-test" @@ -232,27 +232,27 @@ func TestValidateExperiment(t *testing.T) { testDescription: "Change algorithm name when resuming experiment", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.ResumePolicy = "invalid-policy" return i }(), - Err: true, + err: true, testDescription: "Invalid resume policy", }, // Validate NAS Config { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.Parameters = []experimentsv1beta1.ParameterSpec{} i.Spec.NasConfig = nil return i }(), - Err: true, + err: true, testDescription: "Parameters and NAS config is nil", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.NasConfig = &experimentsv1beta1.NasConfig{ Operations: []experimentsv1beta1.Operation{ @@ -263,29 +263,29 @@ func TestValidateExperiment(t *testing.T) { } return i }(), - Err: true, + err: true, testDescription: "Parameters and NAS config is not nil", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate = nil return i }(), - Err: true, + err: true, testDescription: "Trial template is nil", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.Parameters[1].FeasibleSpace.Max = "5" return i }(), - Err: true, + err: true, testDescription: "Invalid feasible space in parameters", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { maxTrialCount := int32(5) invalidMaxFailedTrialCount := int32(6) i := newFakeInstance() @@ -293,11 +293,11 @@ func TestValidateExperiment(t *testing.T) { i.Spec.MaxFailedTrialCount = &invalidMaxFailedTrialCount return i }(), - Err: true, + err: true, testDescription: "maxFailedTrialCount greater than maxTrialCount", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { maxTrialCount := int32(5) validMaxFailedTrialCount := int32(5) i := newFakeInstance() @@ -305,11 +305,11 @@ func TestValidateExperiment(t *testing.T) { i.Spec.MaxFailedTrialCount = &validMaxFailedTrialCount return i }(), - Err: false, + err: false, testDescription: "maxFailedTrialCount equal to maxTrialCount", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { maxTrialCount := int32(5) invalidParallelTrialCount := int32(6) i := newFakeInstance() @@ -317,11 +317,11 @@ func TestValidateExperiment(t *testing.T) { i.Spec.ParallelTrialCount = &invalidParallelTrialCount return i }(), - Err: true, + err: true, testDescription: "parallelTrialCount greater than maxTrialCount", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { maxTrialCount := int32(5) validParallelTrialCount := int32(5) i := newFakeInstance() @@ -329,16 +329,16 @@ func TestValidateExperiment(t *testing.T) { i.Spec.ParallelTrialCount = &validParallelTrialCount return i }(), - Err: false, + err: false, testDescription: "parallelTrialCount equal to maxTrialCount", }, } for _, tc := range tcs { - err := g.ValidateExperiment(tc.Instance, tc.oldInstance) - if !tc.Err && err != nil { + err := g.ValidateExperiment(tc.instance, tc.oldInstance) + if !tc.err && err != nil { t.Errorf("Case: %v failed. Expected nil, got %v", tc.testDescription, err) - } else if tc.Err && err == nil { + } else if tc.err && err == nil { t.Errorf("Case: %v failed. Expected err, got nil", tc.testDescription) } } @@ -504,45 +504,45 @@ spec: ) tcs := []struct { - Instance *experimentsv1beta1.Experiment - Err bool + instance *experimentsv1beta1.Experiment + err bool testDescription string }{ // TrialParamters is nil { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialParameters = nil return i }(), - Err: true, + err: true, testDescription: "Trial parameters is nil", }, // TrialSpec and ConfigMap is nil { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialSpec = nil return i }(), - Err: true, + err: true, testDescription: "Trial spec nil", }, // TrialSpec and ConfigMap is not nil { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialSource.ConfigMap = &experimentsv1beta1.ConfigMapSource{ ConfigMapName: "config-map-name", } return i }(), - Err: true, + err: true, testDescription: "Trial spec and ConfigMap is not nil", }, // ConfigMap missed template path { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialSource = experimentsv1beta1.TrialSource{ ConfigMap: &experimentsv1beta1.ConfigMapSource{ @@ -552,13 +552,13 @@ spec: } return i }(), - Err: true, + err: true, testDescription: "Missed template path in ConfigMap", }, // Wrong path in configMap // emptyConfigMap case { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialSpec = nil i.Spec.TrialTemplate.TrialSource = experimentsv1beta1.TrialSource{ @@ -570,188 +570,188 @@ spec: } return i }(), - Err: true, + err: true, testDescription: "Wrong template path in ConfigMap", }, // Empty Reference or Name in trialParameters { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialParameters[0].Reference = "" return i }(), - Err: true, + err: true, testDescription: "Empty reference or name in Trial parameters", }, // Wrong Name in trialParameters { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialParameters[0].Name = "{invalid-name}" return i }(), - Err: true, + err: true, testDescription: "Wrong name in Trial parameters", }, // Duplicate Name in trialParameters { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialParameters[1].Name = i.Spec.TrialTemplate.TrialParameters[0].Name return i }(), - Err: true, + err: true, testDescription: "Duplicate name in Trial parameters", }, // Duplicate Reference in trialParameters { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialParameters[1].Reference = i.Spec.TrialTemplate.TrialParameters[0].Reference return i }(), - Err: true, + err: true, testDescription: "Duplicate reference in Trial parameters", }, // Trial template contains Trial parameters which weren't referenced from spec.parameters { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialParameters[1].Reference = "wrong-ref" return i }(), - Err: true, + err: true, testDescription: "Trial template contains Trial parameters which weren't referenced from spec.parameters", }, // Trial template contains Trial parameters when spec.parameters is empty { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.Parameters = nil i.Spec.TrialTemplate.TrialParameters[1].Reference = "wrong-ref" return i }(), - Err: false, + err: false, testDescription: "Trial template contains Trial parameters when spec.parameters is empty", }, // Trial template contains Trial metadata parameter substitution { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialParameters[1].Reference = "${trialSpec.Name}" return i }(), - Err: false, + err: false, testDescription: "Trial template contains Trial metadata reference as parameter", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialParameters[1].Reference = "${trialSpec.Annotations[test-annotation]}" return i }(), - Err: false, + err: false, testDescription: "Trial template contains Trial annotation reference as parameter", }, { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialParameters[1].Reference = "${trialSpec.Labels[test-label]}" return i }(), - Err: false, + err: false, testDescription: "Trial template contains Trial's label reference as parameter", }, // Trial Template doesn't contain parameter from trialParameters // missedParameterTemplate case { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() return i }(), - Err: true, + err: true, testDescription: "Trial template doesn't contain parameter from Trial parameters", }, // Trial Template contains extra parameter // oddParameterTemplate case { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() return i }(), - Err: true, + err: true, testDescription: "Trial template contains extra parameter", }, // Trial Template parameter is invalid after substitution // Unable convert string to unstructured // invalidParameterTemplate case { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() return i }(), - Err: true, + err: true, testDescription: "Trial template is unable to convert to unstructured after substitution", }, // Trial Template contains Name and Namespace // notEmptyMetadataTemplate case { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.TrialSpec.SetName("trial-name") i.Spec.TrialTemplate.TrialSpec.SetNamespace("trial-namespace") return i }(), - Err: true, + err: true, testDescription: "Trial template contains metadata.name or metadata.namespace", }, // Trial Template doesn't contain APIVersion or Kind // emptyAPIVersionTemplate case { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() return i }(), - Err: true, + err: true, testDescription: "Trial template doesn't contain APIVersion or Kind", }, // Trial Template has custom Kind // customJobTypeTemplate case { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() return i }(), - Err: false, + err: false, testDescription: "Trial template has custom Kind", }, // Trial Template doesn't have PrimaryContainerName { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.PrimaryContainerName = "" return i }(), - Err: true, + err: true, testDescription: "Trial template doesn't have PrimaryContainerName", }, // Trial Template doesn't have SuccessCondition { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.TrialTemplate.SuccessCondition = "" return i }(), - Err: true, + err: true, testDescription: "Trial template doesn't have SuccessCondition", }, } for _, tc := range tcs { - err := g.(*DefaultValidator).validateTrialTemplate(tc.Instance) - if !tc.Err && err != nil { + err := g.(*DefaultValidator).validateTrialTemplate(tc.instance) + if !tc.err && err != nil { t.Errorf("Case: %v failed. Expected nil, got %v", tc.testDescription, err) - } else if tc.Err && err == nil { + } else if tc.err && err == nil { t.Errorf("Case: %v failed. Expected err, got nil", tc.testDescription) } } @@ -823,14 +823,14 @@ spec: } tcs := []struct { - RunSpec *unstructured.Unstructured - Err bool + runSpec *unstructured.Unstructured + err bool testDescription string }{ // Invalid Field Batch Job { - RunSpec: invalidFieldBatchJobUnstr, - Err: true, + runSpec: invalidFieldBatchJobUnstr, + err: true, testDescription: "Trial template has invalid Batch Job parameter", }, // Invalid Structure Batch Job @@ -838,29 +838,29 @@ spec: // Patch must have only "remove" operations // Then all parameters from trial Template were correctly merged { - RunSpec: invalidStructureBatchJobUnstr, - Err: true, + runSpec: invalidStructureBatchJobUnstr, + err: true, testDescription: "Trial template has invalid Batch Job structure", }, // Valid case with not default Kubernetes resource (nvidia.com/gpu: 1) { - RunSpec: notDefaultResourceBatchUnstr, - Err: false, + runSpec: notDefaultResourceBatchUnstr, + err: false, testDescription: "Valid case with nvidia.com/gpu resource in Trial template", }, // Not kubernetes batch job { - RunSpec: notKubernetesBatchJobUnstr, - Err: false, + runSpec: notKubernetesBatchJobUnstr, + err: false, testDescription: "Only validate Kuernetes Job", }, } for _, tc := range tcs { - err := g.(*DefaultValidator).validateTrialJob(tc.RunSpec) - if !tc.Err && err != nil { + err := g.(*DefaultValidator).validateTrialJob(tc.runSpec) + if !tc.err && err != nil { t.Errorf("Case: %v failed. Expected nil, got %v", tc.testDescription, err) - } else if tc.Err && err == nil { + } else if tc.err && err == nil { t.Errorf("Case: %v failed. Expected err, got nil", tc.testDescription) } } @@ -881,13 +881,13 @@ func TestValidateMetricsCollector(t *testing.T) { p.EXPECT().GetMetricsCollectorConfigData(gomock.Any()).Return(metricsCollectorConfigData, nil).AnyTimes() tcs := []struct { - Instance *experimentsv1beta1.Experiment - Err bool + instance *experimentsv1beta1.Experiment + err bool testDescription string }{ // Invalid Metrics Collector Kind { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MetricsCollectorSpec = &commonv1beta1.MetricsCollectorSpec{ Collector: &commonv1beta1.CollectorSpec{ @@ -896,12 +896,12 @@ func TestValidateMetricsCollector(t *testing.T) { } return i }(), - Err: true, + err: true, testDescription: "Invalid metrics collector Kind", }, // FileCollector invalid Path { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MetricsCollectorSpec = &commonv1beta1.MetricsCollectorSpec{ Collector: &commonv1beta1.CollectorSpec{ @@ -916,12 +916,12 @@ func TestValidateMetricsCollector(t *testing.T) { } return i }(), - Err: true, + err: true, testDescription: "Invalid path for File metrics collector", }, // TfEventCollector invalid Path { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MetricsCollectorSpec = &commonv1beta1.MetricsCollectorSpec{ Collector: &commonv1beta1.CollectorSpec{ @@ -935,12 +935,12 @@ func TestValidateMetricsCollector(t *testing.T) { } return i }(), - Err: true, + err: true, testDescription: "Invalid path for TF event metrics collector", }, // TfEventCollector invalid file format { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MetricsCollectorSpec = &commonv1beta1.MetricsCollectorSpec{ Collector: &commonv1beta1.CollectorSpec{ @@ -956,12 +956,12 @@ func TestValidateMetricsCollector(t *testing.T) { } return i }(), - Err: true, + err: true, testDescription: "Invalid file format for TF event metrics collector", }, // PrometheusMetricCollector invalid Port { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MetricsCollectorSpec = &commonv1beta1.MetricsCollectorSpec{ Collector: &commonv1beta1.CollectorSpec{ @@ -977,12 +977,12 @@ func TestValidateMetricsCollector(t *testing.T) { } return i }(), - Err: true, + err: true, testDescription: "Invalid port for Prometheus metrics collector", }, // PrometheusMetricCollector invalid Path { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MetricsCollectorSpec = &commonv1beta1.MetricsCollectorSpec{ Collector: &commonv1beta1.CollectorSpec{ @@ -999,12 +999,12 @@ func TestValidateMetricsCollector(t *testing.T) { } return i }(), - Err: true, + err: true, testDescription: "Invalid path for Prometheus metrics collector", }, // CustomCollector empty CustomCollector { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MetricsCollectorSpec = &commonv1beta1.MetricsCollectorSpec{ Collector: &commonv1beta1.CollectorSpec{ @@ -1013,12 +1013,12 @@ func TestValidateMetricsCollector(t *testing.T) { } return i }(), - Err: true, + err: true, testDescription: "Empty container for Custom metrics collector", }, // CustomCollector invalid Path { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MetricsCollectorSpec = &commonv1beta1.MetricsCollectorSpec{ Collector: &commonv1beta1.CollectorSpec{ @@ -1035,12 +1035,12 @@ func TestValidateMetricsCollector(t *testing.T) { } return i }(), - Err: true, + err: true, testDescription: "Invalid path for Custom metrics collector", }, // FileMetricCollector invalid regexp in metrics format { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MetricsCollectorSpec = &commonv1beta1.MetricsCollectorSpec{ Collector: &commonv1beta1.CollectorSpec{ @@ -1061,12 +1061,12 @@ func TestValidateMetricsCollector(t *testing.T) { } return i }(), - Err: true, + err: true, testDescription: "Invalid metrics format regex for File metrics collector", }, // FileMetricCollector one subexpression in metrics format { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MetricsCollectorSpec = &commonv1beta1.MetricsCollectorSpec{ Collector: &commonv1beta1.CollectorSpec{ @@ -1087,12 +1087,12 @@ func TestValidateMetricsCollector(t *testing.T) { } return i }(), - Err: true, + err: true, testDescription: "One subexpression in metrics format", }, // FileMetricCollector invalid file format { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MetricsCollectorSpec = &commonv1beta1.MetricsCollectorSpec{ Collector: &commonv1beta1.CollectorSpec{ @@ -1108,12 +1108,12 @@ func TestValidateMetricsCollector(t *testing.T) { } return i }(), - Err: true, + err: true, testDescription: "Invalid file format for File metrics collector", }, // FileMetricCollector invalid metrics filter { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MetricsCollectorSpec = &commonv1beta1.MetricsCollectorSpec{ Collector: &commonv1beta1.CollectorSpec{ @@ -1130,12 +1130,12 @@ func TestValidateMetricsCollector(t *testing.T) { } return i }(), - Err: true, + err: true, testDescription: "Invalid metrics filer for File metrics collector when file format is `JSON`", }, // Valid FileMetricCollector { - Instance: func() *experimentsv1beta1.Experiment { + instance: func() *experimentsv1beta1.Experiment { i := newFakeInstance() i.Spec.MetricsCollectorSpec = &commonv1beta1.MetricsCollectorSpec{ Collector: &commonv1beta1.CollectorSpec{ @@ -1151,16 +1151,16 @@ func TestValidateMetricsCollector(t *testing.T) { } return i }(), - Err: false, + err: false, testDescription: "Run validator for correct File metrics collector", }, } for _, tc := range tcs { - err := g.(*DefaultValidator).validateMetricsCollector(tc.Instance) - if !tc.Err && err != nil { + err := g.(*DefaultValidator).validateMetricsCollector(tc.instance) + if !tc.err && err != nil { t.Errorf("Case: %v failed. Expected nil, got %v", tc.testDescription, err) - } else if tc.Err && err == nil { + } else if tc.err && err == nil { t.Errorf("Case: %v failed. Expected err, got nil", tc.testDescription) } } @@ -1199,25 +1199,25 @@ func TestValidateConfigData(t *testing.T) { p.EXPECT().GetTrialTemplate(gomock.Any()).Return(batchJobStr, nil).AnyTimes() tcs := []struct { - Instance *experimentsv1beta1.Experiment + instance *experimentsv1beta1.Experiment testDescription string }{ { - Instance: newFakeInstance(), + instance: newFakeInstance(), testDescription: "Get metrics collector config data error", }, { - Instance: newFakeInstance(), + instance: newFakeInstance(), testDescription: "Get early stopping config data error", }, { - Instance: newFakeInstance(), + instance: newFakeInstance(), testDescription: "Get suggestion config data error", }, } for _, tc := range tcs { - err := g.ValidateExperiment(tc.Instance, nil) + err := g.ValidateExperiment(tc.instance, nil) if err == nil { t.Errorf("Case: %v failed. Expected err, got nil", tc.testDescription) } diff --git a/pkg/webhook/v1beta1/pod/inject_webhook_test.go b/pkg/webhook/v1beta1/pod/inject_webhook_test.go index c3d456ac269..ab1646b6769 100644 --- a/pkg/webhook/v1beta1/pod/inject_webhook_test.go +++ b/pkg/webhook/v1beta1/pod/inject_webhook_test.go @@ -77,17 +77,17 @@ func TestWrapWorkerContainer(t *testing.T) { metricsFile := "metric.log" testCases := []struct { - Trial *trialsv1beta1.Trial - Pod *v1.Pod - MetricsFile string - PathKind common.FileSystemKind - ExpectedPod *v1.Pod - Err bool - TestDescription string + trial *trialsv1beta1.Trial + pod *v1.Pod + metricsFile string + pathKind common.FileSystemKind + expectedPod *v1.Pod + err bool + testDescription string }{ { - Trial: trial, - Pod: &v1.Pod{ + trial: trial, + pod: &v1.Pod{ Spec: v1.PodSpec{ Containers: []v1.Container{ { @@ -99,9 +99,9 @@ func TestWrapWorkerContainer(t *testing.T) { }, }, }, - MetricsFile: metricsFile, - PathKind: common.FileKind, - ExpectedPod: &v1.Pod{ + metricsFile: metricsFile, + pathKind: common.FileKind, + expectedPod: &v1.Pod{ Spec: v1.PodSpec{ Containers: []v1.Container{ { @@ -116,12 +116,12 @@ func TestWrapWorkerContainer(t *testing.T) { }, }, }, - Err: false, - TestDescription: "Tensorflow container without sh -c", + err: false, + testDescription: "Tensorflow container without sh -c", }, { - Trial: trial, - Pod: &v1.Pod{ + trial: trial, + pod: &v1.Pod{ Spec: v1.PodSpec{ Containers: []v1.Container{ { @@ -134,9 +134,9 @@ func TestWrapWorkerContainer(t *testing.T) { }, }, }, - MetricsFile: metricsFile, - PathKind: common.FileKind, - ExpectedPod: &v1.Pod{ + metricsFile: metricsFile, + pathKind: common.FileKind, + expectedPod: &v1.Pod{ Spec: v1.PodSpec{ Containers: []v1.Container{ { @@ -151,12 +151,12 @@ func TestWrapWorkerContainer(t *testing.T) { }, }, }, - Err: false, - TestDescription: "Tensorflow container with sh -c", + err: false, + testDescription: "Tensorflow container with sh -c", }, { - Trial: trial, - Pod: &v1.Pod{ + trial: trial, + pod: &v1.Pod{ Spec: v1.PodSpec{ Containers: []v1.Container{ { @@ -165,12 +165,12 @@ func TestWrapWorkerContainer(t *testing.T) { }, }, }, - PathKind: common.FileKind, - Err: true, - TestDescription: "Training pod doesn't have primary container", + pathKind: common.FileKind, + err: true, + testDescription: "Training pod doesn't have primary container", }, { - Trial: func() *trialsv1beta1.Trial { + trial: func() *trialsv1beta1.Trial { t := trial.DeepCopy() t.Spec.EarlyStoppingRules = []common.EarlyStoppingRule{ { @@ -181,7 +181,7 @@ func TestWrapWorkerContainer(t *testing.T) { } return t }(), - Pod: &v1.Pod{ + pod: &v1.Pod{ Spec: v1.PodSpec{ Containers: []v1.Container{ { @@ -193,9 +193,9 @@ func TestWrapWorkerContainer(t *testing.T) { }, }, }, - MetricsFile: metricsFile, - PathKind: common.FileKind, - ExpectedPod: &v1.Pod{ + metricsFile: metricsFile, + pathKind: common.FileKind, + expectedPod: &v1.Pod{ Spec: v1.PodSpec{ Containers: []v1.Container{ { @@ -214,21 +214,21 @@ func TestWrapWorkerContainer(t *testing.T) { }, }, }, - Err: false, - TestDescription: "Container with early stopping command", + err: false, + testDescription: "Container with early stopping command", }, } for _, c := range testCases { - err := wrapWorkerContainer(c.Trial, c.Pod, c.Trial.Namespace, c.MetricsFile, c.PathKind) - if c.Err && err == nil { - t.Errorf("Case %s failed. Expected error, got nil", c.TestDescription) - } else if !c.Err { + err := wrapWorkerContainer(c.trial, c.pod, c.trial.Namespace, c.metricsFile, c.pathKind) + if c.err && err == nil { + t.Errorf("Case %s failed. Expected error, got nil", c.testDescription) + } else if !c.err { if err != nil { - t.Errorf("Case %s failed. Expected nil, got error: %v", c.TestDescription, err) - } else if !equality.Semantic.DeepEqual(c.Pod.Spec.Containers, c.ExpectedPod.Spec.Containers) { + t.Errorf("Case %s failed. Expected nil, got error: %v", c.testDescription, err) + } else if !equality.Semantic.DeepEqual(c.pod.Spec.Containers, c.expectedPod.Spec.Containers) { t.Errorf("Case %s failed. Expected pod: %v, got: %v", - c.TestDescription, c.ExpectedPod.Spec.Containers, c.Pod.Spec.Containers) + c.testDescription, c.expectedPod.Spec.Containers, c.pod.Spec.Containers) } } } @@ -319,27 +319,27 @@ func TestGetMetricsCollectorArgs(t *testing.T) { } testCases := []struct { - Trial *trialsv1beta1.Trial - MetricNames string - MCSpec common.MetricsCollectorSpec - EarlyStoppingRules []string - KatibConfig configv1beta1.MetricsCollectorConfig - ExpectedArgs []string - Name string - Err bool + trial *trialsv1beta1.Trial + metricNames string + mCSpec common.MetricsCollectorSpec + earlyStoppingRules []string + katibConfig configv1beta1.MetricsCollectorConfig + expectedArgs []string + name string + err bool }{ { - Trial: testTrial, - MetricNames: testMetricName, - MCSpec: common.MetricsCollectorSpec{ + trial: testTrial, + metricNames: testMetricName, + mCSpec: common.MetricsCollectorSpec{ Collector: &common.CollectorSpec{ Kind: common.StdOutCollector, }, }, - KatibConfig: configv1beta1.MetricsCollectorConfig{ + katibConfig: configv1beta1.MetricsCollectorConfig{ WaitAllProcesses: &waitAllProcessesValue, }, - ExpectedArgs: []string{ + expectedArgs: []string{ "-t", testTrialName, "-m", testMetricName, "-o-type", string(testObjective), @@ -348,12 +348,12 @@ func TestGetMetricsCollectorArgs(t *testing.T) { "-format", string(common.TextFormat), "-w", "false", }, - Name: "StdOut MC", + name: "StdOut MC", }, { - Trial: testTrial, - MetricNames: testMetricName, - MCSpec: common.MetricsCollectorSpec{ + trial: testTrial, + metricNames: testMetricName, + mCSpec: common.MetricsCollectorSpec{ Collector: &common.CollectorSpec{ Kind: common.FileCollector, }, @@ -370,8 +370,8 @@ func TestGetMetricsCollectorArgs(t *testing.T) { }, }, }, - KatibConfig: configv1beta1.MetricsCollectorConfig{}, - ExpectedArgs: []string{ + katibConfig: configv1beta1.MetricsCollectorConfig{}, + expectedArgs: []string{ "-t", testTrialName, "-m", testMetricName, "-o-type", string(testObjective), @@ -380,12 +380,12 @@ func TestGetMetricsCollectorArgs(t *testing.T) { "-f", "{mn1: ([a-b]), mv1: [0-9]};{mn2: ([a-b]), mv2: ([0-9])}", "-format", string(common.TextFormat), }, - Name: "File MC with Filter", + name: "File MC with Filter", }, { - Trial: testTrial, - MetricNames: testMetricName, - MCSpec: common.MetricsCollectorSpec{ + trial: testTrial, + metricNames: testMetricName, + mCSpec: common.MetricsCollectorSpec{ Collector: &common.CollectorSpec{ Kind: common.FileCollector, }, @@ -396,8 +396,8 @@ func TestGetMetricsCollectorArgs(t *testing.T) { }, }, }, - KatibConfig: configv1beta1.MetricsCollectorConfig{}, - ExpectedArgs: []string{ + katibConfig: configv1beta1.MetricsCollectorConfig{}, + expectedArgs: []string{ "-t", testTrialName, "-m", testMetricName, "-o-type", string(testObjective), @@ -405,12 +405,12 @@ func TestGetMetricsCollectorArgs(t *testing.T) { "-path", testPath, "-format", string(common.JsonFormat), }, - Name: "File MC with Json Format", + name: "File MC with Json Format", }, { - Trial: testTrial, - MetricNames: testMetricName, - MCSpec: common.MetricsCollectorSpec{ + trial: testTrial, + metricNames: testMetricName, + mCSpec: common.MetricsCollectorSpec{ Collector: &common.CollectorSpec{ Kind: common.TfEventCollector, }, @@ -420,37 +420,37 @@ func TestGetMetricsCollectorArgs(t *testing.T) { }, }, }, - KatibConfig: configv1beta1.MetricsCollectorConfig{}, - ExpectedArgs: []string{ + katibConfig: configv1beta1.MetricsCollectorConfig{}, + expectedArgs: []string{ "-t", testTrialName, "-m", testMetricName, "-o-type", string(testObjective), "-s-db", katibDBAddress, "-path", testPath, }, - Name: "Tf Event MC", + name: "Tf Event MC", }, { - Trial: testTrial, - MetricNames: testMetricName, - MCSpec: common.MetricsCollectorSpec{ + trial: testTrial, + metricNames: testMetricName, + mCSpec: common.MetricsCollectorSpec{ Collector: &common.CollectorSpec{ Kind: common.CustomCollector, }, }, - KatibConfig: configv1beta1.MetricsCollectorConfig{}, - ExpectedArgs: []string{ + katibConfig: configv1beta1.MetricsCollectorConfig{}, + expectedArgs: []string{ "-t", testTrialName, "-m", testMetricName, "-o-type", string(testObjective), "-s-db", katibDBAddress, }, - Name: "Custom MC without Path", + name: "Custom MC without Path", }, { - Trial: testTrial, - MetricNames: testMetricName, - MCSpec: common.MetricsCollectorSpec{ + trial: testTrial, + metricNames: testMetricName, + mCSpec: common.MetricsCollectorSpec{ Collector: &common.CollectorSpec{ Kind: common.CustomCollector, }, @@ -460,44 +460,44 @@ func TestGetMetricsCollectorArgs(t *testing.T) { }, }, }, - KatibConfig: configv1beta1.MetricsCollectorConfig{}, - ExpectedArgs: []string{ + katibConfig: configv1beta1.MetricsCollectorConfig{}, + expectedArgs: []string{ "-t", testTrialName, "-m", testMetricName, "-o-type", string(testObjective), "-s-db", katibDBAddress, "-path", testPath, }, - Name: "Custom MC with Path", + name: "Custom MC with Path", }, { - Trial: testTrial, - MetricNames: testMetricName, - MCSpec: common.MetricsCollectorSpec{ + trial: testTrial, + metricNames: testMetricName, + mCSpec: common.MetricsCollectorSpec{ Collector: &common.CollectorSpec{ Kind: common.PrometheusMetricCollector, }, }, - KatibConfig: configv1beta1.MetricsCollectorConfig{}, - ExpectedArgs: []string{ + katibConfig: configv1beta1.MetricsCollectorConfig{}, + expectedArgs: []string{ "-t", testTrialName, "-m", testMetricName, "-o-type", string(testObjective), "-s-db", katibDBAddress, }, - Name: "Prometheus MC without Path", + name: "Prometheus MC without Path", }, { - Trial: testTrial, - MetricNames: testMetricName, - MCSpec: common.MetricsCollectorSpec{ + trial: testTrial, + metricNames: testMetricName, + mCSpec: common.MetricsCollectorSpec{ Collector: &common.CollectorSpec{ Kind: common.StdOutCollector, }, }, - EarlyStoppingRules: earlyStoppingRules, - KatibConfig: configv1beta1.MetricsCollectorConfig{}, - ExpectedArgs: []string{ + earlyStoppingRules: earlyStoppingRules, + katibConfig: configv1beta1.MetricsCollectorConfig{}, + expectedArgs: []string{ "-t", testTrialName, "-m", testMetricName, "-o-type", string(testObjective), @@ -508,23 +508,23 @@ func TestGetMetricsCollectorArgs(t *testing.T) { "-stop-rule", earlyStoppingRules[1], "-s-earlystop", katibEarlyStopAddress, }, - Name: "Trial with EarlyStopping rules", + name: "Trial with EarlyStopping rules", }, { - Trial: func() *trialsv1beta1.Trial { + trial: func() *trialsv1beta1.Trial { trial := testTrial.DeepCopy() trial.ObjectMeta.Labels[consts.LabelExperimentName] = "invalid-name" return trial }(), - MCSpec: common.MetricsCollectorSpec{ + mCSpec: common.MetricsCollectorSpec{ Collector: &common.CollectorSpec{ Kind: common.StdOutCollector, }, }, - EarlyStoppingRules: earlyStoppingRules, - KatibConfig: configv1beta1.MetricsCollectorConfig{}, - Name: "Trial with invalid Experiment label name. Suggestion is not created", - Err: true, + earlyStoppingRules: earlyStoppingRules, + katibConfig: configv1beta1.MetricsCollectorConfig{}, + name: "Trial with invalid Experiment label name. Suggestion is not created", + err: true, }, } @@ -536,25 +536,25 @@ func TestGetMetricsCollectorArgs(t *testing.T) { }, timeout).ShouldNot(gomega.HaveOccurred()) for _, tc := range testCases { - args, err := si.getMetricsCollectorArgs(tc.Trial, tc.MetricNames, tc.MCSpec, tc.KatibConfig, tc.EarlyStoppingRules) + args, err := si.getMetricsCollectorArgs(tc.trial, tc.metricNames, tc.mCSpec, tc.katibConfig, tc.earlyStoppingRules) - if !tc.Err && err != nil { - t.Errorf("Case: %v failed. Expected nil, got %v", tc.Name, err) - } else if tc.Err && err == nil { - t.Errorf("Case: %v failed. Expected err, got nil", tc.Name) - } else if !tc.Err && !reflect.DeepEqual(tc.ExpectedArgs, args) { - t.Errorf("Case %v failed. ExpectedArgs: %v, got %v", tc.Name, tc.ExpectedArgs, args) + if !tc.err && err != nil { + t.Errorf("Case: %v failed. Expected nil, got %v", tc.name, err) + } else if tc.err && err == nil { + t.Errorf("Case: %v failed. Expected err, got nil", tc.name) + } else if !tc.err && !reflect.DeepEqual(tc.expectedArgs, args) { + t.Errorf("Case %v failed. ExpectedArgs: %v, got %v", tc.name, tc.expectedArgs, args) } } } func TestNeedWrapWorkerContainer(t *testing.T) { testCases := []struct { - MCSpec common.MetricsCollectorSpec + mCSpec common.MetricsCollectorSpec needWrap bool }{ { - MCSpec: common.MetricsCollectorSpec{ + mCSpec: common.MetricsCollectorSpec{ Collector: &common.CollectorSpec{ Kind: common.StdOutCollector, }, @@ -562,7 +562,7 @@ func TestNeedWrapWorkerContainer(t *testing.T) { needWrap: true, }, { - MCSpec: common.MetricsCollectorSpec{ + mCSpec: common.MetricsCollectorSpec{ Collector: &common.CollectorSpec{ Kind: common.CustomCollector, }, @@ -572,7 +572,7 @@ func TestNeedWrapWorkerContainer(t *testing.T) { } for _, tc := range testCases { - needWrap := needWrapWorkerContainer(tc.MCSpec) + needWrap := needWrapWorkerContainer(tc.mCSpec) if needWrap != tc.needWrap { t.Errorf("Expected needWrap %v, got %v", tc.needWrap, needWrap) } @@ -581,16 +581,16 @@ func TestNeedWrapWorkerContainer(t *testing.T) { func TestMutateMetricsCollectorVolume(t *testing.T) { tc := struct { - Pod v1.Pod - ExpectedPod v1.Pod + pod v1.Pod + expectedPod v1.Pod JobKind string MountPath string SidecarContainerName string PrimaryContainerName string - PathKind common.FileSystemKind - Err bool + pathKind common.FileSystemKind + err bool }{ - Pod: v1.Pod{ + pod: v1.Pod{ Spec: v1.PodSpec{ Containers: []v1.Container{ { @@ -605,7 +605,7 @@ func TestMutateMetricsCollectorVolume(t *testing.T) { }, }, }, - ExpectedPod: v1.Pod{ + expectedPod: v1.Pod{ Spec: v1.PodSpec{ Containers: []v1.Container{ { @@ -643,41 +643,41 @@ func TestMutateMetricsCollectorVolume(t *testing.T) { MountPath: common.DefaultFilePath, SidecarContainerName: "metrics-collector", PrimaryContainerName: "train-job", - PathKind: common.FileKind, + pathKind: common.FileKind, } err := mutateMetricsCollectorVolume( - &tc.Pod, + &tc.pod, tc.MountPath, tc.SidecarContainerName, tc.PrimaryContainerName, - tc.PathKind) + tc.pathKind) if err != nil { t.Errorf("mutateMetricsCollectorVolume failed: %v", err) - } else if !equality.Semantic.DeepEqual(tc.Pod, tc.ExpectedPod) { - t.Errorf("Expected pod %v, got %v", tc.ExpectedPod, tc.Pod) + } else if !equality.Semantic.DeepEqual(tc.pod, tc.expectedPod) { + t.Errorf("Expected pod %v, got %v", tc.expectedPod, tc.pod) } } func TestGetSidecarContainerName(t *testing.T) { testCases := []struct { - CollectorKind common.CollectorKind - ExpectedCollectorKind string + collectorKind common.CollectorKind + expectedCollectorKind string }{ { - CollectorKind: common.StdOutCollector, - ExpectedCollectorKind: mccommon.MetricLoggerCollectorContainerName, + collectorKind: common.StdOutCollector, + expectedCollectorKind: mccommon.MetricLoggerCollectorContainerName, }, { - CollectorKind: common.TfEventCollector, - ExpectedCollectorKind: mccommon.MetricCollectorContainerName, + collectorKind: common.TfEventCollector, + expectedCollectorKind: mccommon.MetricCollectorContainerName, }, } for _, tc := range testCases { - collectorKind := getSidecarContainerName(tc.CollectorKind) - if collectorKind != tc.ExpectedCollectorKind { - t.Errorf("Expected Collector Kind: %v, got %v", tc.ExpectedCollectorKind, collectorKind) + collectorKind := getSidecarContainerName(tc.collectorKind) + if collectorKind != tc.expectedCollectorKind { + t.Errorf("Expected Collector Kind: %v, got %v", tc.expectedCollectorKind, collectorKind) } } } @@ -721,16 +721,16 @@ func TestGetKatibJob(t *testing.T) { jobName := "job-name" testCases := []struct { - Pod *v1.Pod - Job *batchv1.Job - Deployment *appsv1.Deployment - ExpectedJobKind string - ExpectedJobName string - Err bool - TestDescription string + pod *v1.Pod + job *batchv1.Job + deployment *appsv1.Deployment + expectedJobKind string + expectedJobName string + err bool + testDescription string }{ { - Pod: &v1.Pod{ + pod: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: podName, Namespace: namespace, @@ -743,7 +743,7 @@ func TestGetKatibJob(t *testing.T) { }, }, }, - Job: &batchv1.Job{ + job: &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ Name: jobName + "-1", Namespace: namespace, @@ -770,13 +770,13 @@ func TestGetKatibJob(t *testing.T) { }, }, }, - ExpectedJobKind: "Job", - ExpectedJobName: jobName + "-1", - Err: false, - TestDescription: "Valid run with ownership sequence: Trial -> Job -> Pod", + expectedJobKind: "Job", + expectedJobName: jobName + "-1", + err: false, + testDescription: "Valid run with ownership sequence: Trial -> Job -> Pod", }, { - Pod: &v1.Pod{ + pod: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: podName, Namespace: namespace, @@ -794,7 +794,7 @@ func TestGetKatibJob(t *testing.T) { }, }, }, - Job: &batchv1.Job{ + job: &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ Name: jobName + "-2", Namespace: namespace, @@ -813,7 +813,7 @@ func TestGetKatibJob(t *testing.T) { }, }, }, - Deployment: &appsv1.Deployment{ + deployment: &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: deployName + "-2", Namespace: namespace, @@ -849,13 +849,13 @@ func TestGetKatibJob(t *testing.T) { }, }, }, - ExpectedJobKind: "Deployment", - ExpectedJobName: deployName + "-2", - Err: false, - TestDescription: "Valid run with ownership sequence: Trial -> Deployment -> Pod, Job -> Pod", + expectedJobKind: "Deployment", + expectedJobName: deployName + "-2", + err: false, + testDescription: "Valid run with ownership sequence: Trial -> Deployment -> Pod, Job -> Pod", }, { - Pod: &v1.Pod{ + pod: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: podName, Namespace: namespace, @@ -868,7 +868,7 @@ func TestGetKatibJob(t *testing.T) { }, }, }, - Job: &batchv1.Job{ + job: &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ Name: jobName + "-3", Namespace: namespace, @@ -887,11 +887,11 @@ func TestGetKatibJob(t *testing.T) { }, }, }, - Err: true, - TestDescription: "Run for not Trial's pod with ownership sequence: Job -> Pod", + err: true, + testDescription: "Run for not Trial's pod with ownership sequence: Job -> Pod", }, { - Pod: &v1.Pod{ + pod: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: podName, Namespace: namespace, @@ -904,11 +904,11 @@ func TestGetKatibJob(t *testing.T) { }, }, }, - Err: true, - TestDescription: "Run when Pod owns Job that doesn't exists", + err: true, + testDescription: "Run when Pod owns Job that doesn't exists", }, { - Pod: &v1.Pod{ + pod: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: podName, Namespace: namespace, @@ -921,15 +921,15 @@ func TestGetKatibJob(t *testing.T) { }, }, }, - Err: true, - TestDescription: "Run when Pod owns Job with invalid API version", + err: true, + testDescription: "Run when Pod owns Job with invalid API version", }, } for _, tc := range testCases { // Create Job if it is needed - if tc.Job != nil { - jobUnstr, err := util.ConvertObjectToUnstructured(tc.Job) + if tc.job != nil { + jobUnstr, err := util.ConvertObjectToUnstructured(tc.job) gvk := schema.GroupVersionKind{ Group: "batch", Version: "v1", @@ -944,28 +944,28 @@ func TestGetKatibJob(t *testing.T) { // Wait that Job is created g.Eventually(func() error { - return c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: tc.Job.Name}, jobUnstr) + return c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: tc.job.Name}, jobUnstr) }, timeout).ShouldNot(gomega.HaveOccurred()) } // Create Deployment if it is needed - if tc.Deployment != nil { - g.Expect(c.Create(context.TODO(), tc.Deployment)).NotTo(gomega.HaveOccurred()) + if tc.deployment != nil { + g.Expect(c.Create(context.TODO(), tc.deployment)).NotTo(gomega.HaveOccurred()) // Wait that Deployment is created g.Eventually(func() error { - return c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: tc.Deployment.Name}, tc.Deployment) + return c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: tc.deployment.Name}, tc.deployment) }, timeout).ShouldNot(gomega.HaveOccurred()) } - object, _ := util.ConvertObjectToUnstructured(tc.Pod) + object, _ := util.ConvertObjectToUnstructured(tc.pod) jobKind, jobName, err := si.getKatibJob(object, namespace) - if !tc.Err && err != nil { - t.Errorf("Case %v failed. Error %v", tc.TestDescription, err) - } else if !tc.Err && (tc.ExpectedJobKind != jobKind || tc.ExpectedJobName != jobName) { + if !tc.err && err != nil { + t.Errorf("Case %v failed. Error %v", tc.testDescription, err) + } else if !tc.err && (tc.expectedJobKind != jobKind || tc.expectedJobName != jobName) { t.Errorf("Case %v failed. Expected jobKind %v, got %v, Expected jobName %v, got %v", - tc.TestDescription, tc.ExpectedJobKind, jobKind, tc.ExpectedJobName, jobName) - } else if tc.Err && err == nil { + tc.testDescription, tc.expectedJobKind, jobKind, tc.expectedJobName, jobName) + } else if tc.err && err == nil { t.Errorf("Expected error got nil") } }