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.

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 some unit tests #8960

Merged
merged 5 commits into from
May 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 8 additions & 1 deletion pkg/model/defaults/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "go_default_library",
Expand All @@ -7,3 +7,10 @@ go_library(
visibility = ["//visibility:public"],
deps = ["//pkg/apis/kops:go_default_library"],
)

go_test(
name = "go_default_test",
srcs = ["volumes_test.go"],
embed = [":go_default_library"],
deps = ["//pkg/apis/kops:go_default_library"],
)
41 changes: 41 additions & 0 deletions pkg/model/defaults/volumes_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
Copyright 2020 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package defaults

import (
"testing"

"k8s.io/kops/pkg/apis/kops"
)

func TestDefaultInstanceGroupVolumeSize(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

This test feels like it creates maintenance work rather than unit testing "kops logic". That is to say, DefaultInstanceGroupVolumeSize basically just returns different constants with a switch.

IMO, changes that would break this test are likely to be intentional changes, so my vote would be to remove volumes_test.go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fine.

tests := []struct {
role kops.InstanceGroupRole
expected int32
}{
{
role: "Node2",
expected: -1,
Copy link
Member

Choose a reason for hiding this comment

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

@geojaz this particular one could perhaps have some value. I suppose we want to preserve -1 being the return value for unknown roles. The above could perhaps just test for positive integers instead.

Copy link
Member

Choose a reason for hiding this comment

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

If we were to add another role, it would at least be nice to see this test failing.

Copy link
Member

Choose a reason for hiding this comment

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

Great point +1 @olemarkus

},
}
for _, test := range tests {
result, _ := DefaultInstanceGroupVolumeSize(test.role)
if test.expected != result {
t.Errorf("Expected %d, got %d", test.expected, result)
}
}
}
12 changes: 11 additions & 1 deletion upup/pkg/fi/utils/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "go_default_library",
Expand All @@ -16,3 +16,13 @@ go_library(
"//vendor/k8s.io/client-go/util/homedir:go_default_library",
],
)

go_test(
name = "go_default_test",
srcs = [
"equals_test.go",
"hash_test.go",
"sanitize_test.go",
],
embed = [":go_default_library"],
)
83 changes: 83 additions & 0 deletions upup/pkg/fi/utils/equals_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
Copyright 2020 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package utils

import (
"testing"
)

func TestStringSlicesEqual(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

FYI: I think this is a much more useful unit test than the volumes_test.

tests := []struct {
l []string
r []string
expected bool
}{
{
l: []string{"a", "b"},
r: []string{"a"},
expected: false,
},
{
l: []string{"a", "b"},
r: []string{"a", "c"},
expected: false,
},
{
l: []string{"a", "b"},
r: []string{"a", "b"},
expected: true,
},
}

for _, test := range tests {
result := StringSlicesEqual(test.l, test.r)
if test.expected != result {
t.Errorf("Expected %v, got %v", test.expected, result)
}
}
}

func TestStringSlicesEqualIgnoreOrder(t *testing.T) {
tests := []struct {
l []string
r []string
expected bool
}{
{
l: []string{"a", "b"},
r: []string{"a"},
expected: false,
},
{
l: []string{"a", "b"},
r: []string{"a", "c"},
expected: false,
},
{
l: []string{"a", "b"},
r: []string{"b", "a"},
expected: true,
},
}

for _, test := range tests {
result := StringSlicesEqualIgnoreOrder(test.l, test.r)
if test.expected != result {
t.Errorf("Expected %v, got %v", test.expected, result)
}
}
}
52 changes: 52 additions & 0 deletions upup/pkg/fi/utils/hash_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
Copyright 2020 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package utils

import (
"testing"
)

func TestHashString(t *testing.T) {
tests := []struct {
s string
expectedStr string
}{
{
s: "test",
Copy link
Member

Choose a reason for hiding this comment

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

I think this is also a fairly useful test... could you add some more spicy or exciting edge cases? For these relatively simple functions, I would prefer to exercise the hard cases and assume the libraries generally do what they're supposed to.

expectedStr: "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3",
},
{
s: "~!*/?",
expectedStr: "783c2ea26549ce90df7cd90a27bf9094c226c406",
},
{
s: "测试1",
expectedStr: "6d972f7f1450aba1f7496f685c3c656c4fca9624",
},
{
s: "-897668",
expectedStr: "c8facb588b36948d5da0e3a7e16977a701331b0a",
},
}

for _, test := range tests {
result, _ := HashString(test.s)
if test.expectedStr != result {
t.Errorf("Expected %s, got %s", test.expectedStr, result)
}
}
}
44 changes: 44 additions & 0 deletions upup/pkg/fi/utils/sanitize_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
Copyright 2020 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package utils

import (
"testing"
)

func TestSanitizeString(t *testing.T) {
tests := []struct {
s string
expected string
}{
{
s: "te_ST-01",
expected: "te_ST-01",
},
{
s: "(/%test!&*)~!@#$%^&<>?/+",
expected: "___test_________________",
},
}

for _, test := range tests {
result := SanitizeString(test.s)
if test.expected != result {
t.Errorf("Expected %s, got %s", test.expected, result)
}
}
}