VMIRS controller - move unit tests to fake client #11859
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kubevirt-bot
added
release-note-none
kubevirt-bot
added
sig/buildsystem
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>
fossedihelm
force-pushed
the
use-fakeclient-vmirs-controller
branch
from
e57988a
to
d449cad
Compare
kubevirt-bot
removed
the
needs-rebase
|
@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. |
acardace
reviewed
May 9, 2024
| @@ -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.
Can we get rid of virtClient completely?
There was a problem hiding this comment.
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 |
kubevirt-bot
added
the
approved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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