Author: todd Date: Wed Mar 28 04:52:51 2012 New Revision: 1306160 URL: http://svn.apache.org/viewvc?rev=1306160&view=rev Log: HADOOP-8218. RPC.closeProxy shouldn't throw error when closing a mock. Contributed by Todd Lipcon.
Added: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/MockitoUtil.java Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/DummyHAService.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestFailoverController.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1306160&r1=1306159&r2=1306160&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt Wed Mar 28 04:52:51 2012 @@ -184,6 +184,9 @@ Release 0.23.3 - UNRELEASED HADOOP-8202. RPC stopProxy() does not close the proxy correctly. (Hari Mankude via suresh) + HADOOP-8218. RPC.closeProxy shouldn't throw error when closing a mock + (todd) + BREAKDOWN OF HADOOP-7454 SUBTASKS HADOOP-7455. HA: Introduce HA Service Protocol Interface. (suresh) Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java?rev=1306160&r1=1306159&r2=1306160&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HealthMonitor.java Wed Mar 28 04:52:51 2012 @@ -17,7 +17,6 @@ */ package org.apache.hadoop.ha; -import java.io.Closeable; import java.io.IOException; import java.util.Collections; import java.util.LinkedList; @@ -195,9 +194,7 @@ class HealthMonitor { } catch (Throwable t) { LOG.warn("Transport-level exception trying to monitor health of " + targetToMonitor + ": " + t.getLocalizedMessage()); - if (proxy instanceof Closeable) { - RPC.stopProxy(proxy); - } + RPC.stopProxy(proxy); proxy = null; enterState(State.SERVICE_NOT_RESPONDING); Thread.sleep(sleepAfterDisconnectMillis); Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java?rev=1306160&r1=1306159&r2=1306160&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java Wed Mar 28 04:52:51 2012 @@ -604,6 +604,9 @@ public class RPC { LOG.error("RPC.stopProxy called on non proxy.", e); } + // If you see this error on a mock object in a unit test you're + // developing, make sure to use MockitoUtil.mockProtocol() to + // create your mock. throw new HadoopIllegalArgumentException( "Cannot close proxy - is not Closeable or " + "does not provide closeable invocation handler " Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/DummyHAService.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/DummyHAService.java?rev=1306160&r1=1306159&r2=1306160&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/DummyHAService.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/DummyHAService.java Wed Mar 28 04:52:51 2012 @@ -17,6 +17,7 @@ */ package org.apache.hadoop.ha; +import java.io.Closeable; import java.io.IOException; import java.net.InetSocketAddress; import java.util.ArrayList; @@ -56,50 +57,7 @@ class DummyHAService extends HAServiceTa } private HAServiceProtocol makeMock() { - return Mockito.spy(new HAServiceProtocol() { - @Override - public void monitorHealth() throws HealthCheckFailedException, - AccessControlException, IOException { - checkUnreachable(); - if (!isHealthy) { - throw new HealthCheckFailedException("not healthy"); - } - } - - @Override - public void transitionToActive() throws ServiceFailedException, - AccessControlException, IOException { - checkUnreachable(); - if (failToBecomeActive) { - throw new ServiceFailedException("injected failure"); - } - - state = HAServiceState.ACTIVE; - } - - @Override - public void transitionToStandby() throws ServiceFailedException, - AccessControlException, IOException { - checkUnreachable(); - state = HAServiceState.STANDBY; - } - - @Override - public HAServiceStatus getServiceStatus() throws IOException { - checkUnreachable(); - HAServiceStatus ret = new HAServiceStatus(state); - if (state == HAServiceState.STANDBY) { - ret.setReadyToBecomeActive(); - } - return ret; - } - - private void checkUnreachable() throws IOException { - if (actUnreachable) { - throw new IOException("Connection refused (fake)"); - } - } - }); + return Mockito.spy(new MockHAProtocolImpl()); } @Override @@ -130,4 +88,54 @@ class DummyHAService extends HAServiceTa public static HAServiceTarget getInstance(int serial) { return instances.get(serial - 1); } + + private class MockHAProtocolImpl implements + HAServiceProtocol, Closeable { + @Override + public void monitorHealth() throws HealthCheckFailedException, + AccessControlException, IOException { + checkUnreachable(); + if (!isHealthy) { + throw new HealthCheckFailedException("not healthy"); + } + } + + @Override + public void transitionToActive() throws ServiceFailedException, + AccessControlException, IOException { + checkUnreachable(); + if (failToBecomeActive) { + throw new ServiceFailedException("injected failure"); + } + + state = HAServiceState.ACTIVE; + } + + @Override + public void transitionToStandby() throws ServiceFailedException, + AccessControlException, IOException { + checkUnreachable(); + state = HAServiceState.STANDBY; + } + + @Override + public HAServiceStatus getServiceStatus() throws IOException { + checkUnreachable(); + HAServiceStatus ret = new HAServiceStatus(state); + if (state == HAServiceState.STANDBY) { + ret.setReadyToBecomeActive(); + } + return ret; + } + + private void checkUnreachable() throws IOException { + if (actUnreachable) { + throw new IOException("Connection refused (fake)"); + } + } + + @Override + public void close() throws IOException { + } + } } Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestFailoverController.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestFailoverController.java?rev=1306160&r1=1306159&r2=1306160&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestFailoverController.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestFailoverController.java Wed Mar 28 04:52:51 2012 @@ -17,6 +17,7 @@ */ package org.apache.hadoop.ha; +import java.io.Closeable; import java.io.IOException; import java.net.InetSocketAddress; @@ -33,7 +34,6 @@ import org.apache.hadoop.security.Access import org.junit.Test; import org.mockito.Mockito; import org.mockito.internal.stubbing.answers.ThrowsException; -import org.mockito.stubbing.Answer; import static org.junit.Assert.*; @@ -228,7 +228,10 @@ public class TestFailoverController { // Getting a proxy to a dead server will throw IOException on call, // not on creation of the proxy. HAServiceProtocol errorThrowingProxy = Mockito.mock(HAServiceProtocol.class, - new ThrowsException(new IOException("Could not connect to host"))); + Mockito.withSettings() + .defaultAnswer(new ThrowsException( + new IOException("Could not connect to host"))) + .extraInterfaces(Closeable.class)); Mockito.doReturn(errorThrowingProxy).when(svc1).getProxy(); DummyHAService svc2 = new DummyHAService(HAServiceState.STANDBY, svc2Addr); svc1.fencer = svc2.fencer = setupFencer(AlwaysSucceedFencer.class.getName()); Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java?rev=1306160&r1=1306159&r2=1306160&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java Wed Mar 28 04:52:51 2012 @@ -50,6 +50,7 @@ import org.apache.hadoop.security.token. import org.apache.hadoop.security.token.TokenIdentifier; import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.security.UserGroupInformation; +import org.apache.hadoop.test.MockitoUtil; import org.junit.Test; import static org.junit.Assert.*; @@ -58,8 +59,6 @@ import com.google.protobuf.DescriptorPro import static org.apache.hadoop.test.MetricsAsserts.*; -import static org.mockito.Mockito.*; - /** Unit tests for RPC. */ @SuppressWarnings("deprecation") public class TestRPC { @@ -587,9 +586,17 @@ public class TestRPC { */ @Test(expected=HadoopIllegalArgumentException.class) public void testStopNonRegisteredProxy() throws Exception { - RPC.stopProxy(mock(TestProtocol.class)); RPC.stopProxy(null); } + + /** + * Test that the mockProtocol helper returns mock proxies that can + * be stopped without error. + */ + @Test + public void testStopMockObject() throws Exception { + RPC.stopProxy(MockitoUtil.mockProtocol(TestProtocol.class)); + } @Test public void testStopProxy() throws IOException { Added: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/MockitoUtil.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/MockitoUtil.java?rev=1306160&view=auto ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/MockitoUtil.java (added) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/MockitoUtil.java Wed Mar 28 04:52:51 2012 @@ -0,0 +1,36 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.test; + +import java.io.Closeable; + +import org.mockito.Mockito; + +public abstract class MockitoUtil { + + /** + * Return a mock object for an IPC protocol. This special + * method is necessary, since the IPC proxies have to implement + * Closeable in addition to their protocol interface. + * @param clazz the protocol class + */ + public static <T> T mockProtocol(Class<T> clazz) { + return Mockito.mock(clazz, + Mockito.withSettings().extraInterfaces(Closeable.class)); + } +}