Skip to content

Drop From field validation check #1464

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

Merged
openshift-bot merged 1 commit into from
Mar 26, 2015
Merged

Drop From field validation check #1464

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

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions pkg/build/api/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,6 @@ func validateImageChange(imageChange *buildapi.ImageChangeTrigger) errs.Validati
allErrs = append(allErrs, errs.NewFieldRequired("image"))
}
if len(imageChange.From.Name) == 0 {
allErrs = append(allErrs, errs.NewFieldRequired("from"))
} else if len(imageChange.From.Name) == 0 {
allErrs = append(allErrs, errs.ValidationErrorList{errs.NewFieldRequired("name")}.Prefix("from")...)
}
return allErrs
Expand Down
63 changes: 63 additions & 0 deletions pkg/build/api/validation/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,3 +505,66 @@ func TestValidateTrigger(t *testing.T) {
}
}
}

func TestValidateImageChange(t *testing.T) {
tests := []struct {
name string
ict *buildapi.ImageChangeTrigger
expectedErrNum int
}{
{
name: "Pass",
ict: &buildapi.ImageChangeTrigger{
Image: "openshift",
From: kapi.ObjectReference{
Name: "default/repo",
},
},
expectedErrNum: 0,
},
{
name: "Missing image ref",
ict: &buildapi.ImageChangeTrigger{
Image: "",
From: kapi.ObjectReference{
Name: "default/repo",
},
},
expectedErrNum: 1,
},
{
name: "Missing From ref",
ict: &buildapi.ImageChangeTrigger{
Image: "openshift",
From: kapi.ObjectReference{
Name: "",
},
},
expectedErrNum: 1,
},
{
name: "Both missing refs",
ict: &buildapi.ImageChangeTrigger{
Image: "",
From: kapi.ObjectReference{
Copy link
Contributor

@bparees bparees Mar 26, 2015

Choose a reason for hiding this comment

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

how about a test where you don't even define the From field? Just in case someone changes it to a ptr in the future and sets us up for a nil ptr error.

Copy link
Contributor Author

@0xmichalis 0xmichalis Mar 26, 2015

Choose a reason for hiding this comment

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

Done

Copy link
Contributor Author

@0xmichalis 0xmichalis Mar 26, 2015

Choose a reason for hiding this comment

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

But even if they change it to a pointer, they would have to turn into a pointer every from field in the previous tests too

Copy link
Contributor

@bparees bparees Mar 26, 2015

Choose a reason for hiding this comment

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

yup. i'm just paranoid and it seemed like an easy add. thanks for doing it! :)

Name: "",
},
},
expectedErrNum: 2,
},
{
name: "Undefined from field",
ict: &buildapi.ImageChangeTrigger{
Image: "openshift",
},
expectedErrNum: 1,
},
}

for _, test := range tests {
got := len(validateImageChange(test.ict))
if test.expectedErrNum != got {
t.Errorf("%s: Expected %d error(s), got %d", test.name, test.expectedErrNum, got)
}
}
}