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

Print a newline after ginkgo tests so the test infra doesn't think th… #45320

Merged
merged 1 commit into from
May 8, 2017
Merged

Print a newline after ginkgo tests so the test infra doesn't think th… #45320

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 19 additions & 19 deletions pkg/kubectl/cmd/util/openapi/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,6 @@ load(
"go_test",
)

go_test(
name = "go_default_test",
srcs = [
"openapi_cache_test.go",
"openapi_getter_test.go",
"openapi_test.go",
],
library = ":go_default_library",
tags = ["automanaged"],
deps = [
"//vendor/github.com/go-openapi/loads:go_default_library",
"//vendor/github.com/go-openapi/spec:go_default_library",
"//vendor/github.com/onsi/ginkgo:go_default_library",
"//vendor/github.com/onsi/gomega:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
],
)

go_library(
name = "go_default_library",
srcs = [
Expand All @@ -47,11 +29,24 @@ go_library(

go_test(
name = "go_default_xtest",
srcs = ["openapi_suite_test.go"],
size = "small",
srcs = [
"openapi_cache_test.go",
"openapi_getter_test.go",
"openapi_suite_test.go",
"openapi_test.go",
],
data = ["//api/openapi-spec:swagger-spec"],
tags = ["automanaged"],
deps = [
"//pkg/kubectl/cmd/util/openapi:go_default_library",
"//vendor/github.com/go-openapi/loads:go_default_library",
"//vendor/github.com/go-openapi/spec:go_default_library",
"//vendor/github.com/onsi/ginkgo:go_default_library",
"//vendor/github.com/onsi/ginkgo/config:go_default_library",
"//vendor/github.com/onsi/ginkgo/types:go_default_library",
"//vendor/github.com/onsi/gomega:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
],
)

Expand All @@ -67,3 +62,8 @@ filegroup(
srcs = [":package-srcs"],
tags = ["automanaged"],
)

filegroup(
name = "testdata",
srcs = glob(["testdata/*"]),
)
6 changes: 3 additions & 3 deletions pkg/kubectl/cmd/util/openapi/openapi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ type Type struct {
Extensions spec.Extensions
}

// newOpenAPIData parses the resource definitions in openapi data by groupversionkind and name
func newOpenAPIData(s *spec.Swagger) (*Resources, error) {
// NewOpenAPIData parses the resource definitions in openapi data by groupversionkind and name
func NewOpenAPIData(s *spec.Swagger) (*Resources, error) {
o := &Resources{
GroupVersionKindToName: map[schema.GroupVersionKind]string{},
NameToDefinition: map[string]Kind{},
Expand Down Expand Up @@ -338,7 +338,7 @@ func (o *Resources) nameForDefinitionField(s spec.Schema) string {
return strings.Replace(p, "/definitions/", "", -1)
}

// getGroupVersionKind implements openAPIData
// getGroupVersionKind implements OpenAPIData
// getGVK parses the gropuversionkind for a resource definition from the x-kubernetes
// extensions
// Expected format for s.Extensions: map[string][]map[string]string
Expand Down
26 changes: 13 additions & 13 deletions pkg/kubectl/cmd/util/openapi/openapi_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,31 +37,31 @@ func init() {

const openapiFileName = "openapi_cache"

type cachingOpenAPIClient struct {
type CachingOpenAPIClient struct {
version string
client discovery.OpenAPISchemaInterface
cacheDirName string
}

// newCachingOpenAPIClient returns a new discovery.OpenAPISchemaInterface
// NewCachingOpenAPIClient returns a new discovery.OpenAPISchemaInterface
// that will read the openapi spec from a local cache if it exists, and
// if not will then fetch an openapi spec using a client.
// client: used to fetch a new openapi spec if a local cache is not found
// version: the server version and used as part of the cache file location
// cacheDir: the directory under which the cache file will be written
func newCachingOpenAPIClient(client discovery.OpenAPISchemaInterface, version, cacheDir string) *cachingOpenAPIClient {
return &cachingOpenAPIClient{
func NewCachingOpenAPIClient(client discovery.OpenAPISchemaInterface, version, cacheDir string) *CachingOpenAPIClient {
return &CachingOpenAPIClient{
client: client,
version: version,
cacheDirName: cacheDir,
}
}

// openAPIData returns an openapi spec.
// OpenAPIData returns an openapi spec.
// It will first attempt to read the spec from a local cache
// If it cannot read a local cache, it will read the file
// using the client and then write the cache.
func (c *cachingOpenAPIClient) openAPIData() (*Resources, error) {
func (c *CachingOpenAPIClient) OpenAPIData() (*Resources, error) {
// Try to use the cached version
if c.useCache() {
doc, err := c.readOpenAPICache()
Expand All @@ -77,7 +77,7 @@ func (c *cachingOpenAPIClient) openAPIData() (*Resources, error) {
return nil, err
}

oa, err := newOpenAPIData(s)
oa, err := NewOpenAPIData(s)
if err != nil {
glog.V(2).Infof("Failed to parse openapi data %v", err)
return nil, err
Expand All @@ -97,12 +97,12 @@ func (c *cachingOpenAPIClient) openAPIData() (*Resources, error) {
}

// useCache returns true if the client should try to use the cache file
func (c *cachingOpenAPIClient) useCache() bool {
func (c *CachingOpenAPIClient) useCache() bool {
return len(c.version) > 0 && len(c.cacheDirName) > 0
}

// readOpenAPICache tries to read the openapi spec from the local file cache
func (c *cachingOpenAPIClient) readOpenAPICache() (*Resources, error) {
func (c *CachingOpenAPIClient) readOpenAPICache() (*Resources, error) {
// Get the filename to read
filename := c.openAPICacheFilename()

Expand All @@ -119,7 +119,7 @@ func (c *cachingOpenAPIClient) readOpenAPICache() (*Resources, error) {
}

// decodeSpec binary decodes the openapi spec
func (c *cachingOpenAPIClient) decodeSpec(data []byte) (*Resources, error) {
func (c *CachingOpenAPIClient) decodeSpec(data []byte) (*Resources, error) {
b := bytes.NewBuffer(data)
d := gob.NewDecoder(b)
parsed := &Resources{}
Expand All @@ -128,7 +128,7 @@ func (c *cachingOpenAPIClient) decodeSpec(data []byte) (*Resources, error) {
}

// encodeSpec binary encodes the openapi spec
func (c *cachingOpenAPIClient) encodeSpec(parsed *Resources) ([]byte, error) {
func (c *CachingOpenAPIClient) encodeSpec(parsed *Resources) ([]byte, error) {
b := &bytes.Buffer{}
e := gob.NewEncoder(b)
err := e.Encode(parsed)
Expand All @@ -138,7 +138,7 @@ func (c *cachingOpenAPIClient) encodeSpec(parsed *Resources) ([]byte, error) {

// writeToCache tries to write the openapi spec to the local file cache.
// writes the data to a new tempfile, and then links the cache file and the tempfile
func (c *cachingOpenAPIClient) writeToCache(parsed *Resources) error {
func (c *CachingOpenAPIClient) writeToCache(parsed *Resources) error {
// Get the constant filename used to read the cache.
cacheFile := c.openAPICacheFilename()

Expand Down Expand Up @@ -168,7 +168,7 @@ func (c *cachingOpenAPIClient) writeToCache(parsed *Resources) error {
}

// openAPICacheFilename returns the filename to read the cache from
func (c *cachingOpenAPIClient) openAPICacheFilename() string {
func (c *CachingOpenAPIClient) openAPICacheFilename() string {
// Cache using the client and server versions
return filepath.Join(c.cacheDirName, c.version, version.Get().GitVersion, openapiFileName)
}
Expand Down
36 changes: 19 additions & 17 deletions pkg/kubectl/cmd/util/openapi/openapi_cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package openapi
package openapi_test
Copy link
Member

Choose a reason for hiding this comment

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

Why change the package name? Is this because trying to match the package name of openapi_suite_test.go?
I found other pkg usually the same name for the test.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, to match the name of the suite. I forget the tradeoffs, but this is the recommended way for ginkgo


import (
"fmt"
Expand All @@ -27,25 +27,27 @@ import (
"github.com/go-openapi/spec"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi"
)

var _ = Describe("When reading openAPIData", func() {
var tmpDir string
var err error
var client *fakeOpenAPIClient
var instance *cachingOpenAPIClient
var expectedData *Resources
var instance *openapi.CachingOpenAPIClient
var expectedData *openapi.Resources

BeforeEach(func() {
tmpDir, err = ioutil.TempDir("", "openapi_cache_test")
Expect(err).To(BeNil())
client = &fakeOpenAPIClient{}
instance = newCachingOpenAPIClient(client, "v1.6", tmpDir)
instance = openapi.NewCachingOpenAPIClient(client, "v1.6", tmpDir)

d, err := data.OpenAPISchema()
Expect(err).To(BeNil())

expectedData, err = newOpenAPIData(d)
expectedData, err = openapi.NewOpenAPIData(d)
Expect(err).To(BeNil())
})

Expand All @@ -55,7 +57,7 @@ var _ = Describe("When reading openAPIData", func() {

It("should write to the cache", func() {
By("getting the live openapi spec from the server")
result, err := instance.openAPIData()
result, err := instance.OpenAPIData()
Expect(err).To(BeNil())
expectEqual(result, expectedData)
Expect(client.calls).To(Equal(1))
Expand All @@ -77,13 +79,13 @@ var _ = Describe("When reading openAPIData", func() {

It("should read from the cache", func() {
// First call should use the client
result, err := instance.openAPIData()
result, err := instance.OpenAPIData()
Expect(err).To(BeNil())
expectEqual(result, expectedData)
Expect(client.calls).To(Equal(1))

// Second call shouldn't use the client
result, err = instance.openAPIData()
result, err = instance.OpenAPIData()
Expect(err).To(BeNil())
expectEqual(result, expectedData)
Expect(client.calls).To(Equal(1))
Expand All @@ -96,7 +98,7 @@ var _ = Describe("When reading openAPIData", func() {
It("propagate errors that are encountered", func() {
// Expect an error
client.err = fmt.Errorf("expected error")
result, err := instance.openAPIData()
result, err := instance.OpenAPIData()
Expect(err.Error()).To(Equal(client.err.Error()))
Expect(result).To(BeNil())
Expect(client.calls).To(Equal(1))
Expand All @@ -107,7 +109,7 @@ var _ = Describe("When reading openAPIData", func() {
Expect(files).To(HaveLen(0))

// Client error is not cached
result, err = instance.openAPIData()
result, err = instance.OpenAPIData()
Expect(err.Error()).To(Equal(client.err.Error()))
Expect(result).To(BeNil())
Expect(client.calls).To(Equal(2))
Expand Down Expand Up @@ -138,16 +140,16 @@ var _ = Describe("Reading openAPIData", func() {
It("should not cache the result", func() {
client := &fakeOpenAPIClient{}

instance := newCachingOpenAPIClient(client, serverVersion, cacheDir)
instance := openapi.NewCachingOpenAPIClient(client, serverVersion, cacheDir)

d, err := data.OpenAPISchema()
Expect(err).To(BeNil())

expectedData, err := newOpenAPIData(d)
expectedData, err := openapi.NewOpenAPIData(d)
Expect(err).To(BeNil())

By("getting the live openapi schema")
result, err := instance.openAPIData()
result, err := instance.OpenAPIData()
Expect(err).To(BeNil())
expectEqual(result, expectedData)
Expect(client.calls).To(Equal(1))
Expand All @@ -166,16 +168,16 @@ var _ = Describe("Reading openAPIData", func() {
It("should not cache the result", func() {
client := &fakeOpenAPIClient{}

instance := newCachingOpenAPIClient(client, serverVersion, cacheDir)
instance := openapi.NewCachingOpenAPIClient(client, serverVersion, cacheDir)

d, err := data.OpenAPISchema()
Expect(err).To(BeNil())

expectedData, err := newOpenAPIData(d)
expectedData, err := openapi.NewOpenAPIData(d)
Expect(err).To(BeNil())

By("getting the live openapi schema")
result, err := instance.openAPIData()
result, err := instance.OpenAPIData()
Expect(err).To(BeNil())
expectEqual(result, expectedData)
Expect(client.calls).To(Equal(1))
Expand All @@ -200,7 +202,7 @@ func getFilenames(path string) ([]string, error) {
return result, nil
}

func expectEqual(a *Resources, b *Resources) {
func expectEqual(a *openapi.Resources, b *openapi.Resources) {
Expect(a.NameToDefinition).To(HaveLen(len(b.NameToDefinition)))
for k, v := range a.NameToDefinition {
Expect(v).To(Equal(b.NameToDefinition[k]),
Expand Down
6 changes: 3 additions & 3 deletions pkg/kubectl/cmd/util/openapi/openapi_getter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ var _ Getter = &synchronizedOpenAPIGetter{}

// Getter is an interface for fetching openapi specs and parsing them into an Resources struct
type Getter interface {
// openAPIData returns the parsed openAPIData
// OpenAPIData returns the parsed OpenAPIData
Get() (*Resources, error)
}

Expand All @@ -55,8 +55,8 @@ func NewOpenAPIGetter(cacheDir, serverVersion string, openAPIClient discovery.Op
// Resources implements Getter
func (g *synchronizedOpenAPIGetter) Get() (*Resources, error) {
g.Do(func() {
client := newCachingOpenAPIClient(g.openAPIClient, g.serverVersion, g.cacheDir)
result, err := client.openAPIData()
client := NewCachingOpenAPIClient(g.openAPIClient, g.serverVersion, g.cacheDir)
result, err := client.OpenAPIData()
if err != nil {
g.err = err
return
Expand Down
12 changes: 7 additions & 5 deletions pkg/kubectl/cmd/util/openapi/openapi_getter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,31 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package openapi
package openapi_test

import (
"fmt"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi"
)

var _ = Describe("Getting the Resources", func() {
var client *fakeOpenAPIClient
var expectedData *Resources
var instance Getter
var expectedData *openapi.Resources
var instance openapi.Getter

BeforeEach(func() {
client = &fakeOpenAPIClient{}
d, err := data.OpenAPISchema()
Expect(err).To(BeNil())

expectedData, err = newOpenAPIData(d)
expectedData, err = openapi.NewOpenAPIData(d)
Expect(err).To(BeNil())

instance = NewOpenAPIGetter("", "", client)
instance = openapi.NewOpenAPIGetter("", "", client)
})

Context("when the server returns a successful result", func() {
Expand Down