-
Notifications
You must be signed in to change notification settings - Fork 238
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.
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
Move e2e-test helpers to a separate package #854
Move e2e-test helpers to a separate package #854
Conversation
Hi @fromanirh. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
WIP: I think I addressed all the relevant commits, but I'd like to run clean and CI and want to run tests on my side before to mark it reviewable |
9a4368c
to
43990cc
Compare
@marquiz could I please have ok-to-test here so I can run the final checks before to mark this reviewable? |
seems to work fine, albeit on a 1.22 kind cluster. OTOH nothing here requires kube >= 1.23, so removing the WIP |
/ok-to-test |
43990cc
to
c5eacfa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @fromanirh! This is a good improvement on the code base and it makes sense to review this in a separate PR. I had a few minor notes but after these are fixed lgtm.
Co-Authored-By Francesco Romani <fromani@redhat.com> Signed-off-by: Artyom Lukianov <alukiano@redhat.com>
c5eacfa
to
e169edc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ggod to me now 👍 If anything shows up let's fix it separately later 😄
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fromanirh, marquiz 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 |
/retitle Move e2e-test helpers method to the separate package |
sure thing! thanks for the quick reviews! |
/retitle Move e2e-test helpers to a separate package |
/test pull-node-feature-discovery-build-image-cross |
This cleanup/refactoring will facilitate the addition of the nfd-topology-updater tests (#528)