Maor Lipchuk has posted comments on this change. Change subject: core: Add test for import unregistered VM ......................................................................
Patch Set 2: (12 comments) http://gerrit.ovirt.org/#/c/29795/2/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImportVMFromConfigurationCommandTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImportVMFromConfigurationCommandTest.java: Line 39: import org.ovirt.engine.core.utils.RandomUtilsSeedingRule; Line 40: Line 41: @RunWith(MockitoJUnitRunner.class) Line 42: public class ImportVMFromConfigurationCommandTest { Line 43: Guid vmId = Guid.newGuid(); > why not set these ad 'private final static'? done Line 44: Guid storageDomainId = Guid.newGuid(); Line 45: Guid storagePoolId = Guid.newGuid(); Line 46: Guid clusterId = Guid.newGuid(); Line 47: VDSGroup vdsGroup; Line 43: Guid vmId = Guid.newGuid(); Line 44: Guid storageDomainId = Guid.newGuid(); Line 45: Guid storagePoolId = Guid.newGuid(); Line 46: Guid clusterId = Guid.newGuid(); Line 47: VDSGroup vdsGroup; > private? done Line 48: Line 49: ImportVmFromConfigurationCommand<ImportVmParameters> cmd; Line 50: Line 51: @ClassRule Line 66: public void setUp() { Line 67: // init the injector with the osRepository instance Line 68: SimpleDependecyInjector.getInstance().bind(OsRepository.class, osRepository); Line 69: Line 70: final int osId = 0; > why final? isn't the variable redundant any way? removed it Line 71: Line 72: Map<Integer, Map<Version, List<DisplayType>>> displayTypeMap = new HashMap<>(); Line 73: displayTypeMap.put(osId, new HashMap<Version, List<DisplayType>>()); Line 74: displayTypeMap.get(osId).put(null, Arrays.asList(DisplayType.qxl)); Line 67: // init the injector with the osRepository instance Line 68: SimpleDependecyInjector.getInstance().bind(OsRepository.class, osRepository); Line 69: Line 70: final int osId = 0; Line 71: > blank line can be removed removed Line 72: Map<Integer, Map<Version, List<DisplayType>>> displayTypeMap = new HashMap<>(); Line 73: displayTypeMap.put(osId, new HashMap<Version, List<DisplayType>>()); Line 74: displayTypeMap.get(osId).put(null, Arrays.asList(DisplayType.qxl)); Line 75: when(osRepository.getDisplayTypes()).thenReturn(displayTypeMap); Line 75: when(osRepository.getDisplayTypes()).thenReturn(displayTypeMap); Line 76: setupTest(); Line 77: } Line 78: Line 79: protected ImportVmParameters createParametersWhenImagesExistOnTargetStorageDomain() { > this method should probably be near to 'initCommand' done Line 80: ImportVmParameters params = new ImportVmParameters(); Line 81: params.setContainerId(vmId); Line 82: params.setStorageDomainId(storageDomainId); Line 83: params.setVdsGroupId(clusterId); Line 95: } Line 96: Line 97: @Test Line 98: public void testImportVMFromConfigurationWhenStorageDomainIsInMaintenance() Line 99: { > formatter It is already fomatted Line 100: initCommand(getOvfEntityData()); Line 101: StorageDomain storageDomain = createStorageDomain(); Line 102: storageDomain.setStatus(StorageDomainStatus.Maintenance); Line 103: doReturn(storageDomain).when(cmd).getStorageDomain(); Line 104: assertFalse(cmd.canDoAction()); Line 105: } Line 106: Line 107: @Test Line 108: public void testImportVMFromConfigurationWhenStorageDomainIsInactive() { > The only difference between this test and the above one is StorageDomainSta I prefer not to, since it is only two methods and make another method might be more confusing then leave it as it is. It will also make the tests to be coupled, and changes will be much harder to implement that way Line 109: initCommand(getOvfEntityData()); Line 110: StorageDomain storageDomain = createStorageDomain(); Line 111: storageDomain.setStatus(StorageDomainStatus.Inactive); Line 112: doReturn(storageDomain).when(cmd).getStorageDomain(); Line 116: @Test Line 117: public void testImportVMFromConfigurationWhenVMDoesNotExists() { Line 118: initCommand(null); Line 119: assertFalse(cmd.canDoAction()); Line 120: Assert.assertTrue(cmd.getReturnValue() > remove 'Assert.' (there's already a static import) done Line 121: .getCanDoActionMessages() Line 122: .contains(VdcBllMessages.ACTION_TYPE_FAILED_UNSUPPORTED_OVF.toString())); Line 123: } Line 124: Line 128: ovfEntity.setOvfData("This is not a valid XML"); Line 129: initCommand(ovfEntity); Line 130: when(unregisteredOVFDataDao.getByEntityIdAndStorageDomain(vmId, storageDomainId)).thenReturn(ovfEntity); Line 131: assertFalse(cmd.canDoAction()); Line 132: Assert.assertTrue(cmd.getReturnValue() > remove 'Assert.' (there's already a static import) done Line 133: .getCanDoActionMessages() Line 134: .contains(VdcBllMessages.ACTION_TYPE_FAILED_OVF_CONFIGURATION_NOT_SUPPORTED.toString())); Line 135: } Line 136: Line 162: ovfEntity.setOvfData(getVmOvfDataXmlString()); Line 163: return ovfEntity; Line 164: } Line 165: Line 166: private void setVdsGroup() { > rename to mockVdsGroup done Line 167: vdsGroup = new VDSGroup(); Line 168: vdsGroup.setId(clusterId); Line 169: vdsGroup.setStoragePoolId(storagePoolId); Line 170: } Line 168: vdsGroup.setId(clusterId); Line 169: vdsGroup.setStoragePoolId(storagePoolId); Line 170: } Line 171: Line 172: private String getVmOvfDataXmlString() { > can we extract it to some helper (e.g. fixtures*)? moving this to backend/manager/modules/bll/src/test/resources Line 173: return "<?xml version='1.0' encoding='UTF-8'?><ovf:Envelope xmlns:ovf=\"http://schemas.dmtf.org/ovf/envelope/1/\"; xmlns:rasd=\"http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData\"; xmlns:vssd=\"http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData\"; xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"; ovf:version=\"3.5.0.0\"><References><File ovf:href=\"1107554f-ce13-44c0-bcf5-8a6f8837bc6c/3bb151be-7503-4511-820d-68eb52409c1d\" ovf:id=\"3bb151be-7503-4511-820d-68eb52409c1d\" ovf:size=\"1073741824\" ovf:description=\"Active VM\"/></References><Section xsi:type=\"ovf:NetworkSection_Type\"><Info>List of networks</Info><Network ovf:name=\"Network 1\"/></Section><Section xsi:type=\"ovf:DiskSection_Type\"><Info>List of Virtual Disks</Info><Disk ovf:diskId=\"3bb151be-7503-4511-820d-68eb52409c1d\" ovf:size=\"1\" ovf:actual_size=\"0\" ovf:vm_snapshot_id=\"e80cbc53-233d-4748-95bf-2499b34afda6\" ovf:parentRef! =\"\" ovf:fileRef=\"1107554f-ce13-44c0-bcf5-8a6f8837bc6c/3bb151be-7503-4511-820d-68eb52409c1d\" ovf:format=\"http://www.vmware.com/specifications/vmdk.html#sparse\"; ovf:volume-format=\"RAW\" ovf:volume-type=\"Sparse\" ovf:disk-interface=\"VirtIO\" ovf:boot=\"true\" ovf:disk-alias=\"Test_Disk1\" ovf:wipe-after-delete=\"false\"/></Section><Content ovf:id=\"out\" xsi:type=\"ovf:VirtualSystem_Type\"><Description></Description><CreationDate>2014/07/09 12:16:35</CreationDate><ExportDate>2014/07/09 12:18:19</ExportDate><DeleteProtected>false</DeleteProtected><SsoMethod>guest_agent</SsoMethod><IsSmartcardEnabled>false</IsSmartcardEnabled><TimeZone>Etc/GMT</TimeZone><default_boot_sequence>0</default_boot_sequence><Generation>2</Generation><VmType>1</VmType><MinAllocatedMem>1024</MinAllocatedMem><IsStateless>false</IsStateless><IsRunAndPause>false</IsRunAndPause><AutoStartup>false</AutoStartup><Priority>1</Priority><CreatedByUserId>fdfc627c-d875-11e0-90f0-83df133b58cc</CreatedByUserI! d><IsBootMenuEnabled>false</IsBootMenuEnabled><IsSpiceFileTransferEnabled>true</IsSpiceFileTransferEnabled><IsSpiceCopyPasteEnabled>true</IsSpiceCopyPasteEnabled><Name>Test</Name><TemplateId>00000000-0000-0000-0000-000000000000</TemplateId><TemplateName>Blank</TemplateName><IsInitilized>false</IsInitilized><Origin>3</Origin><DefaultDisplayType>1</DefaultDisplayType><TrustedService>false</TrustedService><OriginalTemplateId>00000000-0000-0000-0000-000000000000</OriginalTemplateId><OriginalTemplateName>Blank</OriginalTemplateName><UseLatestVersion>false</UseLatestVersion><Section ovf:id=\"2d3f2c0d-efe8-4428-aea7-16cc648dfef3\" ovf:required=\"false\" xsi:type=\"ovf:OperatingSystemSection_Type\"><Info>Guest Operating System</Info><Description>other</Description></Section><Section xsi:type=\"ovf:VirtualHardwareSection_Type\"><Info>1 CPU, 1024 Memeory</Info><System><vssd:VirtualSystemType>ENGINE 3.5.0.0</vssd:VirtualSystemType></System><Item><rasd:Caption>1 virtual cpu</rasd:Captio n><rasd:Description>Number of virtual CPU</rasd:Description><rasd:Inst! anceId>1</rasd:InstanceId><rasd:ResourceType>3</rasd:ResourceType><rasd:num_of_sockets>1</rasd:num_of_sockets><rasd:cpu_per_socket>1</rasd:cpu_per_socket></Item><Item><rasd:Caption>1024 MB of memory</rasd:Caption><rasd:Description>Memory Size</rasd:Description><rasd:InstanceId>2</rasd:InstanceId><rasd:ResourceType>4</rasd:ResourceType><rasd:AllocationUnits>MegaBytes</rasd:AllocationUnits><rasd:VirtualQuantity>1024</rasd:VirtualQuantity></Item><Item><rasd:Caption>Test_Disk1</rasd:Caption><rasd:InstanceId>3bb151be-7503-4511-820d-68eb52409c1d</rasd:InstanceId><rasd:ResourceType>17</rasd:ResourceType><rasd:HostResource>1107554f-ce13-44c0-bcf5-8a6f8837bc6c/3bb151be-7503-4511-820d-68eb52409c1d</rasd:HostResource><rasd:Parent>00000000-0000-0000-0000-000000000000</rasd:Parent><rasd:Template>00000000-0000-0000-0000-000000000000</rasd:Template><rasd:ApplicationList></rasd:ApplicationList><rasd:StorageId>3bf14d11-db1d-461b-b2c4-66499e243f38</rasd:StorageId><rasd:StoragePoolId>64c894a2! -05c6-4bb8-bb2d-2672224becae</rasd:StoragePoolId><rasd:CreationDate>2014/07/09 12:16:47</rasd:CreationDate><rasd:LastModified>1970/01/01 00:00:00</rasd:LastModified><rasd:last_modified_date>2014/07/09 12:18:19</rasd:last_modified_date></Item><Item><rasd:Caption>USB Controller</rasd:Caption><rasd:InstanceId>3</rasd:InstanceId><rasd:ResourceType>23</rasd:ResourceType><rasd:UsbPolicy>DISABLED</rasd:UsbPolicy></Item></Section><Section xsi:type=\"ovf:SnapshotsSection_Type\"><Snapshot ovf:id=\"e80cbc53-233d-4748-95bf-2499b34afda6\"><Type>ACTIVE</Type><Description>Active VM</Description><CreationDate>2014/07/09 12:16:35</CreationDate></Snapshot></Section></Content></ovf:Envelope>"; Line 174: } Line 175: Line 176: private void setupTest() { Line 172: private String getVmOvfDataXmlString() { Line 173: return "<?xml version='1.0' encoding='UTF-8'?><ovf:Envelope xmlns:ovf=\"http://schemas.dmtf.org/ovf/envelope/1/\"; xmlns:rasd=\"http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData\"; xmlns:vssd=\"http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData\"; xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"; ovf:version=\"3.5.0.0\"><References><File ovf:href=\"1107554f-ce13-44c0-bcf5-8a6f8837bc6c/3bb151be-7503-4511-820d-68eb52409c1d\" ovf:id=\"3bb151be-7503-4511-820d-68eb52409c1d\" ovf:size=\"1073741824\" ovf:description=\"Active VM\"/></References><Section xsi:type=\"ovf:NetworkSection_Type\"><Info>List of networks</Info><Network ovf:name=\"Network 1\"/></Section><Section xsi:type=\"ovf:DiskSection_Type\"><Info>List of Virtual Disks</Info><Disk ovf:diskId=\"3bb151be-7503-4511-820d-68eb52409c1d\" ovf:size=\"1\" ovf:actual_size=\"0\" ovf:vm_snapshot_id=\"e80cbc53-233d-4748-95bf-2499b34afda6\" ovf:parentRef! =\"\" ovf:fileRef=\"1107554f-ce13-44c0-bcf5-8a6f8837bc6c/3bb151be-7503-4511-820d-68eb52409c1d\" ovf:format=\"http://www.vmware.com/specifications/vmdk.html#sparse\"; ovf:volume-format=\"RAW\" ovf:volume-type=\"Sparse\" ovf:disk-interface=\"VirtIO\" ovf:boot=\"true\" ovf:disk-alias=\"Test_Disk1\" ovf:wipe-after-delete=\"false\"/></Section><Content ovf:id=\"out\" xsi:type=\"ovf:VirtualSystem_Type\"><Description></Description><CreationDate>2014/07/09 12:16:35</CreationDate><ExportDate>2014/07/09 12:18:19</ExportDate><DeleteProtected>false</DeleteProtected><SsoMethod>guest_agent</SsoMethod><IsSmartcardEnabled>false</IsSmartcardEnabled><TimeZone>Etc/GMT</TimeZone><default_boot_sequence>0</default_boot_sequence><Generation>2</Generation><VmType>1</VmType><MinAllocatedMem>1024</MinAllocatedMem><IsStateless>false</IsStateless><IsRunAndPause>false</IsRunAndPause><AutoStartup>false</AutoStartup><Priority>1</Priority><CreatedByUserId>fdfc627c-d875-11e0-90f0-83df133b58cc</CreatedByUserI! d><IsBootMenuEnabled>false</IsBootMenuEnabled><IsSpiceFileTransferEnabled>true</IsSpiceFileTransferEnabled><IsSpiceCopyPasteEnabled>true</IsSpiceCopyPasteEnabled><Name>Test</Name><TemplateId>00000000-0000-0000-0000-000000000000</TemplateId><TemplateName>Blank</TemplateName><IsInitilized>false</IsInitilized><Origin>3</Origin><DefaultDisplayType>1</DefaultDisplayType><TrustedService>false</TrustedService><OriginalTemplateId>00000000-0000-0000-0000-000000000000</OriginalTemplateId><OriginalTemplateName>Blank</OriginalTemplateName><UseLatestVersion>false</UseLatestVersion><Section ovf:id=\"2d3f2c0d-efe8-4428-aea7-16cc648dfef3\" ovf:required=\"false\" xsi:type=\"ovf:OperatingSystemSection_Type\"><Info>Guest Operating System</Info><Description>other</Description></Section><Section xsi:type=\"ovf:VirtualHardwareSection_Type\"><Info>1 CPU, 1024 Memeory</Info><System><vssd:VirtualSystemType>ENGINE 3.5.0.0</vssd:VirtualSystemType></System><Item><rasd:Caption>1 virtual cpu</rasd:Captio n><rasd:Description>Number of virtual CPU</rasd:Description><rasd:Inst! anceId>1</rasd:InstanceId><rasd:ResourceType>3</rasd:ResourceType><rasd:num_of_sockets>1</rasd:num_of_sockets><rasd:cpu_per_socket>1</rasd:cpu_per_socket></Item><Item><rasd:Caption>1024 MB of memory</rasd:Caption><rasd:Description>Memory Size</rasd:Description><rasd:InstanceId>2</rasd:InstanceId><rasd:ResourceType>4</rasd:ResourceType><rasd:AllocationUnits>MegaBytes</rasd:AllocationUnits><rasd:VirtualQuantity>1024</rasd:VirtualQuantity></Item><Item><rasd:Caption>Test_Disk1</rasd:Caption><rasd:InstanceId>3bb151be-7503-4511-820d-68eb52409c1d</rasd:InstanceId><rasd:ResourceType>17</rasd:ResourceType><rasd:HostResource>1107554f-ce13-44c0-bcf5-8a6f8837bc6c/3bb151be-7503-4511-820d-68eb52409c1d</rasd:HostResource><rasd:Parent>00000000-0000-0000-0000-000000000000</rasd:Parent><rasd:Template>00000000-0000-0000-0000-000000000000</rasd:Template><rasd:ApplicationList></rasd:ApplicationList><rasd:StorageId>3bf14d11-db1d-461b-b2c4-66499e243f38</rasd:StorageId><rasd:StoragePoolId>64c894a2! -05c6-4bb8-bb2d-2672224becae</rasd:StoragePoolId><rasd:CreationDate>2014/07/09 12:16:47</rasd:CreationDate><rasd:LastModified>1970/01/01 00:00:00</rasd:LastModified><rasd:last_modified_date>2014/07/09 12:18:19</rasd:last_modified_date></Item><Item><rasd:Caption>USB Controller</rasd:Caption><rasd:InstanceId>3</rasd:InstanceId><rasd:ResourceType>23</rasd:ResourceType><rasd:UsbPolicy>DISABLED</rasd:UsbPolicy></Item></Section><Section xsi:type=\"ovf:SnapshotsSection_Type\"><Snapshot ovf:id=\"e80cbc53-233d-4748-95bf-2499b34afda6\"><Type>ACTIVE</Type><Description>Active VM</Description><CreationDate>2014/07/09 12:16:35</CreationDate></Snapshot></Section></Content></ovf:Envelope>"; Line 174: } Line 175: Line 176: private void setupTest() { > I think we can remove this method for now and revisit if extraction is need done Line 177: setVdsGroup(); Line 178: } Line 179: Line 180: protected StorageDomain createStorageDomain() { -- To view, visit http://gerrit.ovirt.org/29795 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie6f68a66da4de6b9dbfb664d3678b25582a82496 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: [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
