Vered Volansky has posted comments on this change.

Change subject: engine: Disk interface validation
......................................................................


Patch Set 7:

(2 comments)

A general comment regarding to all the tests here.
The DiskValidator should be tested once in it's own test.
When testing the commands what we should do is check that assuming the 
vaildator behaves correctly, the CDA yields the right value, not re-test the 
vaildator over and over again.
This can be done by mocking the DiskValidator in the relevant commands.

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java
Line 66:      */
Line 67:     public ValidationResult isOsSupportedForVirtIoScsi(VM vm) {
Line 68: 
Line 69:         List<DiskInterface> supportedDiskInterfaces =
Line 70:                 getSupportedDiskInterfaces(vm.getOs(), 
vm.getVdsGroupCompatibilityVersion());
Consider extracting this to a method since it's used again in 
isDiskInterfaceSupported.
Line 71: 
Line 72:         if 
(!supportedDiskInterfaces.contains(DiskInterface.VirtIO_SCSI)) {
Line 73:             return new 
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_GUEST_OS_VERSION_IS_NOT_SUPPORTED);
Line 74:         }


Line 88:      * @param version
Line 89:      *            The cluster version.
Line 90:      * @return An error if the disk interface is not compatible with 
the selected operating system.
Line 91:      */
Line 92:     public ValidationResult isDiskInterfaceSupported(VM vm) {
Should add a test in DiskValidatorTest
Line 93:         if (vm != null) {
Line 94:             List<DiskInterface> supportedDiskInterfaces =
Line 95:                     getSupportedDiskInterfaces(vm.getOs(), 
vm.getVdsGroupCompatibilityVersion());
Line 96: 


-- 
To view, visit http://gerrit.ovirt.org/18648
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe095557089aa5670c50eaa120eac9f60e13aea0
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gustavo Frederico Temple Pedrosa <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa 
<[email protected]>
Gerrit-Reviewer: Leonardo Bianconi <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
Gerrit-Reviewer: Vitor de Lima <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to