🌱 cleanup plugin_cluster-test.go #4137
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
Hi @mogsie. 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-sigs/prow repository. |
k8s-ci-robot
added
the
size/L
mogsie
changed the title
mogsie
changed the title
k8s-ci-robot
added
lgtm
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
HI @mogsie This one broke the tests so we need to fix the changes. /lgtm cancel |
camilamacedo86
removed
lgtm
Part of kubernetes-sigs#4135 This commits cleans up "plugin_cluster_test.go" to be less verbose and use better use of Gomega's capabilities. Notably, variables are used only when the output of some command is used later on in the test. If we only assert things about the command output, it is generally in-lined. Eventually functions are converted from `() error` to `(g Gomega)`, and Expects are converted to g.Expect inside the functions. All offsets have been removed, since this isn't "helper" code, we want to know which line the test failed on. Finally, the test for verifying available status has been fixed so that it checks the correct pod. Also, the "run-as-user" flag is passed in the "no options" case, since kubernetes doesn't allow the target pod to run as root.
mogsie
force-pushed
the
fix-plugin-cluster-test
branch
from
06c6355
to
2c65e83
Compare
k8s-ci-robot
added
the
approved
|
@camilamacedo86 the test that is failing is a real failure. I think the previous test was masking the problem (it was not expecting correctly, and the tests passed even though they were in error). The error is that docker / kubernetes does not support the "run as root" default of kubebuilder. I have added the "--run-as- |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, mogsie 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 |
Part of #4135
This commits cleans up "plugin_cluster_test.go" to be less verbose and use better use of Gomega's capabilities.
Notably, variables are used only when the output of some command is used later on in the test. If we only assert things about the command output, it is generally in-lined.
Eventually functions are converted from
() errorto(g Gomega), and Expects are converted to g.Expect inside the functions. All offsets have been removed, since this isn't "helper" code, we want to know which line the test failed on.Finally, the test for verifying available status has been fixed so that it checks the correct pod.