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

Speed up CRD apply/openapi unit tests #98694

Merged
merged 3 commits into from Feb 2, 2021
Merged

Speed up CRD apply/openapi unit tests #98694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4b527af
Speed up CRD apply unit test
liggitt Feb 2, 2021
eff40e6
Lower openapi filter fuzz iterations
liggitt Feb 2, 2021
394914f
Add testdata readme
liggitt Feb 2, 2021
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
12 changes: 2 additions & 10 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/BUILD
View file
Open in desktop
@@ -1,10 +1,4 @@
package(default_visibility = ["//visibility:public"])

load(
"@io_bazel_rules_go//go:def.bzl",
"go_library",
"go_test",
)
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "go_default_library",
Expand Down Expand Up @@ -100,9 +94,7 @@ go_library(
go_test(
name = "go_default_test",
srcs = ["customresource_handler_test.go"],
data = [
"//api/openapi-spec",
],
data = glob(["testdata/**"]),
Copy link
Member

Choose a reason for hiding this comment

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

Since we're not going toward more bazel, AFAIK, it's probably not important, but do we need to depend on everything here?

Copy link
Member Author

Choose a reason for hiding this comment

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

this was autogenerated, so... /shrug ?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, if we were building a super awesome bazel thing, then it means this test would be re-run anytime something changes anything in testdata which wouldn't be great. I can't really care less :-)

embed = [":go_default_library"],
deps = [
"//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1:go_default_library",
Expand Down
13 changes: 7 additions & 6 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler_test.go
View file
Open in desktop
Expand Up @@ -28,7 +28,6 @@ import (
"os"
"path/filepath"
"strconv"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -825,18 +824,20 @@ func TestBuildOpenAPIModelsForApply(t *testing.T) {
},
}

for _, test := range tests {
for i, test := range tests {
crd.Spec.Versions[0].Schema = &test
if _, err := buildOpenAPIModelsForApply(staticSpec, &crd); err != nil {
models, err := buildOpenAPIModelsForApply(staticSpec, &crd)
if err != nil {
t.Fatalf("failed to convert to apply model: %v", err)
}
if models == nil {
Copy link
Member Author

Choose a reason for hiding this comment

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

adds a check that the model was actually built successfully

t.Fatalf("%d: failed to convert to apply model: nil", i)
}
}
}

func getOpenAPISpecFromFile() (*spec.Swagger, error) {
path := filepath.Join(
strings.Repeat(".."+string(filepath.Separator), 6),
"api", "openapi-spec", "swagger.json")
path := filepath.Join("testdata", "swagger.json")
Copy link
Member

Choose a reason for hiding this comment

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

That is simpler.

_, err := os.Stat(path)
if err != nil {
return nil, err
Expand Down
4 changes: 4 additions & 0 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/testdata/README.md
View file
Open in desktop
@@ -0,0 +1,4 @@
`swagger.json` contains static openapi definitions needed to build models for custom resource types.

It was created by removing all path and non-objectmeta/int-or-string definitions from
Copy link
Member

Choose a reason for hiding this comment

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

I'm not entirely sure I understand that?

Copy link
Member Author

Choose a reason for hiding this comment

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

started with api/openapi-spec/swagger.json and removed all the .paths bits, and all the .definitions that weren't related to int-or-string or objectmeta types

the `api/openapi-spec/swagger.json` in the `k8s.io/kubernetes` module.