Mike Kolesnik has posted comments on this change. Change subject: core: pool per DC ......................................................................
Patch Set 44: Code-Review-1 (7 comments) Just a few small fixes http://gerrit.ovirt.org/#/c/26799/44/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 32: import org.ovirt.engine.core.dao.StoragePoolDAO; Line 33: import org.ovirt.engine.core.dao.network.VmNicDao; Line 34: Line 35: @RunWith(MockitoJUnitRunner.class) Line 36: public class MacPoolPerDcTest { You can extract a field for the entire class (I see it in every test): final MacPoolPerDc pool = new MacPoolPerDc(); Line 37: @Rule Line 38: public ExpectedException expectedException = ExpectedException.none(); Line 39: Line 40: @Rule Line 69: when(dbFacade.getVmNicDao()).thenReturn(vmNicDao); Line 70: when(dbFacade.getMacPoolDao()).thenReturn(macPoolDao); Line 71: Line 72: macFrom = "00:1a:4a:15:c0:00"; Line 73: macTo = "00:1a:4a:15:c0:ff"; These two seem to me like constants so should be converted to constants.. Line 74: macPool = createMacPool(macFrom, macTo); Line 75: dataCenter = createStoragePool(macPool); Line 76: vmNic = createVmNic(); Line 77: } Line 145: Line 146: @Test Line 147: public void testModifyOfExistingMacPool() throws Exception { Line 148: final MacPoolPerDc pool = new MacPoolPerDc(); Line 149: MacPool macPool = createMacPool("00:00:00:00:00:01", "00:00:00:00:00:01"); "00:00:00:00:00:01" should be extracted to a local constant. Line 150: StoragePool dataCenter = createStoragePool(macPool); Line 151: Line 152: when(dbFacade.getAuditLogDao()).thenReturn(auditLogDao); Line 153: Line 146: @Test Line 147: public void testModifyOfExistingMacPool() throws Exception { Line 148: final MacPoolPerDc pool = new MacPoolPerDc(); Line 149: MacPool macPool = createMacPool("00:00:00:00:00:01", "00:00:00:00:00:01"); Line 150: StoragePool dataCenter = createStoragePool(macPool); You could override the fields that you already have, but it's not critical Line 151: Line 152: when(dbFacade.getAuditLogDao()).thenReturn(auditLogDao); Line 153: Line 154: mockStoragePool(dataCenter); Line 148: final MacPoolPerDc pool = new MacPoolPerDc(); Line 149: MacPool macPool = createMacPool("00:00:00:00:00:01", "00:00:00:00:00:01"); Line 150: StoragePool dataCenter = createStoragePool(macPool); Line 151: Line 152: when(dbFacade.getAuditLogDao()).thenReturn(auditLogDao); This is constant behavior so it can be done in setUp method Line 153: Line 154: mockStoragePool(dataCenter); Line 155: mockGettingAllMacPools(macPool); Line 156: pool.initialize(); Line 157: Line 158: assertThat(pool.poolForDataCenter(dataCenter.getId()).addMac(macFrom), is(true)); Line 159: assertThat(pool.poolForDataCenter(dataCenter.getId()).addMac(macFrom), is(false)); Line 160: Line 161: pool.poolForDataCenter(dataCenter.getId()).allocateNewMac(); You should check that you get the expected value here. Line 162: try { Line 163: pool.poolForDataCenter(dataCenter.getId()).allocateNewMac(); Line 164: Assert.fail("this allocation should not succeed."); Line 165: } catch (VdcBLLException e) { Line 174: macPool.setRanges(Collections.singletonList(macRange)); Line 175: pool.modifyPool(macPool); Line 176: Line 177: assertThat(pool.poolForDataCenter(dataCenter.getId()).addMac(macFrom), is(true)); Line 178: assertThat(pool.poolForDataCenter(dataCenter.getId()).allocateNewMac(), notNullValue()); You should check that it's the second MAC, as the first one was already allocated before the modify, leaving only one possible MAC to allocate - "00:00:00:00:00:02" Line 179: } Line 180: Line 181: @Test Line 182: public void testModifyOfNotExistingMacPool() throws Exception { -- 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: 44 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
