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

Add more test cases for Generator #1216

Merged
merged 6 commits into from
Jun 16, 2020
Merged

Add more test cases for Generator #1216

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bfce2c1
Init commit
andreyvelich Jun 13, 2020
49e332a
Add test to generator for unstructured
andreyvelich Jun 13, 2020
fd2a6ca
Merge remote-tracking branch 'upstream/master' into generator-add-tests
andreyvelich Jun 15, 2020
cd2b92b
Add test to generator for configmap
andreyvelich Jun 15, 2020
4ebb5af
Fix log
andreyvelich Jun 15, 2020
95a3ba3
Add comment
andreyvelich Jun 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
261 changes: 214 additions & 47 deletions pkg/controller.v1beta1/experiment/manifest/generator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package manifest

import (
"errors"
"math"
"reflect"
"testing"

Expand All @@ -12,10 +14,10 @@ import (
batchv1 "k8s.io/api/batch/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

func TestGetRunSpecWithHP(t *testing.T) {
tc := newFakeInstance()

mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
Expand All @@ -26,22 +28,6 @@ func TestGetRunSpecWithHP(t *testing.T) {
client: c,
}

// TODO: Add more test cases
actual, err := p.GetRunSpecWithHyperParameters(tc, "trial-name", "trial-namespace", []commonapiv1beta1.ParameterAssignment{
{
Name: "lr",
Value: "0.05",
},
{
Name: "num-layers",
Value: "5",
},
})

if err != nil {
t.Errorf("Expected nil, got %v", err)
}

expectedJob := batchv1.Job{
TypeMeta: metav1.TypeMeta{
APIVersion: "batch/v1",
Expand Down Expand Up @@ -71,13 +57,79 @@ func TestGetRunSpecWithHP(t *testing.T) {
},
}

expected, err := util.ConvertObjectToUnstructured(expectedJob)
expectedRunSpec, err := util.ConvertObjectToUnstructured(expectedJob)
if err != nil {
t.Errorf("ConvertObjectToUnstructured failed: %v", err)
}

if !reflect.DeepEqual(expected.Object, actual.Object) {
t.Errorf("Expected %v\n got %v", expected.Object, actual.Object)
tcs := []struct {
Instance *experimentsv1beta1.Experiment
ParameterAssignments []commonapiv1beta1.ParameterAssignment
expectedRunSpec *unstructured.Unstructured
Err bool
testDescription string
}{
// Valid run
{
Instance: newFakeInstance(),
ParameterAssignments: newFakeParameterAssignment(),
expectedRunSpec: expectedRunSpec,
Err: false,
testDescription: "Run with valid parameters",
},
// Invalid JSON in unstructured
{
Instance: func() *experimentsv1beta1.Experiment {
i := newFakeInstance()
trialSpec := i.Spec.TrialTemplate.TrialSource.TrialSpec
trialSpec.Object = map[string]interface{}{
"invalidJSON": math.NaN(),
}
return i
}(),
ParameterAssignments: newFakeParameterAssignment(),
Err: true,
testDescription: "Invalid JSON in Trial template",
},
// len(parameterAssignment) != len(trialParameters)
{
Instance: newFakeInstance(),
ParameterAssignments: func() []commonapiv1beta1.ParameterAssignment {
pa := newFakeParameterAssignment()
pa = pa[1:]
return pa
}(),
Err: true,
testDescription: "Number of parameter assignments is not equal to number of Trial parameters",
},
// Parameter from assignments not found in Trial paramters
{
Instance: newFakeInstance(),
ParameterAssignments: func() []commonapiv1beta1.ParameterAssignment {
pa := newFakeParameterAssignment()
pa[0] = commonapiv1beta1.ParameterAssignment{
Name: "invalid-name",
Value: "invalid-value",
}
return pa
}(),
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)

if tc.Err && err == nil {
t.Errorf("Case: %v failed. Expected err, got nil", tc.testDescription)
} 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) {
t.Errorf("Case: %v failed. Expected %v\n got %v", tc.testDescription, tc.expectedRunSpec.Object, actualRunSpec.Object)
}
}
}
}

Expand All @@ -91,7 +143,7 @@ func TestGetRunSpecWithHPConfigMap(t *testing.T) {
client: c,
}

templatePath := "trial-template.yaml"
templatePath := "trial-template-path"

trialSpec := `apiVersion: batch/v1
kind: Job
Expand All @@ -107,30 +159,42 @@ spec:
- "--lr=${trialParameters.learningRate}"
- "--num-layers=${trialParameters.numberLayers}"`

c.EXPECT().GetConfigMap(gomock.Any(), gomock.Any()).Return(map[string]string{
templatePath: trialSpec,
}, nil)
invalidTrialSpec := `apiVersion: batch/v1
kind: Job
spec:
template:
spec:
containers:
- name: training-container
image: docker.io/kubeflowkatib/mxnet-mnist
command:
- python3
- /opt/mxnet-mnist/mnist.py
- --lr=${trialParameters.learningRate}
- --num-layers=${trialParameters.numberLayers}
- --invalidParameter={'num_layers': 2, 'input_sizes': [32, 32, 3]}`

instance := newFakeInstance()
instance.Spec.TrialTemplate.TrialSource.ConfigMap = &experimentsv1beta1.ConfigMapSource{
TemplatePath: templatePath,
}
instance.Spec.TrialTemplate.TrialSource.TrialSpec = nil
actual, err := p.GetRunSpecWithHyperParameters(instance, "trial-name", "trial-namespace", []commonapiv1beta1.ParameterAssignment{
{
Name: "lr",
Value: "0.05",
},
{
Name: "num-layers",
Value: "5",
},
})
if err != nil {
t.Errorf("Expected nil, got %v", err)
}
// We can't compare structures, because trialSpec is a string and creationTimestamp was not added
expectedJob := `apiVersion: batch/v1
validGetConfigMap1 := c.EXPECT().GetConfigMap(gomock.Any(), gomock.Any()).Return(
map[string]string{templatePath: trialSpec}, nil)

invalidConfigMapName := c.EXPECT().GetConfigMap(gomock.Any(), gomock.Any()).Return(
nil, errors.New("Unable to get ConfigMap"))

validGetConfigMap3 := c.EXPECT().GetConfigMap(gomock.Any(), gomock.Any()).Return(
map[string]string{templatePath: trialSpec}, nil)

invalidTemplate := c.EXPECT().GetConfigMap(gomock.Any(), gomock.Any()).Return(
map[string]string{templatePath: invalidTrialSpec}, nil)

gomock.InOrder(
validGetConfigMap1,
invalidConfigMapName,
validGetConfigMap3,
invalidTemplate,
)

// We can't compare structures, because in ConfigMap trialSpec is a string and creationTimestamp was not added
expectedStr := `apiVersion: batch/v1
kind: Job
metadata:
name: trial-name
Expand All @@ -147,12 +211,102 @@ spec:
- "--lr=0.05"
- "--num-layers=5"`

expected, err := util.ConvertStringToUnstructured(expectedJob)
expectedRunSpec, err := util.ConvertStringToUnstructured(expectedStr)
if err != nil {
t.Errorf("ConvertStringToUnstructured failed: %v", err)
}
if !reflect.DeepEqual(expected, actual) {
t.Errorf("Expected %s\n got %s", expected.Object, actual.Object)

tcs := []struct {
Instance *experimentsv1beta1.Experiment
ParameterAssignments []commonapiv1beta1.ParameterAssignment
Err bool
testDescription string
}{
// Valid run
// validGetConfigMap1 case
{
Instance: func() *experimentsv1beta1.Experiment {
i := newFakeInstance()
i.Spec.TrialTemplate.TrialSource = experimentsv1beta1.TrialSource{
ConfigMap: &experimentsv1beta1.ConfigMapSource{
ConfigMapName: "config-map-name",
ConfigMapNamespace: "config-map-namespace",
TemplatePath: "trial-template-path",
},
}
return i
}(),
ParameterAssignments: newFakeParameterAssignment(),
Err: false,
testDescription: "Run with valid parameters",
},
// Invalid ConfigMap name
// invalidConfigMapName case
{
Instance: func() *experimentsv1beta1.Experiment {
i := newFakeInstance()
i.Spec.TrialTemplate.TrialSource = experimentsv1beta1.TrialSource{
ConfigMap: &experimentsv1beta1.ConfigMapSource{
ConfigMapName: "invalid-name",
},
}
return i
}(),
ParameterAssignments: newFakeParameterAssignment(),
Err: true,
testDescription: "Invalid ConfigMap name",
},
// Invalid template path in ConfigMap name
// validGetConfigMap3 case
{
Instance: func() *experimentsv1beta1.Experiment {
i := newFakeInstance()
i.Spec.TrialTemplate.TrialSource = experimentsv1beta1.TrialSource{
ConfigMap: &experimentsv1beta1.ConfigMapSource{
ConfigMapName: "config-map-name",
ConfigMapNamespace: "config-map-namespace",
TemplatePath: "invalid-path",
},
}
return i
}(),
ParameterAssignments: newFakeParameterAssignment(),
Err: true,
testDescription: "Invalid template path in ConfigMap",
},
// Invalid Trial template spec in ConfigMap
// Trial template is a string in ConfigMap
// Because of that, user can specify not valid unstructured template
// invalidTemplate case
{
Instance: func() *experimentsv1beta1.Experiment {
i := newFakeInstance()
i.Spec.TrialTemplate.TrialSource = experimentsv1beta1.TrialSource{
ConfigMap: &experimentsv1beta1.ConfigMapSource{
ConfigMapName: "config-map-name",
ConfigMapNamespace: "config-map-namespace",
TemplatePath: "trial-template-path",
},
}
return i
}(),
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 {
t.Errorf("Case: %v failed. Expected err, got nil", tc.testDescription)
} 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) {
t.Errorf("Case: %v failed. Expected %v\n got %v", tc.testDescription, expectedRunSpec.Object, actualRunSpec.Object)
}
}
}
}

Expand Down Expand Up @@ -206,3 +360,16 @@ func newFakeInstance() *experimentsv1beta1.Experiment {
},
}
}

func newFakeParameterAssignment() []commonapiv1beta1.ParameterAssignment {
return []commonapiv1beta1.ParameterAssignment{
{
Name: "lr",
Value: "0.05",
},
{
Name: "num-layers",
Value: "5",
},
}
}