Martin Peřina has posted comments on this change. Change subject: Engine: Add unit-tests for fence execution ......................................................................
Patch Set 1: (9 comments) http://gerrit.ovirt.org/#/c/36494/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/FenceExecutor.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/FenceExecutor.java: Line 96: public VDSFenceReturnValue fence(FenceActionType action, FenceAgent agent, VDS proxyHost) { Line 97: VDSReturnValue result = null; Line 98: try { Line 99: if (action == FenceActionType.Restart || action == FenceActionType.Stop) { Line 100: stopSPM(action); We should move SPM stop command outside FenceExecutor in different patch, this logic doesn't belong into FenceExecutor class. Line 101: } Line 102: result = runFenceAction(action, agent, proxyHost); Line 103: // if fence failed, retry with another proxy. Line 104: if (!result.getSucceeded()) { http://gerrit.ovirt.org/#/c/36494/1/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/FenceExecutorTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/FenceExecutorTest.java: Please add comments for each test methods, it's not always clear what kind of test is executed in each method. Also please don't use underscore char in method name. Underscore chars in Java should be used only in upper case constant names. For example: public void successfulFence() Line 1: package org.ovirt.engine.core.bll; Line 2: Line 3: import java.util.LinkedList; Line 4: import java.util.List; Line 64: @Mock Line 65: private DbFacade dbFacade; Line 66: Line 67: @Mock Line 68: private VDS vds; Please rename this to fencedVds or nonrespondingVds so it cannot be confused with VDS vds used in each test method Line 69: Line 70: @Mock Line 71: private VdsDAO vdsDao; Line 72: Line 183: fence_handleSuccess I would prefer public void successfulFence() Line 189: assertEquals(result.getFenceAgentUsed(), agent); Line 190: } Line 191: Line 192: @Test Line 193: public void fence_stopSpmCalled() { I would prefer public void successfullSpmFence() Line 194: mockProxyHost(); Line 195: mockFenceSuccess(); Line 196: FenceAgent agent = createAgent(); Line 197: VDSFenceReturnValue result = executor.fence(FenceActionType.Stop, agent); Line 201: } Line 202: Line 203: Line 204: @Test Line 205: public void fence_retryWithDifferentProxyAndSucceed() { I would prefer public void successfulFenceWithDifferentProxyRetry() Line 206: mockProxyHost(true); Line 207: mockFenceFailure(true); Line 208: FenceAgent agent = createAgent(); Line 209: VDSFenceReturnValue result = executor.fence(FenceActionType.Start, agent); Line 211: verify(proxyLocator).findProxyHost(true, PROXY_HOST_ID); Line 212: } Line 213: Line 214: @Test Line 215: public void fence_retryWithSameProxyAndSucceed() { I would prefer public void successfulFenceWithSameProxyRetry() Line 216: mockProxyHost(false); Line 217: mockFenceFailure(true); Line 218: FenceAgent agent = createAgent(); Line 219: VDSFenceReturnValue result = executor.fence(FenceActionType.Start, agent); Line 221: verify(proxyLocator).findProxyHost(true, PROXY_HOST_ID); Line 222: } Line 223: Line 224: @Test Line 225: public void fence_retryWithDifferentProxyAndFail() { I would prefer public void failedFenceWithDifferentProxyRetry() Line 226: mockProxyHost(true); Line 227: mockFenceFailure(false); Line 228: FenceAgent agent = createAgent(); Line 229: VDSFenceReturnValue result = executor.fence(FenceActionType.Start, agent); Line 231: verify(proxyLocator).findProxyHost(true, PROXY_HOST_ID); Line 232: } Line 233: Line 234: @Test Line 235: public void fence_retryWithSameProxyAndFail() { I would prefer public void failedFenceWithSameProxyRetry() Line 236: mockProxyHost(false); Line 237: mockFenceFailure(false); Line 238: FenceAgent agent = createAgent(); Line 239: VDSFenceReturnValue result = executor.fence(FenceActionType.Start, agent); -- To view, visit http://gerrit.ovirt.org/36494 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I311933f0e3d23489c44faa6dced4373f7254b56e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ori Liel <ol...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@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
