Mike Kolesnik has posted comments on this change. Change subject: core: pool per DC ......................................................................
Patch Set 42: Code-Review-1 (6 comments) http://gerrit.ovirt.org/#/c/26799/42/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/MacPoolPerDcTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/MacPoolPerDcTest.java: Line 44: private DbFacade dbFacadeMock; Line 45: Line 46: @Before Line 47: public void setUp() throws Exception { Line 48: dbFacadeMock = mock(DbFacade.class); You should use @Mock annotation for all these mocks and achieve cleaner code. Line 49: storagePoolDaoMock = mock(StoragePoolDAO.class); Line 50: vmNicDaoMock = mock(VmNicDao.class); Line 51: macPoolDaoMock = mock(MacPoolDao.class); Line 52: Line 67: pool.initialize(); Line 68: Mockito.verify(dbFacadeMock).getMacPoolDao(); Line 69: Mockito.verify(macPoolDaoMock).getAll(); Line 70: Line 71: Mockito.verifyNoMoreInteractions(storagePoolDaoMock, vmNicDaoMock, macPoolDaoMock, dbFacadeMock); I believe that this should come after the second call, not before.. Or even better, both here and there.. Line 72: pool.initialize(); Line 73: //should not fail. Line 74: } Line 75: Line 87: final MacPoolPerDc pool = new MacPoolPerDc(); Line 88: mockStoragePools(storagePool); Line 89: mockGettingAllMacPools(macPool); Line 90: pool.initialize(); Line 91: assertThat(pool.getMacPoolId(storagePool.getId()), is(notNullValue())); You shouldn't be testing this method since it's not public. Line 92: assertThat(pool.poolForDataCenter(storagePool.getId()), is(notNullValue())); Line 93: } Line 94: Line 95: @Test Line 121: @Test Line 122: public void testCreatePoolWhichExists() throws Exception { Line 123: final MacPoolPerDc pool = new MacPoolPerDc(); Line 124: Line 125: mockGettingAllMacPools(macPool); No need for this.. Line 126: pool.initialize(); Line 127: Line 128: expectedException.expect(IllegalStateException.class); Line 129: expectedException.expectMessage(MacPoolPerDc.UNABLE_TO_CREATE_MAC_POOL_IT_ALREADY_EXIST); Line 219: vmNic.setMacAddress("00:1a:4a:15:c0:fe"); Line 220: return vmNic; Line 221: } Line 222: Line 223: protected void mockStoragePools(StoragePool storagePool) { s/mockStoragePools/mockStoragePool/ Line 224: //mock obtaining storagepool by id. Line 225: when(storagePoolDaoMock.get(eq(storagePool.getId()))).thenReturn(storagePool); Line 226: } Line 227: Line 220: return vmNic; Line 221: } Line 222: Line 223: protected void mockStoragePools(StoragePool storagePool) { Line 224: //mock obtaining storagepool by id. This comment is unnecessary (code is self explanatory) Line 225: when(storagePoolDaoMock.get(eq(storagePool.getId()))).thenReturn(storagePool); Line 226: } Line 227: Line 228: private void expectNotInitializedException() { -- To view, visit http://gerrit.ovirt.org/26799 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If522a0b0d5f810281bed010b46b5242f7dbdcc29 Gerrit-PatchSet: 42 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> Gerrit-Reviewer: Martin Mucha <[email protected]> Gerrit-Reviewer: Mike Kolesnik <[email protected]> Gerrit-Reviewer: Moti Asayag <[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
