-
Notifications
You must be signed in to change notification settings - Fork 2
test(vmop): add restore vmop e2e test #1648
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's GuideThis PR implements a full end-to-end test for VM restore operations via VMOP by extending the test framework with new util and builder helpers, adding a dedicated restore test suite, and updating object constructors to support restore-specific options. Class diagram for new VM snapshot builder and optionsclassDiagram
class VirtualMachineSnapshot {
+TypeMeta
+ObjectMeta
+Spec: VirtualMachineSnapshotSpec
}
class VirtualMachineSnapshotSpec {
+RequiredConsistency: bool
+KeepIPAddress: KeepIPAddress
+VirtualMachineName: string
}
class Option {
<<interface>>
+apply(vmsnapshot: VirtualMachineSnapshot)
}
VirtualMachineSnapshotSpec <|-- VirtualMachineSnapshot
Option <|.. VirtualMachineSnapshot
VirtualMachineSnapshot "1" *-- "1" VirtualMachineSnapshotSpec
Class diagram for new VMOP restore optionsclassDiagram
class VirtualMachineOperation {
+Spec: VirtualMachineOperationSpec
}
class VirtualMachineOperationSpec {
+Force: bool
+Restore: VirtualMachineOperationRestoreSpec
}
class VirtualMachineOperationRestoreSpec {
+Mode: VMOPRestoreMode
+VirtualMachineSnapshotName: string
}
VirtualMachineOperationSpec <|-- VirtualMachineOperation
VirtualMachineOperationRestoreSpec <|-- VirtualMachineOperationSpec
VirtualMachineOperation "1" *-- "1" VirtualMachineOperationSpec
VirtualMachineOperationSpec "1" *-- "0..1" VirtualMachineOperationRestoreSpec
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- The
vmop/restore.goe2e test is quite large and repetitive; consider breaking it into smaller table-driven subtests or extracting common setup/validation logic into reusable helpers to improve readability and maintainability. - In the builder functions wrapping
vd.New, you append user-provided options after base options, which can lead to precedence issues; consider documenting the override order clearly or prepending base options so callers can reliably override defaults. - You introduced
Framework.UpdateFromClusterbut most retrievals still usef.Clients.GenericClient(); consolidating on a single pattern for updating objects (either the new helper or the existing client) would reduce confusion and duplication.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `vmop/restore.go` e2e test is quite large and repetitive; consider breaking it into smaller table-driven subtests or extracting common setup/validation logic into reusable helpers to improve readability and maintainability.
- In the builder functions wrapping `vd.New`, you append user-provided options after base options, which can lead to precedence issues; consider documenting the override order clearly or prepending base options so callers can reliably override defaults.
- You introduced `Framework.UpdateFromCluster` but most retrievals still use `f.Clients.GenericClient()`; consolidating on a single pattern for updating objects (either the new helper or the existing client) would reduce confusion and duplication.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
ffd1511 to
558e087
Compare
4227e81 to
45e1ed1
Compare
73ae2a3 to
33ea848
Compare
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
472bbc7 to
7bb38ab
Compare
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Description
Add e2e test for VM restore VMOP.
Checklist
Changelog entries