Tal Nisan has posted comments on this change. Change subject: core: Add tests to StorageHandlingCommandBaseTest ......................................................................
Patch Set 4: (6 comments) http://gerrit.ovirt.org/#/c/23522/4/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBaseTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBaseTest.java: Line 70: ); Line 71: Line 72: @Before Line 73: public void setUp() { Line 74: storagePool = createStoragePool(new Version(3, 4)); > use Version.v3_4 Done Line 75: Line 76: initCommand(); Line 77: Line 78: when(facade.getStoragePoolDao()).thenReturn(storagePoolDAO); Line 77: Line 78: when(facade.getStoragePoolDao()).thenReturn(storagePoolDAO); Line 79: when(facade.getStoragePoolDao().get(storagePool.getId())).thenReturn(storagePool); Line 80: Line 81: when(facade.getStorageDomainDao()).thenReturn(storageDomainDAO); > why do you need the dbfacade here? Can't you use the command directly? This convention was used here before so I continued the same way Line 82: when(storageDomainDAO.getAllForStoragePool(storagePool.getId())).thenReturn(attachedDomains); Line 83: Line 84: when(facade.getStoragePoolIsoMapDao()).thenReturn(storagePoolIsoMapDAO); Line 85: when(storagePoolIsoMapDAO.getAllForStorage(any(Guid.class))).thenReturn(isoMap); Line 133: } Line 134: Line 135: @Test Line 136: public void testMixedTypesUnsupported() { Line 137: storagePool.setcompatibility_version(Version.v3_0); // Mixed types are not allowed on V3.0 > This comment should be in the javadoc, not inline. Done Line 138: Line 139: StorageDomain existingStorageDomain = createValidStorageDomain(); Line 140: existingStorageDomain.setStorageType(StorageType.NFS); Line 141: addDomainToPool(existingStorageDomain); Line 148: domainToAttach.setStorageType(StorageType.ISCSI); Line 149: initCommand(); Line 150: assertFalse("Attaching an ISCSI domain to a pool with NFS domain with no mixed type allowed succeeded", cmd.checkDomainCanBeAttached(domainToAttach)); Line 151: CanDoActionTestUtils.assertCanDoActionMessages("Attaching an ISCSI domain to a pool with NFS domain with no mixed type failed with the wrong message", cmd, Line 152: VdcBllMessages.ACTION_TYPE_FAILED_MIXED_STORAGE_TYPES_NOT_ALLOWED); > I'd have this block in a separate test I see both tests as a part of the same general test hence I put them together Line 153: Line 154: } Line 155: Line 156: Line 177: initCommand(); Line 178: storageDomain.setStorageType(StorageType.GLUSTERFS); Line 179: assertFalse("Attaching a GLUSTER domain succeeded while it should have failed", cmd.checkDomainCanBeAttached(storageDomain)); Line 180: CanDoActionTestUtils.assertCanDoActionMessages("Attaching a GLUSTER domain failed failed with the wrong message", cmd, Line 181: VdcBllMessages.DATA_CENTER_GLUSTER_STORAGE_NOT_SUPPORTED_IN_CURRENT_VERSION); > here too - I'd put each scenario in a separate test case. Done Line 182: } Line 183: Line 184: @Test Line 185: public void testAttachFailDomainTypeIncorrect() { Line 219: } Line 220: Line 221: @Test Line 222: public void testCanAttachISOOrExport() { Line 223: for (StorageDomainType type : Arrays.<StorageDomainType>asList(StorageDomainType.ISO, StorageDomainType.ImportExport)) { > I think you need to apply the formatter here, but nice idea! Done Line 224: StorageDomain storageDomainToAttach = createValidStorageDomain(); Line 225: storageDomainToAttach.setStorageDomainType(type); Line 226: testCanAttachWithISOOrExport(storageDomainToAttach); Line 227: } -- To view, visit http://gerrit.ovirt.org/23522 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I97e45957f7ff0fe4c9e7d88e0d8dbab7e874c79b Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tal Nisan <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Tal Nisan <[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
