Allon Mureinik has posted comments on this change.
Change subject: core : QuotaMnager Test - basic structure
......................................................................
Patch Set 4: (9 inline comments)
some commends, see inline.
Also, I'm not sure i wouldn't squash this patch into the next one once the
review is finished, since on it's own it offers no benefit to the project (it
just sets the group for the subsequent patches).
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/quota/QuotaManager.java
Line 41:
Line 42: /**
Line 43: * This method is public for testing use only
Line 44: */
Line 45: public QuotaDAO getQuotaDAO() {
a/public/protected/
Line 46: return DbFacade.getInstance()
Line 47: .getQuotaDao();
Line 48: }
Line 49:
Line 43: * This method is public for testing use only
Line 44: */
Line 45: public QuotaDAO getQuotaDAO() {
Line 46: return DbFacade.getInstance()
Line 47: .getQuotaDao();
Also, while you are here, perhaps take the .getQuotaDao() up a line?
Line 48: }
Line 49:
Line 50: private AuditLogableBase getLoggableQuotaStorageParams(String
quotaName,
Line 51: double storageUsagePercentage,
Line 320:
Line 321: /**
Line 322: * This method is public for testing use only
Line 323: */
Line 324: public void log(Pair<AuditLogType, AuditLogableBase> logPair) {
1. s/pubic/protected/
2. Also, since you always seem to have the same "if (logPair.getFirst() !=
nul)" etc in the finally block, consider moving that if inside this function.
3. consider renaming the method to "auditLog" or something similar to indicate
it's a AuditLLog issue and not a log4j one.
Line 325: AuditLogDirector.log(logPair.getSecond(), logPair.getFirst());
Line 326: }
Line 327:
Line 328: private boolean checkQuotaStorageLimits(QuotaEnforcementTypeEnum
quotaEnforcementTypeEnum,
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/QuotaManagerTest.java
Line 8: import org.mockito.runners.MockitoJUnitRunner;
Line 9: import org.ovirt.engine.core.bll.quota.QuotaManager;
Line 10: import org.ovirt.engine.core.bll.quota.StorageQuotaValidationParameter;
Line 11: import org.ovirt.engine.core.common.AuditLogType;
Line 12: import org.ovirt.engine.core.common.businessentities.*;
Usw FQCN imports.
Line 13: import org.ovirt.engine.core.compat.Guid;
Line 14: import org.ovirt.engine.core.dal.dbbroker.DbFacade;
Line 15: import
org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector;
Line 16: import
org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase;
Line 74:
Line 75: private QuotaManager quotaManager =
Mockito.spy(QuotaManager.getInstance());
Line 76:
Line 77: private storage_pool storage_pool = new storage_pool();
Line 78: private ArrayList<String> canDoActionMessages = new
ArrayList<String>();
can you define the variable as "List" instead of "ArrayList"?
Line 79: private boolean hardEnforcement = true;
Line 80: private Guid destinationGuid = new
Guid("00000000-0000-0000-0000-000000002222");
Line 81:
Line 82: private boolean dbWasCalled = false;
Line 76:
Line 77: private storage_pool storage_pool = new storage_pool();
Line 78: private ArrayList<String> canDoActionMessages = new
ArrayList<String>();
Line 79: private boolean hardEnforcement = true;
Line 80: private Guid destinationGuid = new
Guid("00000000-0000-0000-0000-000000002222");
I'd move this a block up, together with the rest of the constants.
Line 81:
Line 82: private boolean dbWasCalled = false;
Line 83:
Line 84: @Before
Line 125: storage_pool.setId(new
Guid("00000000-0000-0000-0000-000000001111"));
Line 126: }
Line 127:
Line 128: private static void assertNotEmpty(List<?> list){
Line 129: assertTrue(list != null && list.size() > 0);
add a message please
Line 130: }
Line 131:
Line 132: private static void assertEmpty(List<?> list){
Line 133: assertTrue(list == null || list.size() == 0);
Line 129: assertTrue(list != null && list.size() > 0);
Line 130: }
Line 131:
Line 132: private static void assertEmpty(List<?> list){
Line 133: assertTrue(list == null || list.size() == 0);
here too
Line 134: }
Line 135:
Line 136: @Test
Line 137: public void testValidateAndSetStorageQuota() throws Exception {
....................................................
Commit Message
Line 5: CommitDate: 2012-10-09 18:29:55 +0200
Line 6:
Line 7: core : QuotaMnager Test - basic structure
Line 8:
Line 9: Basic structure to to QuotaManagerTest
s/to to/for/
Line 10:
Line 11: Change-Id: Ie2ff7377f01fdc9de9edb5650cc796e24a8f65c3
--
To view, visit http://gerrit.ovirt.org/8422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2ff7377f01fdc9de9edb5650cc796e24a8f65c3
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: ofri masad <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: ofri masad <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
