-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
VMIRS controller - move unit tests to fake client #11859
base: main
Are you sure you want to change the base?
VMIRS controller - move unit tests to fake client #11859
Conversation
Add the virtclientset to be used when vmirs and vmi client is interrogated. Add k8s.io/apiserver/pkg/storage/names dependency to allow performing server-side name generation behavior. (kubernetes/client-go#439) Signed-off-by: fossedihelm <ffossemo@redhat.com>
Register resources to the fakeclient during their addition to the informers. In this way wa can interact directly with the fakeclient that will store the changes that will be applied to the resources. Created `addVMI`convenience function. This allows us to remove some fake prepend reactor, since the resources are correctly registered (or not), in the client. Signed-off-by: fossedihelm <ffossemo@redhat.com>
The mockinterfaces need to be instructed to what expect before the controller execution, while using the fakeclient allow us to check resources after the controller execution. Convert the `shouldExpect` functions, which prepare the mockinterface to `expect` ones, which check the fakeclient. Signed-off-by: fossedihelm <ffossemo@redhat.com>
With fakeclient we can check action list to ensure that nothing happens, and filter for specific registered actions(`filterFakeActions`). Also, we can use prepend reactor to differentiate and customize behavior of the requests(`failSecondVMIAction`). Signed-off-by: fossedihelm <ffossemo@redhat.com>
vmiInterface and rsInterface are not needed anymore, since we now use the fakeclient. Also, moved the AfterEach near the BeforeEach to improve readability of the whole file and added the uid to the RS resource to better represent the reality. Signed-off-by: fossedihelm <ffossemo@redhat.com>
Introduce `expectReplicasAndReadyReplicas` and `expectConditions` functions that can be widely used in the test file, reducing code duplication and improving readability. Also, unexported functions: - `defaultReplicaSet` - `replicaSetFromVMI` Re-arranged tests code using always the same schema: - resource test setup - <newline> - controller execution - <newline> - resource expectations - events expectations Signed-off-by: fossedihelm <ffossemo@redhat.com>
e57988a
to
d449cad
Compare
@fossedihelm: The following test failed, say
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. I understand the commands that are listed here. |
@@ -59,8 +66,7 @@ var _ = Describe("Replicaset", func() { | |||
stop = make(chan struct{}) | |||
ctrl = gomock.NewController(GinkgoT()) | |||
virtClient := kubecli.NewMockKubevirtClient(ctrl) |
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.
Can we get rid of virtClient
completely?
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.
We cannot unfortunately. This is because the fakeclient does not implement the KubevirtClient
interface.
The latter defines the "shortcut" methods for:
VirtualMachineInstance(namespace string) VirtualMachineInstanceInterface
VirtualMachineInstanceMigration(namespace string) VirtualMachineInstanceMigrationInterface
ReplicaSet(namespace string) ReplicaSetInterface
VirtualMachinePool(namespace string) poolv1.VirtualMachinePoolInterface
VirtualMachine(namespace string) VirtualMachineInterface
KubeVirt(namespace string) KubeVirtInterface
VirtualMachineInstancePreset(namespace string) VirtualMachineInstancePresetInterface
VirtualMachineSnapshot(namespace string) vmsnapshotv1alpha1.VirtualMachineSnapshotInterface
VirtualMachineSnapshotContent(namespace string) vmsnapshotv1alpha1.VirtualMachineSnapshotContentInterface
VirtualMachineRestore(namespace string) vmsnapshotv1alpha1.VirtualMachineRestoreInterface
VirtualMachineExport(namespace string) vmexportv1alpha1.VirtualMachineExportInterface
VirtualMachineInstancetype(namespace string) instancetypev1beta1.VirtualMachineInstancetypeInterface
VirtualMachineClusterInstancetype() instancetypev1beta1.VirtualMachineClusterInstancetypeInterface
VirtualMachinePreference(namespace string) instancetypev1beta1.VirtualMachinePreferenceInterface
VirtualMachineClusterPreference() instancetypev1beta1.VirtualMachineClusterPreferenceInterface
and others, and these methods are not generated by the fakeclient.
Passing the fakeclient
to the controller creation causes a build error since the controller' services.TemplateService
requires a kubecli.KubevirtClient
.
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: acardace 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 |
What this PR does
Use fake client instead of mocking in order to simulate real behavior.
Fixes #
Why we need it and why it was done in this way
Lower the bar for unit tests, eliminate false positives. The fake is standard for testing.
Special notes for your reviewer
Checklist
This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.
Release note