Maor Lipchuk has posted comments on this change. Change subject: core: Add test for import unregistered VM ......................................................................
Patch Set 4: (10 comments) http://gerrit.ovirt.org/#/c/29795/4/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 42: public class ImportVMFromConfigurationCommandTest { Line 43: private final Guid vmId = Guid.newGuid(); Line 44: private final Guid storageDomainId = Guid.newGuid(); Line 45: private final Guid storagePoolId = Guid.newGuid(); Line 46: private final Guid clusterId = Guid.newGuid(); > All of these should be initialized in the @Before method. done although I'm not sure what is the difference, we will still declare them in the class, and we are actually adding more lines of code by initializing them in the before method Line 47: private static final String VM_OVF_XML_DATA = "src/test/resources/vmOvfData.xml"; Line 48: private String xmlOvfData; Line 49: private VDSGroup vdsGroup; Line 50: Line 47: private static final String VM_OVF_XML_DATA = "src/test/resources/vmOvfData.xml"; Line 48: private String xmlOvfData; Line 49: private VDSGroup vdsGroup; Line 50: Line 51: ImportVmFromConfigurationCommand<ImportVmParameters> cmd; > should be private done Line 52: Line 53: @ClassRule Line 54: public static MockConfigRule mcr = new MockConfigRule( Line 55: mockConfig(ConfigValues.VirtIoScsiEnabled, Version.v3_2.toString(), false) Line 55: mockConfig(ConfigValues.VirtIoScsiEnabled, Version.v3_2.toString(), false) Line 56: ); Line 57: Line 58: @Mock Line 59: OsRepository osRepository; > should be private done Line 60: Line 61: @Mock Line 62: UnregisteredOVFDataDAO unregisteredOVFDataDao; Line 63: Line 58: @Mock Line 59: OsRepository osRepository; Line 60: Line 61: @Mock Line 62: UnregisteredOVFDataDAO unregisteredOVFDataDao; > should be private done Line 63: Line 64: @Before Line 65: public void setUp() { Line 66: // init the injector with the osRepository instance Line 76: Line 77: private void setXmlOvfData() { Line 78: try { Line 79: xmlOvfData = new String(Files.readAllBytes(Paths.get(VM_OVF_XML_DATA))); Line 80: } catch (IOException e) { > Just throw it upwards. done Line 81: e.printStackTrace(); Line 82: } Line 83: } Line 84: Line 77: private void setXmlOvfData() { Line 78: try { Line 79: xmlOvfData = new String(Files.readAllBytes(Paths.get(VM_OVF_XML_DATA))); Line 80: } catch (IOException e) { Line 81: e.printStackTrace(); > https://encrypted-tbn2.gstatic.com/images?q=tbn:ANd9GcQdlzbyrKlqB7J6fGUW86c :) http://thumb9.shutterstock.com/photos/display_pic_with_logo/576460/576460,1274832009,2.jpg Line 82: } Line 83: } Line 84: Line 85: @Test Line 89: doReturn(Boolean.TRUE).when(cmd).canDoActionAfterCloneVm(anyMap()); Line 90: doReturn(Boolean.TRUE).when(cmd).canDoActionBeforeCloneVm(anyMap()); Line 91: boolean canDoReturn = cmd.canDoAction(); Line 92: assertTrue("The CDA assertion is false, Those are the following messages " Line 93: + cmd.getReturnValue().getCanDoActionMessages(), canDoReturn); > Please use CanDoActionAssertUtils. nice, done Line 94: } Line 95: Line 96: @Test Line 97: public void testImportVMFromConfigurationWhenStorageDomainIsInMaintenance() { Line 98: initCommand(getOvfEntityData()); Line 99: StorageDomain storageDomain = createStorageDomain(); Line 100: storageDomain.setStatus(StorageDomainStatus.Maintenance); Line 101: doReturn(storageDomain).when(cmd).getStorageDomain(); Line 102: assertFalse(cmd.canDoAction()); > Please use CanDoActionAssertUtils. I don't think it is such a good idea to force a message in the assertUtils failure, Tests might fail if someone will change the order of the CDA validation which does not necessarily has any obligation to each other. Line 103: } Line 104: Line 105: @Test Line 106: public void testImportVMFromConfigurationWhenStorageDomainIsInactive() { Line 131: .getCanDoActionMessages() Line 132: .contains(VdcBllMessages.ACTION_TYPE_FAILED_OVF_CONFIGURATION_NOT_SUPPORTED.toString())); Line 133: } Line 134: Line 135: protected ImportVmParameters createParametersWhenImagesExistOnTargetStorageDomain() { > should be private static done Line 136: ImportVmParameters params = new ImportVmParameters(); Line 137: params.setContainerId(vmId); Line 138: params.setStorageDomainId(storageDomainId); Line 139: params.setVdsGroupId(clusterId); http://gerrit.ovirt.org/#/c/29795/4/backend/manager/modules/bll/src/test/resources/vmOvfData.xml File backend/manager/modules/bll/src/test/resources/vmOvfData.xml: Line 1: <?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.3.0.0"><References><File ovf:href="8c634412-1e8b-4ef3-bc40-b67a456e9d2f/f934b12c-1e22-4ad8-bbce-ec0b2a5defa4" ovf:id="f934b12c-1e22-4ad8-bbce-ec0b2a5defa4" 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="f934b12c-1e22-4ad8-bbce-ec0b2a5defa4" ovf:size="1" ovf:actual_size="0" ovf:vm_snapshot_id="b60fcbad-d65a-4248-ae32-7f9411276df0" ovf:parentRef="" ovf:fileRef="8c634412-1e8b-4ef3-bc40-b67a456e9! d2f/f934b12c-1e22-4ad8-bbce-ec0b2a5defa4" 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="exporttedVM_Disk1" ovf:wipe-after-delete="false"/></Section><Content ovf:id="out" xsi:type="ovf:VirtualSystem_Type"><Description>fsdf</Description><Domain></Domain><CreationDate>2013/05/27 14:44:12</CreationDate><ExportDate>2013/05/27 14:47:16</ExportDate><IsAutoSuspend>false</IsAutoSuspend><DeleteProtected>false</DeleteProtected><IsSmartcardEnabled>false</IsSmartcardEnabled><TimeZone></TimeZone><default_boot_sequence>0</default_boot_sequence><Generation>2</Generation><VmType>0</VmType><MinAllocatedMem>1024</MinAllocatedMem><IsStateless>false</IsStateless><IsRunAndPause>false</IsRunAndPause><Name>exporttedVM</Name><TemplateId>00000000-0000-0000-0000-000000000000</TemplateId><TemplateName>Blank</TemplateName><IsInitilized>false</IsInitilized><Origin>0</Origin><! quota_id>00000000-0000-0000-0000-000000000000</quota_id><DefaultDisplayType>1</DefaultDisplayType><Section ovf:id="70f24c82-a7b8-4e0b-9192-cffd5705cf5c" ovf:required="false" xsi:type="ovf:OperatingSystemSection_Type"><Info>Guest Operating System</Info><Description>Unassigned</Description></Section><Section xsi:type="ovf:VirtualHardwareSection_Type"><Info>1 CPU, 1024 Memeory</Info><System><vssd:VirtualSystemType>ENGINE 3.3.0.0</vssd:VirtualSystemType></System><Item><rasd:Caption>1 virtual cpu</rasd:Caption><rasd:Description>Number of virtual CPU</rasd:Description><rasd:InstanceId>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:VirtualQuanti ty>1024</rasd:VirtualQuantity></Item><Item><rasd:Caption>exporttedVM_D! isk1</rasd:Caption><rasd:InstanceId>f934b12c-1e22-4ad8-bbce-ec0b2a5defa4</rasd:InstanceId><rasd:ResourceType>17</rasd:ResourceType><rasd:HostResource>8c634412-1e8b-4ef3-bc40-b67a456e9d2f/f934b12c-1e22-4ad8-bbce-ec0b2a5defa4</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>7e2a7eac-3b76-4d45-a7dd-caae8fe0f588</rasd:StorageId><rasd:StoragePoolId>5c80c932-20ef-4c55-9e06-383c6152a0dc</rasd:StoragePoolId><rasd:CreationDate>2013/05/27 14:44:24</rasd:CreationDate><rasd:LastModified>2013/05/27 14:44:24</rasd:LastModified><rasd:last_modified_date>2013/05/27 14:47:16</rasd:last_modified_date><Type>disk</Type><Device>disk</Device><rasd:Address></rasd:Address><BootOrder>0</BootOrder><IsPlugged>true</IsPlugged><IsReadOnly>false</IsReadOnly><Alias></Alias></Item><Item><rasd:Caption>USB Controller</rasd:Caption><rasd:InstanceId>3<! /rasd:InstanceId><rasd:ResourceType>23</rasd:ResourceType><rasd:UsbPolicy>DISABLED</rasd:UsbPolicy></Item><Item><rasd:Caption>Graphical Controller</rasd:Caption><rasd:InstanceId>d4158260-e1b6-4121-aacf-336da29613a8</rasd:InstanceId><rasd:ResourceType>20</rasd:ResourceType><rasd:VirtualQuantity>1</rasd:VirtualQuantity><Type>video</Type><Device>qxl</Device><rasd:Address></rasd:Address><BootOrder>0</BootOrder><IsPlugged>true</IsPlugged><IsReadOnly>true</IsReadOnly><Alias></Alias><SpecParams><vram>65536</vram></SpecParams></Item><Item><rasd:ResourceType>0</rasd:ResourceType><rasd:InstanceId>0ef21074-0908-4d2d-8355-5e82a3f49881</rasd:InstanceId><Type>balloon</Type><Device>memballoon</Device><rasd:Address></rasd:Address><BootOrder>0</BootOrder><IsPlugged>true</IsPlugged><IsReadOnly>true</IsReadOnly><Alias></Alias><SpecParams><model>virtio</model></SpecParams></Item><Item><rasd:ResourceType>0</rasd:ResourceType><rasd:InstanceId>24572822-aa3d-483d-9639-6dcf01925898</rasd:InstanceId> <Type>sound</Type><Device>ich6</Device><rasd:Address></rasd:Address><B! ootOrder>0</BootOrder><IsPlugged>true</IsPlugged><IsReadOnly>true</IsReadOnly><Alias></Alias></Item></Section><Section xsi:type="ovf:SnapshotsSection_Type"><Snapshot ovf:id="b60fcbad-d65a-4248-ae32-7f9411276df0"><Type>ACTIVE</Type><Description>Active VM</Description><CreationDate>2013/05/27 14:44:12</CreationDate></Snapshot></Section></Content></ovf:Envelope> > Formatting? It is a resource of OVF xml file, this is most close to the real files we are fetching from the tar file -- 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: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Allon Mureinik <[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
