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: handle nil ReplicaSet in checkDeploymentRevision #44859

Merged
merged 1 commit into from
Apr 24, 2017
Merged

e2e: handle nil ReplicaSet in checkDeploymentRevision #44859

merged 1 commit into from
Apr 24, 2017

Conversation

sttts
Copy link
Contributor

@sttts sttts commented Apr 24, 2017

deploymentutil.GetNewReplicaSet is allowed to return nil.

@sttts
e2e: handle nil ReplicaSet in checkDeploymentRevision
6b066ee 
deploymentutil.GetNewReplicaSet is allowed to return nil.
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 24, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@k8s-github-robot k8s-github-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed labels Apr 24, 2017
@sttts sttts assigned janetkuo and unassigned spiffxp and 0xmichalis Apr 24, 2017
@sttts sttts added kind/bug Categorizes issue or PR as related to a bug. release-note Denotes a PR that will be considered when it comes time to generate release notes. area/test and removed release-note-label-needed labels Apr 24, 2017
@0xmichalis
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 24, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@janetkuo
Copy link
Member

@k8s-bot gce etcd3 e2e test this

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 44788, 44859)

@k8s-github-robot k8s-github-robot merged commit 6236dfb into kubernetes:master Apr 24, 2017
@0xmichalis
Copy link
Contributor

@sttts did you see this flake anywhere? I have made a similar change in #44871 and the test fails with:

/go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/deployment.go:83
Expected
    <*v1beta1.ReplicaSet | 0x0>: nil
to equal
    <nil>: nil
/go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/deployment.go:521

@0xmichalis
Copy link
Contributor

Well, the change is a bit different but assuming that when rs is nil and Expect(rs).To(Equal(nil)) fails with the error above then Expect(newRS).NotTo(Equal(nil)) will never fail, right?

@sttts
Copy link
Contributor Author

sttts commented Apr 25, 2017

@Kargakis haven't seen that flake. Only saw a nil pointer panic before this PR. Now it causes a test error.

@0xmichalis 0xmichalis mentioned this pull request May 2, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@janetkuo janetkuo janetkuo approved these changes

Assignees

@0xmichalis 0xmichalis

@janetkuo janetkuo

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/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@sttts @k8s-reviewable @0xmichalis @k8s-github-robot @janetkuo @spiffxp @k8s-ci-robot