Alona Kaplan has posted comments on this change.

Change subject: engine: Add isManagementNetwork(String, Guid) to 
ManagementNetworkUtil
......................................................................


Patch Set 15:

(2 comments)

http://gerrit.ovirt.org/#/c/33816/15/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/cluster/ManagementNetworkUtilImplTest.java
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/cluster/ManagementNetworkUtilImplTest.java:

Line 112: 
Line 113:         assertEquals(expectedResult, actual);
Line 114: 
Line 115:         
verify(mockNetworkClusterDao).get(networkClusterIdCaptor.capture());
Line 116:         final NetworkClusterId networkClusterIdCaptured = 
networkClusterIdCaptor.getValue();
Why do you need the capture? Haven't you already verified it by doing- 
eq(TEST_NETWORK_CLUSTER_ID) and verifying it was called only once?
Line 117:         assertEquals(TEST_CLUSTER_ID, 
networkClusterIdCaptured.getClusterId());
Line 118:         assertEquals(TEST_NETWORK_ID, 
networkClusterIdCaptured.getNetworkId());
Line 119:     }
Line 120: 


Line 138:      * Test method for {@link 
ManagementNetworkUtilImpl#isManagementNetwork(String, Guid)} .
Line 139:      */
Line 140:     @Test
Line 141:     public void testIsManagementNetworkNameInGivenClusterNull() 
throws Exception {
Line 142:         testIsManagementNetworkNameInGivenClusterCommon(null, false);
The management network can't be null, I"m not sure why do you need this test.
I would add a test for- underTest.isManagementNetwork(null, TEST_CLUSTER_ID) 
and maybe even- underTest.isManagementNetwork(TEST_NETWORK_NAME, null)
Line 143:     }
Line 144: 
Line 145:     void testIsManagementNetworkNameInGivenClusterCommon(String 
managementNetworkName, boolean expected) {
Line 146:         if (managementNetworkName != null) {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8a6274614d0102ab944fad7a56b20856cced0a9a
Gerrit-PatchSet: 15
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yevgeny Zaspitsky <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[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