Make MBeanServer tests more robust

This commit refactors several tests to use SocketUtils to find an available port, compose a custom JMX service URL using that port, and start an MBeanServer for the particular test using that port. This commit also makes other changes to MBeanServer related tests in an effort to make them more robust when executed concurrently. Closes gh-23699
spring-projects · Oct 31, 2019 · 74b7b55 · 74b7b55
1 parent 57b4b74
commit 74b7b55
Show file tree

Hide file tree

Showing 12 changed files with 172 additions and 189 deletions.
diff --git a/spring-context/src/test/java/org/springframework/jmx/AbstractMBeanServerTests.java b/spring-context/src/test/java/org/springframework/jmx/AbstractMBeanServerTests.java
@@ -24,15 +24,16 @@
 
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.api.extension.LifecycleMethodExecutionExceptionHandler;
-import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.api.extension.TestExecutionExceptionHandler;
 import org.opentest4j.TestAbortedException;
 
 import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
 import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.support.GenericApplicationContext;
+import org.springframework.jmx.AbstractMBeanServerTests.BindExceptionHandler;
 import org.springframework.jmx.export.MBeanExporter;
 import org.springframework.util.MBeanTestUtils;
 
@@ -57,11 +58,9 @@
  * @author Chris Beams
  * @author Stephane Nicoll
  */
+@ExtendWith(BindExceptionHandler.class)
 public abstract class AbstractMBeanServerTests {
 
-	@RegisterExtension
-	BindExceptionHandler bindExceptionHandler = new BindExceptionHandler();
-
 	protected MBeanServer server;
 
 
@@ -77,32 +76,39 @@ public final void setUp() throws Exception {
 		}
 	}
 
-	protected ConfigurableApplicationContext loadContext(String configLocation) {
-		GenericApplicationContext ctx = new GenericApplicationContext();
-		new XmlBeanDefinitionReader(ctx).loadBeanDefinitions(configLocation);
-		ctx.getDefaultListableBeanFactory().registerSingleton("server", this.server);
-		ctx.refresh();
-		return ctx;
-	}
-
 	@AfterEach
 	public void tearDown() throws Exception {
 		releaseServer();
 		onTearDown();
 	}
 
 	private void releaseServer() throws Exception {
-		MBeanServerFactory.releaseMBeanServer(getServer());
+		try {
+			MBeanServerFactory.releaseMBeanServer(getServer());
+		}
+		catch (IllegalArgumentException ex) {
+			if (!ex.getMessage().contains("not in list")) {
+				throw ex;
+			}
+		}
 		MBeanTestUtils.resetMBeanServers();
 	}
 
-	protected void onTearDown() throws Exception {
+	protected final ConfigurableApplicationContext loadContext(String configLocation) {
+		GenericApplicationContext ctx = new GenericApplicationContext();
+		new XmlBeanDefinitionReader(ctx).loadBeanDefinitions(configLocation);
+		ctx.getDefaultListableBeanFactory().registerSingleton("server", getServer());
+		ctx.refresh();
+		return ctx;
 	}
 
 	protected void onSetUp() throws Exception {
 	}
 
-	public MBeanServer getServer() {
+	protected void onTearDown() throws Exception {
+	}
+
+	protected final MBeanServer getServer() {
 		return this.server;
 	}
 
@@ -123,7 +129,7 @@ protected void assertIsNotRegistered(String message, ObjectName objectName) {
 	}
 
 
-	private static class BindExceptionHandler implements TestExecutionExceptionHandler, LifecycleMethodExecutionExceptionHandler {
+	static class BindExceptionHandler implements TestExecutionExceptionHandler, LifecycleMethodExecutionExceptionHandler {
 
 		@Override
 		public void handleTestExecutionException(ExtensionContext context, Throwable throwable) throws Throwable {

diff --git a/spring-context/src/test/java/org/springframework/jmx/access/MBeanClientInterceptorTests.java b/spring-context/src/test/java/org/springframework/jmx/access/MBeanClientInterceptorTests.java
@@ -17,6 +17,9 @@
 package org.springframework.jmx.access;
 
 import java.beans.PropertyDescriptor;
+import java.lang.management.ManagementFactory;
+import java.lang.management.MemoryMXBean;
+import java.lang.management.ThreadMXBean;
 import java.lang.reflect.Method;
 import java.net.BindException;
 import java.util.HashMap;
@@ -30,6 +33,7 @@
 
 import org.junit.jupiter.api.Test;
 
+import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.jmx.AbstractMBeanServerTests;
 import org.springframework.jmx.IJmxTestBean;
 import org.springframework.jmx.JmxException;
@@ -50,7 +54,7 @@
  * @author Sam Brannen
  * @author Chris Beams
  */
-public class MBeanClientInterceptorTests extends AbstractMBeanServerTests {
+class MBeanClientInterceptorTests extends AbstractMBeanServerTests {
 
 	protected static final String OBJECT_NAME = "spring:test=proxy";
 
@@ -87,14 +91,14 @@ protected IJmxTestBean getProxy() throws Exception {
 	}
 
 	@Test
-	public void testProxyClassIsDifferent() throws Exception {
+	void testProxyClassIsDifferent() throws Exception {
 		assumeTrue(runTests);
 		IJmxTestBean proxy = getProxy();
-		assertThat((proxy.getClass() != IJmxTestBean.class)).as("The proxy class should be different than the base class").isTrue();
+		assertThat(proxy.getClass()).as("The proxy class should be different than the base class").isNotSameAs(IJmxTestBean.class);
 	}
 
 	@Test
-	public void testDifferentProxiesSameClass() throws Exception {
+	void testDifferentProxiesSameClass() throws Exception {
 		assumeTrue(runTests);
 		IJmxTestBean proxy1 = getProxy();
 		IJmxTestBean proxy2 = getProxy();
@@ -104,79 +108,79 @@ public void testDifferentProxiesSameClass() throws Exception {
 	}
 
 	@Test
-	public void testGetAttributeValue() throws Exception {
+	void testGetAttributeValue() throws Exception {
 		assumeTrue(runTests);
 		IJmxTestBean proxy1 = getProxy();
 		int age = proxy1.getAge();
 		assertThat(age).as("The age should be 100").isEqualTo(100);
 	}
 
 	@Test
-	public void testSetAttributeValue() throws Exception {
+	void testSetAttributeValue() throws Exception {
 		assumeTrue(runTests);
 		IJmxTestBean proxy = getProxy();
 		proxy.setName("Rob Harrop");
 		assertThat(target.getName()).as("The name of the bean should have been updated").isEqualTo("Rob Harrop");
 	}
 
 	@Test
-	public void testSetAttributeValueWithRuntimeException() throws Exception {
+	void testSetAttributeValueWithRuntimeException() throws Exception {
 		assumeTrue(runTests);
 		IJmxTestBean proxy = getProxy();
 		assertThatIllegalArgumentException().isThrownBy(() ->
 				proxy.setName("Juergen"));
 	}
 
 	@Test
-	public void testSetAttributeValueWithCheckedException() throws Exception {
+	void testSetAttributeValueWithCheckedException() throws Exception {
 		assumeTrue(runTests);
 		IJmxTestBean proxy = getProxy();
 		assertThatExceptionOfType(ClassNotFoundException.class).isThrownBy(() ->
 			proxy.setName("Juergen Class"));
 	}
 
 	@Test
-	public void testSetAttributeValueWithIOException() throws Exception {
+	void testSetAttributeValueWithIOException() throws Exception {
 		assumeTrue(runTests);
 		IJmxTestBean proxy = getProxy();
 		assertThatIOException().isThrownBy(() ->
 				proxy.setName("Juergen IO"));
 	}
 
 	@Test
-	public void testSetReadOnlyAttribute() throws Exception {
+	void testSetReadOnlyAttribute() throws Exception {
 		assumeTrue(runTests);
 		IJmxTestBean proxy = getProxy();
 		assertThatExceptionOfType(InvalidInvocationException.class).isThrownBy(() ->
 				proxy.setAge(900));
 	}
 
 	@Test
-	public void testInvokeNoArgs() throws Exception {
+	void testInvokeNoArgs() throws Exception {
 		assumeTrue(runTests);
 		IJmxTestBean proxy = getProxy();
 		long result = proxy.myOperation();
 		assertThat(result).as("The operation should return 1").isEqualTo(1);
 	}
 
 	@Test
-	public void testInvokeArgs() throws Exception {
+	void testInvokeArgs() throws Exception {
 		assumeTrue(runTests);
 		IJmxTestBean proxy = getProxy();
 		int result = proxy.add(1, 2);
 		assertThat(result).as("The operation should return 3").isEqualTo(3);
 	}
 
 	@Test
-	public void testInvokeUnexposedMethodWithException() throws Exception {
+	void testInvokeUnexposedMethodWithException() throws Exception {
 		assumeTrue(runTests);
 		IJmxTestBean bean = getProxy();
 		assertThatExceptionOfType(InvalidInvocationException.class).isThrownBy(() ->
 				bean.dontExposeMe());
 	}
 
 	@Test
-	public void testTestLazyConnectionToRemote() throws Exception {
+	void testTestLazyConnectionToRemote() throws Exception {
 		assumeTrue(runTests);
 
 		final int port = SocketUtils.findAvailableTcpPort();
@@ -219,46 +223,25 @@ public void testTestLazyConnectionToRemote() throws Exception {
 		catch (JmxException ex) {
 			// expected
 		}
-
-		connector = JMXConnectorServerFactory.newJMXConnectorServer(url, null, getServer());
-		connector.start();
-
-		// should now be able to access data via the lazy proxy
-		try {
-			assertThat(bean.getName()).isEqualTo("Rob Harrop");
-			assertThat(bean.getAge()).isEqualTo(100);
-		}
-		finally {
-			connector.stop();
-		}
 	}
 
-	/*
 	public void testMXBeanAttributeAccess() throws Exception {
 		MBeanClientInterceptor interceptor = new MBeanClientInterceptor();
 		interceptor.setServer(ManagementFactory.getPlatformMBeanServer());
 		interceptor.setObjectName("java.lang:type=Memory");
 		interceptor.setManagementInterface(MemoryMXBean.class);
 		MemoryMXBean proxy = ProxyFactory.getProxy(MemoryMXBean.class, interceptor);
-		assertTrue(proxy.getHeapMemoryUsage().getMax() > 0);
+		assertThat(proxy.getHeapMemoryUsage().getMax()).isGreaterThan(0);
 	}
 
 	public void testMXBeanOperationAccess() throws Exception {
 		MBeanClientInterceptor interceptor = new MBeanClientInterceptor();
 		interceptor.setServer(ManagementFactory.getPlatformMBeanServer());
 		interceptor.setObjectName("java.lang:type=Threading");
 		ThreadMXBean proxy = ProxyFactory.getProxy(ThreadMXBean.class, interceptor);
-		assertTrue(proxy.getThreadInfo(Thread.currentThread().getId()).getStackTrace() != null);
+		assertThat(proxy.getThreadInfo(Thread.currentThread().getId()).getStackTrace()).isNotNull();
 	}
 
-	public void testMXBeanAttributeListAccess() throws Exception {
-		MBeanClientInterceptor interceptor = new MBeanClientInterceptor();
-		interceptor.setServer(ManagementFactory.getPlatformMBeanServer());
-		interceptor.setObjectName("com.sun.management:type=HotSpotDiagnostic");
-		HotSpotDiagnosticMXBean proxy = ProxyFactory.getProxy(HotSpotDiagnosticMXBean.class, interceptor);
-		assertFalse(proxy.getDiagnosticOptions().isEmpty());
-	}
-	*/
 
 	private static class ProxyTestAssembler extends AbstractReflectiveMBeanInfoAssembler {
 

diff --git a/...ntext/src/test/java/org/springframework/jmx/access/RemoteMBeanClientInterceptorTests.java b/...ntext/src/test/java/org/springframework/jmx/access/RemoteMBeanClientInterceptorTests.java
@@ -35,16 +35,11 @@
  * @author Chris Beams
  * @author Sam Brannen
  */
-public class RemoteMBeanClientInterceptorTests extends MBeanClientInterceptorTests {
+class RemoteMBeanClientInterceptorTests extends MBeanClientInterceptorTests {
 
-	private static final int SERVICE_PORT;
+	private final int servicePort = SocketUtils.findAvailableTcpPort();
 
-	private static final String SERVICE_URL;
-
-	static {
-		SERVICE_PORT = SocketUtils.findAvailableTcpPort();
-		SERVICE_URL = "service:jmx:jmxmp://localhost:" + SERVICE_PORT;
-	}
+	private final String serviceUrl = "service:jmx:jmxmp://localhost:" + servicePort;
 
 
 	private JMXConnectorServer connectorServer;
@@ -61,13 +56,13 @@ public void onSetUp() throws Exception {
 		}
 		catch (BindException ex) {
 			System.out.println("Skipping remote JMX tests because binding to local port ["
-					+ SERVICE_PORT + "] failed: " + ex.getMessage());
+					+ this.servicePort + "] failed: " + ex.getMessage());
 			runTests = false;
 		}
 	}
 
 	private JMXServiceURL getServiceUrl() throws MalformedURLException {
-		return new JMXServiceURL(SERVICE_URL);
+		return new JMXServiceURL(this.serviceUrl);
 	}
 
 	@Override

diff --git a/spring-context/src/test/java/org/springframework/jmx/export/CustomEditorConfigurerTests.java b/spring-context/src/test/java/org/springframework/jmx/export/CustomEditorConfigurerTests.java
@@ -31,7 +31,7 @@
 /**
  * @author Rob Harrop
  */
-public class CustomEditorConfigurerTests extends AbstractJmxTests {
+class CustomEditorConfigurerTests extends AbstractJmxTests {
 
 	private final SimpleDateFormat df = new SimpleDateFormat("yyyy/MM/dd");
 
@@ -41,33 +41,30 @@ protected String getApplicationContextPath() {
 	}
 
 	@Test
-	public void testDatesInJmx() throws Exception {
-		// System.out.println(getServer().getClass().getName());
-		ObjectName oname = new ObjectName("bean:name=dateRange");
-
-		Date startJmx = (Date) getServer().getAttribute(oname, "StartDate");
-		Date endJmx = (Date) getServer().getAttribute(oname, "EndDate");
+	void datesInApplicationContext() throws Exception {
+		DateRange dr = getContext().getBean("dateRange", DateRange.class);
 
-		assertThat(startJmx).as("startDate ").isEqualTo(getStartDate());
-		assertThat(endJmx).as("endDate ").isEqualTo(getEndDate());
+		assertThat(dr.getStartDate()).as("startDate").isEqualTo(getStartDate());
+		assertThat(dr.getEndDate()).as("endDate").isEqualTo(getEndDate());
 	}
 
 	@Test
-	public void testGetDates() throws Exception {
-		DateRange dr = (DateRange) getContext().getBean("dateRange");
+	void datesInJmx() throws Exception {
+		ObjectName oname = new ObjectName("bean:name=dateRange");
+
+		Date startJmx = (Date) getServer().getAttribute(oname, "StartDate");
+		Date endJmx = (Date) getServer().getAttribute(oname, "EndDate");
 
-		assertThat(dr.getStartDate()).as("startDate ").isEqualTo(getStartDate());
-		assertThat(dr.getEndDate()).as("endDate ").isEqualTo(getEndDate());
+		assertThat(startJmx).as("startDate").isEqualTo(getStartDate());
+		assertThat(endJmx).as("endDate").isEqualTo(getEndDate());
 	}
 
 	private Date getStartDate() throws ParseException {
-		Date start = df.parse("2004/10/12");
-		return start;
+		return df.parse("2004/10/12");
 	}
 
 	private Date getEndDate() throws ParseException {
-		Date end = df.parse("2004/11/13");
-		return end;
+		return df.parse("2004/11/13");
 	}
 
 }