From 521d4f24d02aef261ea50bc90546aab33666cc9a Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 18 Mar 2024 14:35:42 +0100 Subject: [PATCH] Remove flaky test in JMS observation There is no way to consistently test this use case because listener container implementations have different behavior for unhandled errors and might retry or close the container altogether. See gh-32458 --- ...sageListenerContainerObservationTests.java | 57 ++----------------- 1 file changed, 4 insertions(+), 53 deletions(-) diff --git a/spring-jms/src/test/java/org/springframework/jms/listener/MessageListenerContainerObservationTests.java b/spring-jms/src/test/java/org/springframework/jms/listener/MessageListenerContainerObservationTests.java index e6984f7be80..1e175d46532 100644 --- a/spring-jms/src/test/java/org/springframework/jms/listener/MessageListenerContainerObservationTests.java +++ b/spring-jms/src/test/java/org/springframework/jms/listener/MessageListenerContainerObservationTests.java @@ -21,9 +21,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Stream; -import io.micrometer.jakarta9.instrument.jms.JmsProcessObservationContext; import io.micrometer.observation.Observation; -import io.micrometer.observation.ObservationHandler; import io.micrometer.observation.tck.TestObservationRegistry; import jakarta.jms.MessageListener; import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory; @@ -65,8 +63,6 @@ class MessageListenerContainerObservationTests { @ParameterizedTest(name = "[{index}] {0}") @MethodSource("listenerContainers") void shouldRecordJmsProcessObservations(AbstractMessageListenerContainer listenerContainer) throws Exception { - JmsTemplate jmsTemplate = new JmsTemplate(connectionFactory); - jmsTemplate.convertAndSend("spring.test.observation", "message content"); CountDownLatch latch = new CountDownLatch(1); listenerContainer.setConnectionFactory(connectionFactory); listenerContainer.setObservationRegistry(registry); @@ -74,6 +70,8 @@ class MessageListenerContainerObservationTests { listenerContainer.setMessageListener((MessageListener) message -> latch.countDown()); listenerContainer.afterPropertiesSet(); listenerContainer.start(); + JmsTemplate jmsTemplate = new JmsTemplate(connectionFactory); + jmsTemplate.convertAndSend("spring.test.observation", "message content"); latch.await(2, TimeUnit.SECONDS); assertThat(registry).hasObservationWithNameEqualTo("jms.message.process") .that() @@ -86,8 +84,6 @@ class MessageListenerContainerObservationTests { @ParameterizedTest(name = "[{index}] {0}") @MethodSource("listenerContainers") void shouldHaveObservationScopeInErrorHandler(AbstractMessageListenerContainer listenerContainer) throws Exception { - JmsTemplate jmsTemplate = new JmsTemplate(connectionFactory); - jmsTemplate.convertAndSend("spring.test.observation", "message content"); CountDownLatch latch = new CountDownLatch(1); AtomicReference observationInErrorHandler = new AtomicReference<>(); listenerContainer.setConnectionFactory(connectionFactory); @@ -102,64 +98,19 @@ class MessageListenerContainerObservationTests { }); listenerContainer.afterPropertiesSet(); listenerContainer.start(); - latch.await(2, TimeUnit.SECONDS); - Assertions.assertThat(observationInErrorHandler.get()).isNotNull(); - assertThat(registry).hasObservationWithNameEqualTo("jms.message.process") - .that() - .hasHighCardinalityKeyValue("messaging.destination.name", "spring.test.observation") - .hasLowCardinalityKeyValue("exception", "none"); - assertThat(registry).hasNumberOfObservationsEqualTo(1); - listenerContainer.stop(); - listenerContainer.shutdown(); - } - - @ParameterizedTest(name = "[{index}] {0}") - @MethodSource("listenerContainers") - void shouldHaveObservationErrorWhenRethrown(AbstractMessageListenerContainer listenerContainer) throws Exception { JmsTemplate jmsTemplate = new JmsTemplate(connectionFactory); jmsTemplate.convertAndSend("spring.test.observation", "message content"); - CountDownLatch latch = new CountDownLatch(1); - registry.observationConfig().observationHandler(new ErrorHandlerObservationHandler(latch)); - listenerContainer.setConnectionFactory(connectionFactory); - listenerContainer.setObservationRegistry(registry); - listenerContainer.setDestinationName("spring.test.observation"); - listenerContainer.setMessageListener((MessageListener) message -> { - throw new IllegalStateException("error"); - }); - listenerContainer.setErrorHandler(error -> { - throw new IllegalStateException("not handled"); - }); - listenerContainer.afterPropertiesSet(); - listenerContainer.start(); latch.await(2, TimeUnit.SECONDS); + Assertions.assertThat(observationInErrorHandler.get()).isNotNull(); assertThat(registry).hasObservationWithNameEqualTo("jms.message.process") .that() .hasHighCardinalityKeyValue("messaging.destination.name", "spring.test.observation") - .hasLowCardinalityKeyValue("exception", "IllegalStateException"); + .hasLowCardinalityKeyValue("exception", "none"); assertThat(registry).hasNumberOfObservationsEqualTo(1); listenerContainer.stop(); listenerContainer.shutdown(); } - static class ErrorHandlerObservationHandler implements ObservationHandler { - - private final CountDownLatch latch; - - ErrorHandlerObservationHandler(CountDownLatch latch) { - this.latch = latch; - } - - @Override - public boolean supportsContext(Observation.Context context) { - return context instanceof JmsProcessObservationContext; - } - - @Override - public void onError(JmsProcessObservationContext context) { - this.latch.countDown(); - } - } - static Stream listenerContainers() { return Stream.of( arguments(named(DefaultMessageListenerContainer.class.getSimpleName(), new DefaultMessageListenerContainer())),