Mike Kolesnik has posted comments on this change. Change subject: core: MacPool related Commands ......................................................................
Patch Set 30: (6 comments) http://gerrit.ovirt.org/#/c/28705/30/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateMacPoolCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateMacPoolCommand.java: Line 79: this.macPoolFromDb = macPoolFromDb; Line 80: this.newMacPool = newMacPool; Line 81: } Line 82: Line 83: public ValidationResult defaultFlagIsNotChanged() { I think this method doesn't really require a separate class, it can be just a method on the UpdateMacPoolCommand class. Line 84: final boolean defaultChanged = macPoolFromDb.isDefaultPool() != newMacPool.isDefaultPool(); Line 85: return ValidationResult.failWith( Line 86: VdcBllMessages.ACTION_TYPE_FAILED_CHANGING_DEFAULT_MAC_POOL_IS_NOT_SUPPORTED) Line 87: .when(defaultChanged); http://gerrit.ovirt.org/#/c/28705/30/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MacPoolValidatorTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MacPoolValidatorTest.java: Line 41: Line 42: @Test Line 43: public void testDefaultPoolFlagIsNotSetValidUsage() throws Exception { Line 44: macPool.setDefaultPool(false); Line 45: Assert.assertThat(new MacPoolValidator(macPool).defaultPoolFlagIsNotSet(), You should statically import Assert.assertThat Line 46: isValid()); Line 47: } Line 48: Line 49: @Test Line 130: public void testNotRemovingUsedPoolRecordIsUsed() throws Exception { Line 131: macPool.setId(Guid.newGuid()); Line 132: when(macPoolDaoMock.getDcUsageCount(eq(macPool.getId()))).thenReturn(1); Line 133: Line 134: final ValidationResult validationResult = new MacPoolValidator(macPool).notRemovingUsedPool(); This can be inlined Line 135: Assert.assertThat(validationResult, Line 136: failsWith(VdcBllMessages.ACTION_TYPE_FAILED_CANNOT_REMOVE_STILL_USED_MAC_POOL)); Line 137: } Line 138: Line 140: public void testNotRemovingUsedPoolRecordNotUsed() throws Exception { Line 141: macPool.setId(Guid.newGuid()); Line 142: when(macPoolDaoMock.getDcUsageCount(eq(macPool.getId()))).thenReturn(0); Line 143: Line 144: final ValidationResult validationResult = new MacPoolValidator(macPool).notRemovingUsedPool(); This can be inlined Line 145: Assert.assertThat(validationResult, isValid()); Line 146: } Line 147: Line 148: @Test http://gerrit.ovirt.org/#/c/28705/30/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateMacPoolValidatorTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateMacPoolValidatorTest.java: Line 23: final MacPool macPool1 = new MacPool(); Line 24: final MacPool macPool2 = new MacPool(); Line 25: macPool2.setDefaultPool(!macPool1.isDefaultPool()); Line 26: Line 27: Assert.assertThat(new UpdateMacPoolCommand.UpdateMacPoolValidator(macPool1, macPool2).defaultFlagIsNotChanged(), ValidationResultMatchers.failsWith(VdcBllMessages.ACTION_TYPE_FAILED_CHANGING_DEFAULT_MAC_POOL_IS_NOT_SUPPORTED)); You should import Assert.assertThat and ValidationResultMatchers.failsWith and ValidationResultMatchers.isValid statically Line 28: } Line 29: Line 30: @Test Line 31: public void testDefaultFlagIsNotChangedFlagNotChanged() throws Exception { http://gerrit.ovirt.org/#/c/28705/30/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/MacPool.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/MacPool.java: Line 13: public class MacPool extends IVdcQueryable implements BusinessEntity<Guid> { Line 14: Line 15: private static final long serialVersionUID = -7952435653821354188L; Line 16: Line 17: @NotNull(message = "ACTION_TYPE_FAILED_MAC_POOL_DOES_NOT_EXIST", I wouldn't specify this message here, it would be confusing. Line 18: groups = { UpdateEntity.class }) Line 19: private Guid id; Line 20: Line 21: @NotEmpty(message = "ACTION_TYPE_FAILED_NAME_MAY_NOT_BE_EMPTY") -- To view, visit http://gerrit.ovirt.org/28705 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9c0c3657e3e53699bcafa375befdce848b01a2f3 Gerrit-PatchSet: 30 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
