Liron Aravot has posted comments on this change. Change subject: core: create ReconstructMasterDomain validators ......................................................................
Patch Set 9: (5 comments) http://gerrit.ovirt.org/#/c/28624/9/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/StorageDomainValidator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/StorageDomainValidator.java: Line 238: private static interface SizeAssessment { Line 239: public double getSizeForDisk(DiskImage diskImage); Line 240: } Line 241: Line 242: public ValidationResult isInProcess() { sorry for not bringing this up before, but consider to export the common logic between the validators to the StoragePoolDomainHelper class Line 243: StoragePoolIsoMap domainIsoMap = storageDomain.getStoragePoolIsoMapData(); Line 244: Line 245: if (domainIsoMap.getStatus() != null && domainIsoMap.getStatus().isStorageDomainInProcess()) { Line 246: return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL2, http://gerrit.ovirt.org/#/c/28624/9/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/ReconstructMasterDomainCommandTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/ReconstructMasterDomainCommandTest.java: Line 51: Line 52: private StoragePoolIsoMap regularDomainIsoMap; Line 53: Line 54: @Before Line 55: public void setUp() { i'd suggest to export the logic here to methods to ease the read here "initializeDomains" , "initializeStoragePoolValidator" and so on to shoretn the code here. Line 56: cmd = spy(new ReconstructMasterDomainCommand<>(new ReconstructMasterParameters())); Line 57: Line 58: masterDomainIsoMap = new StoragePoolIsoMap(masterStorageDomainId, storagePoolId, StorageDomainStatus.Active); Line 59: regularDomainIsoMap = new StoragePoolIsoMap(regularStorageDomainId, storagePoolId, StorageDomainStatus.Active); Line 71: Line 72: doReturn(masterStorageDomain).when(cmd).getStorageDomain(); Line 73: Line 74: doReturn(storagePool).when(cmd).getStoragePool(); Line 75: doReturn(storagePoolValidator).when(cmd).createStoragePoolValidator(); please move the validator related operations to be together Line 76: Line 77: } Line 78: Line 79: public void assertCanDoActionDomainInProcess(StorageDomainStatus masterStatus, StorageDomainStatus regularStatus, Line 75: doReturn(storagePoolValidator).when(cmd).createStoragePoolValidator(); Line 76: Line 77: } Line 78: Line 79: public void assertCanDoActionDomainInProcess(StorageDomainStatus masterStatus, StorageDomainStatus regularStatus, /s/assertCanDoActionDomainInProcess/validateCanDoActionDomainInProcess as we dont just assert here. Line 80: StorageDomainStatus expectedStatus) { Line 81: masterDomainIsoMap.setStatus(masterStatus); Line 82: regularDomainIsoMap.setStatus(regularStatus); Line 83: Line 105: @Test Line 106: public void testCanDoActionRegularLocked() { Line 107: assertCanDoActionDomainInProcess(StorageDomainStatus.Active, StorageDomainStatus.Locked, Line 108: StorageDomainStatus.Locked); Line 109: } can you add a test that verifies that the CDA checks pass? Line 110: Line 111: @Test Line 112: public void testCanDoActionRegularPreparingForMaintenance() { Line 113: assertCanDoActionDomainInProcess( StorageDomainStatus.Active, StorageDomainStatus.PreparingForMaintenance, -- To view, visit http://gerrit.ovirt.org/28624 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1b8c4727336c5df28f06566f7cb05c1871fcb000 Gerrit-PatchSet: 9 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@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
