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

refactor events utils in e2e #85190

Merged
merged 2 commits into from Jan 8, 2020
Merged

refactor events utils in e2e #85190

merged 2 commits into from Jan 8, 2020

Conversation

danielqsj
Copy link
Contributor

What type of PR is this?

/kind feature
/sig testing

What this PR does / why we need it:

test/e2e/common is a mixture of tests (test/e2e/common/downward_api.go) and utility code (test/e2e/common/events.go). It would be nice to refactor it so that the utility code can be imported without also importing the tests. Most likely a better place for the utility code is under test/e2e/framework.

For this PR, the specific reason was the usage of WaitTimeoutForEvent in a storage test, which then made it impossible to import the storage tests without those other tests. See #83609. Once WaitTimeoutForEvent is in an independent utility package, the copied waitTimeoutForEvent can be removed from test/e2e/storage/testsuites/volumemode.go again.

Which issue(s) this PR fixes:

part of #83937

Special notes for your reviewer:

/cc @pohly

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/feature Categorizes issue or PR as related to a new feature. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Nov 13, 2019
@danielqsj danielqsj mentioned this pull request Nov 13, 2019
Closed
@k8s-ci-robot k8s-ci-robot added area/e2e-test-framework Issues or PRs related to refactoring the kubernetes e2e test framework area/test sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/storage Categorizes an issue or PR as relevant to SIG Storage. labels Nov 13, 2019
@danielqsj
Copy link
Contributor Author

/assign @pohly @oomichi

@danielqsj
Copy link
Contributor Author

/retest pull-kubernetes-conformance-image-test

@danielqsj
Copy link
Contributor Author

/retest

@danielqsj
Copy link
Contributor Author

weird, how to retest this case pull-kubernetes-conformance-image-test ?
@dims @BenTheElder

@danielqsj
Copy link
Contributor Author

danielqsj commented Nov 13, 2019 •
edited

it looks like same as #84940

@dims
Copy link
Member

dims commented Nov 13, 2019

/test pull-kubernetes-conformance-image-test

@dims
Copy link
Member

dims commented Nov 13, 2019

you can safely ignore pull-kubernetes-conformance-image-test failures. it won't affect merging this PR as it is not a REQUIRED job.

@dims
Copy link
Member

dims commented Nov 13, 2019

we are waiting for kind 0.6.0 to ship

@danielqsj
Copy link
Contributor Author

thanks @dims

@fejta-bot
Copy link

Unknown CLA label state. Rechecking for CLA labels.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/check-cla

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 13, 2019
@BenTheElder
Copy link
Member

/retest takes no arguments, /test takes a test name.
https://prow.k8s.io/command-help

@danielqsj
Copy link
Contributor Author

As #84439 and #84510 were merged.
Could we speed up this PR ? @pohly @oomichi

@oomichi
Copy link
Member

oomichi commented Dec 17, 2019

This seems good for me to share common code between e2e tests as subpackage of e2e framework without any invalid additional dependencies.

/retest
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 17, 2019
@danielqsj
Copy link
Contributor Author

/retest

@danielqsj
Copy link
Contributor Author

@fejta could you help review this PR which related to sig-testing ? thanks

@oomichi
Copy link
Member

oomichi commented Dec 30, 2019

/retest

@oomichi
Copy link
Member

oomichi commented Dec 30, 2019

/test pull-kubernetes-conformance-image-test

@oomichi
Copy link
Member

oomichi commented Jan 8, 2020

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danielqsj, oomichi

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 the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 8, 2020
@danielqsj
Copy link
Contributor Author

/retest

@danielqsj
Copy link
Contributor Author

/test pull-kubernetes-e2e-gce

@danielqsj
Copy link
Contributor Author

/test pull-kubernetes-e2e-kind

@k8s-ci-robot k8s-ci-robot merged commit 110da20 into kubernetes:master Jan 8, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.18 milestone Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pohly pohly Awaiting requested review from pohly

Assignees

@pohly pohly

@oomichi oomichi

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/e2e-test-framework Issues or PRs related to refactoring the kubernetes e2e test framework area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. release-note-none Denotes a PR that doesn't merit a release note. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.18
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@danielqsj @dims @fejta-bot @BenTheElder @oomichi @k8s-ci-robot @pohly