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

Reply via email to