Liron Aravot has posted comments on this change.

Change subject: core: Use storage helper to fetch (related to BZ882825)
......................................................................


Patch Set 1: (3 inline comments)

....................................................
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageHelperBaseTest.java
Line 26: 
Line 27:     @Test
Line 28:     public void getLunConnectionsForISCSI() {
Line 29:         LUNs lun = new LUNs();
Line 30:         ArrayList<storage_server_connections> connections = new 
ArrayList<storage_server_connections>();
this method can recieve as parameter the name..but the name has no purpose 
here, i think that it will be better in a method rather then have it written 
here few times and also make the code longer.
Line 31:         connections.add(new storage_server_connections("Some LUN 
connection",
Line 32:                 "id",
Line 33:                 "iqn",
Line 34:                 "password",


Line 44:                 "Username",
Line 45:                 "port",
Line 46:                 "portal"));
Line 47: 
Line 48:         lun.setLunConnections(connections);
this checks should be exported as well to a method that will get type and 
expected size..
Line 49:         Map<StorageType, List<storage_server_connections>> 
connectionsByType =
Line 50:                 StorageHelperBase.filterLUNsByStorageType(lun);
Line 51:         assertTrue("Map of ISCSI storage connections should not be 
empty.", !connectionsByType.isEmpty());
Line 52:         assertEquals("Map of ISCSI storage connections should have 
only one type of connections.",


Line 84:                 StorageHelperBase.filterLUNsByStorageType(lun);
Line 85:         assertTrue("Map of storage connections should not be empty.", 
!connectionsByType.isEmpty());
Line 86:         assertEquals("Map of storage connections should have only two 
types of connections.",
Line 87:                 2,
Line 88:                 connectionsByType.size());
here it should be checked that the right connection was added to the map for 
each storage type key, what if by mistake connection with different type was 
added to some key?
Line 89:         assertEquals("Map of ISCSI storage connections should have 
only 1 ISCSI connections.",
Line 90:                 1,
Line 91:                 connectionsByType.get(StorageType.ISCSI).size());
Line 92:         assertEquals("Map of FCP storage connections should have only 
1 FCP connections.",


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida526dd9e08bc9881e65a529cb1b00aee0786af1
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to