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

Jump to bottom

scheduler: fix validation test #48337

Merged
merged 1 commit into from Jul 8, 2017
Merged

scheduler: fix validation test #48337

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
106 changes: 48 additions & 58 deletions plugin/pkg/scheduler/api/validation/validation_test.go
View file
Open in desktop
Expand Up @@ -17,70 +17,60 @@ limitations under the License.
package validation

import (
"errors"
"fmt"
"testing"

"k8s.io/kubernetes/plugin/pkg/scheduler/api"
)

func TestValidatePriorityWithNoWeight(t *testing.T) {
policy := api.Policy{Priorities: []api.PriorityPolicy{{Name: "NoWeightPriority"}}}
if ValidatePolicy(policy) == nil {
t.Errorf("Expected error about priority weight not being positive")
}
}

func TestValidatePriorityWithZeroWeight(t *testing.T) {
policy := api.Policy{Priorities: []api.PriorityPolicy{{Name: "NoWeightPriority", Weight: 0}}}
if ValidatePolicy(policy) == nil {
t.Errorf("Expected error about priority weight not being positive")
}
}

func TestValidatePriorityWithNonZeroWeight(t *testing.T) {
policy := api.Policy{Priorities: []api.PriorityPolicy{{Name: "WeightPriority", Weight: 2}}}
errs := ValidatePolicy(policy)
if errs != nil {
t.Errorf("Unexpected errors %v", errs)
}
}

func TestValidatePriorityWithNegativeWeight(t *testing.T) {
policy := api.Policy{Priorities: []api.PriorityPolicy{{Name: "WeightPriority", Weight: -2}}}
if ValidatePolicy(policy) == nil {
t.Errorf("Expected error about priority weight not being positive")
}
}

func TestValidatePriorityWithOverFlowWeight(t *testing.T) {
policy := api.Policy{Priorities: []api.PriorityPolicy{{Name: "WeightPriority", Weight: api.MaxWeight}}}
if ValidatePolicy(policy) == nil {
t.Errorf("Expected error about priority weight not being overflown.")
}
}

func TestValidateExtenderWithNonNegativeWeight(t *testing.T) {
extenderPolicy := api.Policy{ExtenderConfigs: []api.ExtenderConfig{{URLPrefix: "http://127.0.0.1:8081/extender", FilterVerb: "filter", Weight: 2}}}
errs := ValidatePolicy(extenderPolicy)
if errs != nil {
t.Errorf("Unexpected errors %v", errs)
}
}

func TestValidateExtenderWithNegativeWeight(t *testing.T) {
extenderPolicy := api.Policy{ExtenderConfigs: []api.ExtenderConfig{{URLPrefix: "http://127.0.0.1:8081/extender", FilterVerb: "filter", Weight: -2}}}
if ValidatePolicy(extenderPolicy) == nil {
t.Errorf("Expected error about priority weight for extender not being positive")
}
}

func TestValidateMultipleExtendersWithBind(t *testing.T) {
extenderPolicy := api.Policy{
ExtenderConfigs: []api.ExtenderConfig{
{URLPrefix: "http://127.0.0.1:8081/extender", BindVerb: "bind"},
{URLPrefix: "http://127.0.0.1:8082/extender", BindVerb: "bind"},
func TestValidatePolicy(t *testing.T) {
tests := []struct {
policy api.Policy
expected error
}{
{
policy: api.Policy{Priorities: []api.PriorityPolicy{{Name: "NoWeightPriority"}}},
expected: errors.New("Priority NoWeightPriority should have a positive weight applied to it or it has overflown"),
Copy link
Member

Choose a reason for hiding this comment

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

Can we make those error message a const var?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those error message are actually generated by ValidatePolicy. It makes no sense to make them a const var, since those won't be reused elsewhere.

Copy link
Member

Choose a reason for hiding this comment

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

share same const var between validat and test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like I said, those error message are actually generated by ValidatePolicy. For example:

validationErrors = append(validationErrors, fmt.Errorf("Priority %s should have a positive weight applied to it or it has overflown", priority.Name))

So you want me to change it to something like:

const OverflownError="Priority %s should have a positive weight applied to it or it has overflown"
validationErrors = append(validationErrors, fmt.Errorf(OverflownError, priority.Name))

Copy link
Member

Choose a reason for hiding this comment

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

hm.. , seems both not good enough :). let keep current PR.

},
{
policy: api.Policy{Priorities: []api.PriorityPolicy{{Name: "NoWeightPriority", Weight: 0}}},
expected: errors.New("Priority NoWeightPriority should have a positive weight applied to it or it has overflown"),
},
{
policy: api.Policy{Priorities: []api.PriorityPolicy{{Name: "WeightPriority", Weight: 2}}},
expected: nil,
},
{
policy: api.Policy{Priorities: []api.PriorityPolicy{{Name: "WeightPriority", Weight: -2}}},
expected: errors.New("Priority WeightPriority should have a positive weight applied to it or it has overflown"),
},
{
policy: api.Policy{Priorities: []api.PriorityPolicy{{Name: "WeightPriority", Weight: api.MaxWeight}}},
expected: errors.New("Priority WeightPriority should have a positive weight applied to it or it has overflown"),
},
{
policy: api.Policy{ExtenderConfigs: []api.ExtenderConfig{{URLPrefix: "http://127.0.0.1:8081/extender", FilterVerb: "filter", Weight: 2}}},
expected: nil,
},
{
policy: api.Policy{ExtenderConfigs: []api.ExtenderConfig{{URLPrefix: "http://127.0.0.1:8081/extender", FilterVerb: "filter", Weight: -2}}},
expected: errors.New("Priority for extender http://127.0.0.1:8081/extender should have a positive weight applied to it"),
},
{
policy: api.Policy{
ExtenderConfigs: []api.ExtenderConfig{
{URLPrefix: "http://127.0.0.1:8081/extender", BindVerb: "bind", Weight: 2},
{URLPrefix: "http://127.0.0.1:8082/extender", BindVerb: "bind", Weight: 2},
}},
expected: errors.New("Only one extender can implement bind, found 2"),
},
}
if ValidatePolicy(extenderPolicy) == nil {
t.Errorf("Expected failure when multiple extenders with bind")

for _, test := range tests {
actual := ValidatePolicy(test.policy)
if fmt.Sprint(test.expected) != fmt.Sprint(actual) {
t.Errorf("expected: %s, actual: %s", test.expected, actual)
}
}
}