From 6d4fad7a4c04f1ba96eb3d4fb3d55f6d86787780 Mon Sep 17 00:00:00 2001 From: Robbie Gemmell Date: Fri, 25 Aug 2023 13:08:40 +0100 Subject: [PATCH] ARTEMIS-4391: fix break in findText from prior changes Only report finding matching log message if all requested entries are present in it, not just the last one provided. Also fix the updated AssertionLoggerHandler usage within AddressFullLoggingTest, ensure it is active across the full period expected messages can happen and doesnt miss early ones. --- artemis-unit-test-support/pom.xml | 6 + .../artemis/logs/AssertionLoggerHandler.java | 6 +- .../logs/AssertionLoggerHandlerTest.java | 130 ++++++++++++++++++ .../paging/AddressFullLoggingTest.java | 55 ++++---- 4 files changed, 167 insertions(+), 30 deletions(-) create mode 100644 artemis-unit-test-support/src/test/java/org/apache/activemq/artemis/logs/AssertionLoggerHandlerTest.java diff --git a/artemis-unit-test-support/pom.xml b/artemis-unit-test-support/pom.xml index 0338085d92..19b36ad6e0 100644 --- a/artemis-unit-test-support/pom.xml +++ b/artemis-unit-test-support/pom.xml @@ -51,5 +51,11 @@ log4j-core provided + + + org.apache.logging.log4j + log4j-slf4j-impl + test + diff --git a/artemis-unit-test-support/src/main/java/org/apache/activemq/artemis/logs/AssertionLoggerHandler.java b/artemis-unit-test-support/src/main/java/org/apache/activemq/artemis/logs/AssertionLoggerHandler.java index ae047797df..deb97f65ec 100644 --- a/artemis-unit-test-support/src/main/java/org/apache/activemq/artemis/logs/AssertionLoggerHandler.java +++ b/artemis-unit-test-support/src/main/java/org/apache/activemq/artemis/logs/AssertionLoggerHandler.java @@ -112,12 +112,12 @@ public class AssertionLoggerHandler extends AbstractAppender implements Closeabl */ public boolean findText(final String... text) { for (LogEntry logEntry : messages) { - boolean found = true; + boolean found = false; for (String txtCheck : text) { found = logEntry.message.contains(txtCheck); - if (found) { - continue; + if (!found) { + break; } } diff --git a/artemis-unit-test-support/src/test/java/org/apache/activemq/artemis/logs/AssertionLoggerHandlerTest.java b/artemis-unit-test-support/src/test/java/org/apache/activemq/artemis/logs/AssertionLoggerHandlerTest.java new file mode 100644 index 0000000000..83e4251196 --- /dev/null +++ b/artemis-unit-test-support/src/test/java/org/apache/activemq/artemis/logs/AssertionLoggerHandlerTest.java @@ -0,0 +1,130 @@ +/* + * 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.activemq.artemis.logs; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.lang.invoke.MethodHandles; + +import org.apache.activemq.artemis.logs.AssertionLoggerHandler.LogLevel; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class AssertionLoggerHandlerTest { + + private static final String LOGGER_NAME = MethodHandles.lookup().lookupClass().getName(); + private static final Logger logger = LoggerFactory.getLogger(LOGGER_NAME); + + private static LogLevel origLevel; + + @BeforeClass + public static void setLogLevel() { + origLevel = AssertionLoggerHandler.setLevel(LOGGER_NAME, LogLevel.INFO); + } + + @AfterClass + public static void restoreLogLevel() throws Exception { + AssertionLoggerHandler.setLevel(LOGGER_NAME, origLevel); + } + + @Test + public void testFindText() throws Exception { + final String prefix = "123prefix"; + final String middle = "middle456"; + final String suffix = "suffix789"; + + try (AssertionLoggerHandler loggerHandler = new AssertionLoggerHandler(true)) { + // Try without logging anything + assertFalse("should not have found prefix, not yet logged", loggerHandler.findText(prefix)); + assertFalse("should not have found middle, not yet logged", loggerHandler.findText(middle)); + assertFalse("should not have found suffix, not yet logged", loggerHandler.findText(suffix)); + assertFalse("should not have found combination, not yet logged", loggerHandler.findText(prefix, middle, suffix)); + + // Now log only the prefix. + logger.info("{} -after", prefix); + + assertTrue("should have found prefix logged", loggerHandler.findText(prefix)); + assertFalse("should not have found middle, not yet logged", loggerHandler.findText(middle)); + assertFalse("should not have found suffix, not yet logged", loggerHandler.findText(suffix)); + assertFalse("should not have found combination, not yet logged", loggerHandler.findText(prefix, middle, suffix)); + + // Now log only the middle. + logger.info("before- {} -after", middle); + + assertTrue("should have found prefix logged", loggerHandler.findText(prefix)); + assertTrue("should have found middle logged", loggerHandler.findText(middle)); + assertFalse("should not have found suffix, not yet logged", loggerHandler.findText(suffix)); + assertFalse("should not have found full combination, not yet logged", loggerHandler.findText(prefix, middle, suffix)); + + // Now log only the suffix. + logger.info("before- {}", suffix); + + assertTrue("should have found prefix logged", loggerHandler.findText(prefix)); + assertTrue("should have found middle logged", loggerHandler.findText(middle)); + assertTrue("should have found suffix logged", loggerHandler.findText(suffix)); + assertFalse("should not have found alternative combination, not yet logged", loggerHandler.findText(prefix, suffix)); + assertFalse("should not have found alternative combination, not yet logged", loggerHandler.findText(prefix, middle)); + assertFalse("should not have found alternative combination, not yet logged", loggerHandler.findText(middle, suffix)); + assertFalse("should not have found full combination, not yet logged", loggerHandler.findText(prefix, middle, suffix)); + } + + // Use a new AssertionLoggerHandler to start fresh + try (AssertionLoggerHandler loggerHandler = new AssertionLoggerHandler(true)) { + // Try again without logging anything + assertFalse("should not have found prefix, not yet logged", loggerHandler.findText(prefix)); + assertFalse("should not have found middle, not yet logged", loggerHandler.findText(middle)); + assertFalse("should not have found suffix, not yet logged", loggerHandler.findText(suffix)); + assertFalse("should not have found full combination, not yet logged", loggerHandler.findText(prefix, middle, suffix)); + + // Now log the prefix AND suffix, but NOT middle. + logger.info("{} -inbetween- {}", prefix, suffix); + + assertTrue("should have found prefix logged", loggerHandler.findText(prefix)); + assertFalse("should not have found middle, not yet logged", loggerHandler.findText(middle)); + assertTrue("should have found suffix logged", loggerHandler.findText(suffix)); + assertTrue("should have found combination logged", loggerHandler.findText(prefix, suffix)); + assertFalse("should not have found alternative combination, not yet logged", loggerHandler.findText(prefix, middle)); + assertFalse("should not have found alternative combination, not yet logged", loggerHandler.findText(middle, suffix)); + assertFalse("should not have found full combination, not yet logged", loggerHandler.findText(prefix, middle, suffix)); + } + + // Use a new AssertionLoggerHandler to start fresh + try (AssertionLoggerHandler loggerHandler = new AssertionLoggerHandler(true)) { + // Try again without logging anything + assertFalse("should not have found prefix, not yet logged", loggerHandler.findText(prefix)); + assertFalse("should not have found middle, not yet logged", loggerHandler.findText(middle)); + assertFalse("should not have found suffix, not yet logged", loggerHandler.findText(suffix)); + assertFalse("should not have found combination, not yet logged", loggerHandler.findText(prefix, middle, suffix)); + + // Now log the prefix AND middle AND suffix. + logger.info("{} - {} - {}", prefix, middle, suffix); + + assertTrue("should have found prefix logged", loggerHandler.findText(prefix)); + assertTrue("should have found middle logged", loggerHandler.findText(middle)); + assertTrue("should have found suffix logged", loggerHandler.findText(suffix)); + assertTrue("should have found combination logged", loggerHandler.findText(prefix, suffix)); + assertTrue("should have found combination logged", loggerHandler.findText(prefix, middle)); + assertTrue("should have found combination logged", loggerHandler.findText(middle, suffix)); + assertTrue("should have found full combination logged", loggerHandler.findText(prefix, middle, suffix)); + } + } + +} diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/AddressFullLoggingTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/AddressFullLoggingTest.java index 12f7d47b1e..72151bbb27 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/AddressFullLoggingTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/AddressFullLoggingTest.java @@ -118,34 +118,35 @@ public class AddressFullLoggingTest extends ActiveMQTestBase { } }; - int sendCount = 0; - - for (int i = 0; i < MAX_MESSAGES; i++) { - Future future = executor.submit(sendMessageTask); - try { - future.get(3, TimeUnit.SECONDS); - sendCount++; - } catch (TimeoutException ex) { - // message sending has been blocked - break; - } finally { - future.cancel(true); // may or may not desire this - } - } - - executor.shutdown(); - session.close(); - session = factory.createSession(false, true, true); - session.start(); - ClientConsumer consumer = session.createConsumer(MY_QUEUE); - for (int i = 0; i < sendCount; i++) { - ClientMessage msg = consumer.receive(250); - if (msg == null) - break; - msg.acknowledge(); - } try (AssertionLoggerHandler loggerHandler = new AssertionLoggerHandler()) { - //this is needed to allow to kick-in at least once disk scan + int sendCount = 0; + + for (int i = 0; i < MAX_MESSAGES; i++) { + Future future = executor.submit(sendMessageTask); + try { + future.get(3, TimeUnit.SECONDS); + sendCount++; + } catch (TimeoutException ex) { + // message sending has been blocked + break; + } finally { + future.cancel(true); // may or may not desire this + } + } + + executor.shutdown(); + session.close(); + session = factory.createSession(false, true, true); + session.start(); + ClientConsumer consumer = session.createConsumer(MY_QUEUE); + for (int i = 0; i < sendCount; i++) { + ClientMessage msg = consumer.receive(250); + if (msg == null) + break; + msg.acknowledge(); + } + + //this is needed to allow to kick-in at least one disk scan TimeUnit.MILLISECONDS.sleep(server.getConfiguration().getDiskScanPeriod() * 2); session.close(); locator.close();