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

e2e: cleanup test/util/crd to decouple tests #77927

Merged
merged 1 commit into from
May 17, 2019
Merged

e2e: cleanup test/util/crd to decouple tests #77927

merged 1 commit into from
May 17, 2019

Conversation

sttts
Copy link
Contributor

@sttts sttts commented May 15, 2019 •
edited by liggitt

  • removed duplication of data in TestCRD
  • switch CRD creators to functional options pattern to make CRD spec mutable by tests before the CRD is instantiated.
NONE

@k8s-ci-robot k8s-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 15, 2019
@sttts
Copy link
Contributor Author

sttts commented May 15, 2019

/assign @jpbetz @liggitt

@k8s-ci-robot k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels May 15, 2019
@liggitt
Copy link
Member

liggitt commented May 15, 2019

/kind cleanup
/priority important-soon
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels May 15, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, sttts

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels May 15, 2019
@sttts sttts force-pushed the sttts-e2e-util-crd-cleanup branch from 6a0763e to 3e1fccd Compare May 15, 2019 15:34
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 15, 2019
@sttts sttts force-pushed the sttts-e2e-util-crd-cleanup branch from 3e1fccd to 4456746 Compare May 15, 2019 15:37
@sttts
Copy link
Contributor Author

sttts commented May 15, 2019

Fixed gofmt, golint and two former GetMetaName calls.

@sttts sttts force-pushed the sttts-e2e-util-crd-cleanup branch from 4456746 to 56fe7db Compare May 15, 2019 16:55
@sttts
Copy link
Contributor Author

sttts commented May 15, 2019 •
edited

And json->yaml.Unmarshal for the specs (which were patched in before).

@liggitt
Copy link
Member

liggitt commented May 15, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 15, 2019
@sttts sttts added this to the v1.15 milestone May 15, 2019
@sttts
Copy link
Contributor Author

sttts commented May 15, 2019

/retest

@sttts
Copy link
Contributor Author

sttts commented May 15, 2019

/retest pull-kubernetes-e2e-gce-100-performance

@sttts
Copy link
Contributor Author

sttts commented May 15, 2019

/test pull-kubernetes-e2e-gce-100-performance

@sttts sttts force-pushed the sttts-e2e-util-crd-cleanup branch from 56fe7db to a90cab0 Compare May 15, 2019 20:24
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 15, 2019
@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@sttts sttts added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 15, 2019
@yue9944882
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot merged commit dc462bd into kubernetes:master May 17, 2019
@sttts
Copy link
Contributor Author

sttts commented May 17, 2019

2 days in the merge queue

@alejandrox1 alejandrox1 mentioned this pull request May 19, 2019
Closed
liggitt
liggitt reviewed May 22, 2019
View reviewed changes
test/e2e/apimachinery/crd_publish_openapi.go
crd, err := crd.CreateMultiVersionTestCRD(f, group, apiVersions, nil)
if err != nil {
return nil, fmt.Errorf("failed to create CRD: %v", err)
if schema == nil {
Copy link
Member

Choose a reason for hiding this comment

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

this changed behavior... previously if nil was passed in, we did not set a validation schema on the CRD. the defaulting to type:object was only done in the value passed to waitForDefinition

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liggitt liggitt liggitt left review comments

@andrewsykim andrewsykim Awaiting requested review from andrewsykim

@dims dims Awaiting requested review from dims

Assignees

@jpbetz jpbetz

@liggitt liggitt

@yue9944882 yue9944882

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.15
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@sttts @liggitt @k8s-ci-robot @yue9944882 @jpbetz