Kanagaraj M has posted comments on this change. Change subject: engine : Bll Command to Create Geo Rep session ......................................................................
Patch Set 10: (4 comments) http://gerrit.ovirt.org/#/c/29834/10/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterVolumeGeoRepSessionCreateCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterVolumeGeoRepSessionCreateCommand.java: Line 37: @Override Line 38: protected boolean canDoAction() { Line 39: parameter = getParameters(); Line 40: if (parameter == null || parameter.getVolumeId() == null || parameter.getSlaveNodeIp() == null || parameter.getSlaveVolumeName() == null) { Line 41: return false; you might want to return a specific validation error. Something like, "Slave host name is required" "Slave volumes is required" Line 42: } Line 43: if(masterVolume == null) { Line 44: return false; Line 45: } Line 39: parameter = getParameters(); Line 40: if (parameter == null || parameter.getVolumeId() == null || parameter.getSlaveNodeIp() == null || parameter.getSlaveVolumeName() == null) { Line 41: return false; Line 42: } Line 43: if(masterVolume == null) { where is it getting set? Line 44: return false; Line 45: } Line 46: masterVolume = getGlusterVolumeDao().getById(parameter.getVolumeId()); Line 47: if (masterVolume.getStatus() == GlusterStatus.DOWN) { Line 43: if(masterVolume == null) { Line 44: return false; Line 45: } Line 46: masterVolume = getGlusterVolumeDao().getById(parameter.getVolumeId()); Line 47: if (masterVolume.getStatus() == GlusterStatus.DOWN) { Same here Line 48: return false; Line 49: } Line 50: return true; Line 51: } http://gerrit.ovirt.org/#/c/29834/10/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterVolumeGeoRepSessionCreateCommandTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterVolumeGeoRepSessionCreateCommandTest.java: Line 61: public void canDoActionFails() { Line 62: GlusterVolumeGeoRepSessionCreateCommand command = spy(getCommand(null, slaveNodeIp, slaveVolumeName, slaveRootPassword, true, null)); Line 63: prepareMock(command); Line 64: assertFalse(command.canDoAction()); Line 65: } Add testcases for - When volume doesn't exists - Volume is down -- To view, visit http://gerrit.ovirt.org/29834 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccc92235deea42d7818336b2402476193cbe789c Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: anmolbabu <[email protected]> Gerrit-Reviewer: Kanagaraj M <[email protected]> Gerrit-Reviewer: Ramesh N <[email protected]> Gerrit-Reviewer: Sahina Bose <[email protected]> Gerrit-Reviewer: Shubhendu Tripathi <[email protected]> Gerrit-Reviewer: anmolbabu <[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
