From 5c160ae1a8d4148fef84f7c31517bf18736b3b88 Mon Sep 17 00:00:00 2001 From: Justin Bertram Date: Wed, 17 Jul 2024 14:27:47 -0500 Subject: [PATCH] ARTEMIS-4930 refactor storage manager tests During the investigation for ARTEMIS-4910 it was noted that some of the storage manager tests didn't actually force a full reload (i.e. decoding) of the various configurations from disk. This commit does the following: - refactors these tests so that they all force a reload from disk - organizes imports - eliminates various bits of repeated code in all the tests - improves code formatting This commit exposes a bug in the encoding/decoding of BridgeConfiguration via a failing test in BridgeConfigurationStorageTest. This test is disabled. This failure will be resolved in a subsequent commit at which point the test will be enabled again. --- ...dressSettingsConfigurationStorageTest.java | 36 ++------- .../BridgeConfigurationStorageTest.java | 73 +++++++------------ .../persistence/ConnectorStorageTest.java | 19 +---- .../DeleteMessagesOnStartupTest.java | 2 - .../DivertConfigurationStorageTest.java | 30 ++++---- .../persistence/DuplicateCacheTest.java | 17 +---- .../RolesConfigurationStorageTest.java | 34 ++------- .../persistence/StorageManagerTestBase.java | 2 + 8 files changed, 62 insertions(+), 151 deletions(-) diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/AddressSettingsConfigurationStorageTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/AddressSettingsConfigurationStorageTest.java index 46060901e4..85e4b78de9 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/AddressSettingsConfigurationStorageTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/AddressSettingsConfigurationStorageTest.java @@ -16,8 +16,6 @@ */ package org.apache.activemq.artemis.tests.integration.persistence; -import static org.junit.jupiter.api.Assertions.assertEquals; - import java.util.HashMap; import java.util.List; import java.util.Map; @@ -35,6 +33,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.ExtendWith; +import static org.junit.jupiter.api.Assertions.assertEquals; + //Parameters set in super class @ExtendWith(ParameterizedTestExtension.class) public class AddressSettingsConfigurationStorageTest extends StorageManagerTestBase { @@ -62,17 +62,13 @@ public class AddressSettingsConfigurationStorageTest extends StorageManagerTestB @TestTemplate public void testStoreSecuritySettings() throws Exception { - createStorage(); - - AddressSettings setting = new AddressSettings(); - - setting = new AddressSettings().setAddressFullMessagePolicy(AddressFullMessagePolicy.BLOCK).setDeadLetterAddress(SimpleString.of("some-test")); + AddressSettings setting = new AddressSettings() + .setAddressFullMessagePolicy(AddressFullMessagePolicy.BLOCK) + .setDeadLetterAddress(SimpleString.of("some-test")); addAddress(journal, "a2", setting); - journal.stop(); - - createStorage(); + rebootStorage(); checkAddresses(journal); @@ -81,22 +77,13 @@ public class AddressSettingsConfigurationStorageTest extends StorageManagerTestB // Replacing the first setting addAddress(journal, "a1", setting); - journal.stop(); - - createStorage(); + rebootStorage(); checkAddresses(journal); - - journal.stop(); - - journal = null; - } @TestTemplate public void testStoreConfigDeleteSettings() throws Exception { - createStorage(); - AddressSettings setting = new AddressSettings() .setConfigDeleteDiverts(DeletionPolicy.FORCE) .setConfigDeleteAddresses(DeletionPolicy.FORCE) @@ -104,16 +91,9 @@ public class AddressSettingsConfigurationStorageTest extends StorageManagerTestB addAddress(journal, "a1", setting); - journal.stop(); - - createStorage(); + rebootStorage(); checkAddresses(journal); - - journal.stop(); - - journal = null; - } /** diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/BridgeConfigurationStorageTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/BridgeConfigurationStorageTest.java index 48d5844bfd..17c8f39141 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/BridgeConfigurationStorageTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/BridgeConfigurationStorageTest.java @@ -16,10 +16,6 @@ */ package org.apache.activemq.artemis.tests.integration.persistence; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; - import java.util.Arrays; import java.util.List; import java.util.Map; @@ -29,10 +25,14 @@ import org.apache.activemq.artemis.core.config.StoreConfiguration; import org.apache.activemq.artemis.core.config.TransformerConfiguration; import org.apache.activemq.artemis.core.persistence.config.PersistedBridgeConfiguration; import org.apache.activemq.artemis.tests.extensions.parameterized.ParameterizedTestExtension; -import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.ExtendWith; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + //Parameters set in super class @ExtendWith(ParameterizedTestExtension.class) public class BridgeConfigurationStorageTest extends StorageManagerTestBase { @@ -41,36 +41,28 @@ public class BridgeConfigurationStorageTest extends StorageManagerTestBase { super(storeType); } - @Override - @BeforeEach - public void setUp() throws Exception { - super.setUp(); - } - @TestTemplate + @Disabled public void testStoreBridgeConfiguration() throws Exception { - createStorage(); - - BridgeConfiguration configuration = new BridgeConfiguration(); - configuration.setName("name"); - configuration.setParentName("name"); - configuration.setQueueName("QueueName"); - configuration.setConcurrency(2); - configuration.setPendingAckTimeout(9876); - configuration.setForwardingAddress("forward"); - configuration.setProducerWindowSize(123123); - configuration.setConfirmationWindowSize(123123); - configuration.setStaticConnectors(Arrays.asList("connector1", "connector2")); TransformerConfiguration mytransformer = new TransformerConfiguration("mytransformer"); mytransformer.getProperties().put("key1", "prop1"); mytransformer.getProperties().put("key2", "prop2"); mytransformer.getProperties().put("key3", "prop3"); - configuration.setTransformerConfiguration(mytransformer); + BridgeConfiguration configuration = new BridgeConfiguration() + .setName("name") + .setParentName("name") + .setQueueName("QueueName") + .setConcurrency(2) + .setPendingAckTimeout(9876) + .setForwardingAddress("forward") + .setProducerWindowSize(123123) + .setConfirmationWindowSize(123123) + .setStaticConnectors(Arrays.asList("connector1", "connector2")) + .setTransformerConfiguration(mytransformer); journal.storeBridgeConfiguration(new PersistedBridgeConfiguration(configuration)); - journal.stop(); - journal.start(); + rebootStorage(); List bridgeConfigurations = journal.recoverBridgeConfigurations(); @@ -90,29 +82,21 @@ public class BridgeConfigurationStorageTest extends StorageManagerTestBase { assertEquals("prop1", properties.get("key1")); assertEquals("prop2", properties.get("key2")); assertEquals("prop3", properties.get("key3")); - journal.stop(); - - journal = null; - } @TestTemplate public void testStoreBridgeConfigurationNoTransformer() throws Exception { - createStorage(); - - BridgeConfiguration configuration = new BridgeConfiguration(); - configuration.setName("name"); - configuration.setParentName("name"); - configuration.setQueueName("QueueName"); - configuration.setConcurrency(2); - configuration.setForwardingAddress("forward"); - configuration.setStaticConnectors(Arrays.asList("connector1", "connector2")); + BridgeConfiguration configuration = new BridgeConfiguration() + .setName("name") + .setParentName("name") + .setQueueName("QueueName") + .setConcurrency(2) + .setForwardingAddress("forward") + .setStaticConnectors(Arrays.asList("connector1", "connector2")); journal.storeBridgeConfiguration(new PersistedBridgeConfiguration(configuration)); - journal.stop(); - - journal.start(); + rebootStorage(); List bridgeConfigurations = journal.recoverBridgeConfigurations(); @@ -125,10 +109,5 @@ public class BridgeConfigurationStorageTest extends StorageManagerTestBase { assertEquals(configuration.getForwardingAddress(), persistedBridgeConfiguration.getBridgeConfiguration().getForwardingAddress()); assertEquals(configuration.getStaticConnectors(), persistedBridgeConfiguration.getBridgeConfiguration().getStaticConnectors()); assertNull(persistedBridgeConfiguration.getBridgeConfiguration().getTransformerConfiguration()); - journal.stop(); - - journal = null; - } - } diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/ConnectorStorageTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/ConnectorStorageTest.java index f81ddc94d2..68510ba249 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/ConnectorStorageTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/ConnectorStorageTest.java @@ -16,18 +16,17 @@ */ package org.apache.activemq.artemis.tests.integration.persistence; -import static org.junit.jupiter.api.Assertions.assertEquals; - import java.util.List; import org.apache.activemq.artemis.core.config.StoreConfiguration; import org.apache.activemq.artemis.core.persistence.config.PersistedConnector; import org.apache.activemq.artemis.tests.extensions.parameterized.ParameterizedTestExtension; import org.apache.activemq.artemis.tests.util.RandomUtil; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.ExtendWith; +import static org.junit.jupiter.api.Assertions.assertEquals; + //Parameters set in super class @ExtendWith(ParameterizedTestExtension.class) public class ConnectorStorageTest extends StorageManagerTestBase { @@ -36,24 +35,16 @@ public class ConnectorStorageTest extends StorageManagerTestBase { super(storeType); } - @Override - @BeforeEach - public void setUp() throws Exception { - super.setUp(); - } - @TestTemplate public void testStoreConnector() throws Exception { final String NAME = RandomUtil.randomString(); final String URL = RandomUtil.randomString(); - createStorage(); PersistedConnector connector = new PersistedConnector(NAME, URL); journal.storeConnector(connector); - journal.stop(); - journal.start(); + rebootStorage(); List connectors = journal.recoverConnectors(); @@ -62,9 +53,5 @@ public class ConnectorStorageTest extends StorageManagerTestBase { PersistedConnector persistedConnector = connectors.get(0); assertEquals(NAME, persistedConnector.getName()); assertEquals(URL, persistedConnector.getUrl()); - journal.stop(); - - journal = null; - } } diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DeleteMessagesOnStartupTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DeleteMessagesOnStartupTest.java index 2cd8da2ab3..56fb8fc25f 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DeleteMessagesOnStartupTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DeleteMessagesOnStartupTest.java @@ -58,8 +58,6 @@ public class DeleteMessagesOnStartupTest extends StorageManagerTestBase { @TestTemplate public void testDeleteMessagesOnStartup() throws Exception { - createStorage(); - Queue theQueue = new FakeQueue(SimpleString.of("")); HashMap queues = new HashMap<>(); queues.put(100L, theQueue); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DivertConfigurationStorageTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DivertConfigurationStorageTest.java index 0c4d751e8e..fee1301b98 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DivertConfigurationStorageTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DivertConfigurationStorageTest.java @@ -41,19 +41,17 @@ public class DivertConfigurationStorageTest extends StorageManagerTestBase { @TestTemplate public void testStoreDivertConfiguration() throws Exception { - createStorage(); - - DivertConfiguration configuration = new DivertConfiguration(); - configuration.setName("name"); - configuration.setAddress("address"); - configuration.setExclusive(true); - configuration.setForwardingAddress("forward"); - configuration.setRoutingName("routiingName"); TransformerConfiguration mytransformer = new TransformerConfiguration("mytransformer"); mytransformer.getProperties().put("key1", "prop1"); mytransformer.getProperties().put("key2", "prop2"); mytransformer.getProperties().put("key3", "prop3"); - configuration.setTransformerConfiguration(mytransformer); + DivertConfiguration configuration = new DivertConfiguration() + .setName("name") + .setAddress("address") + .setExclusive(true) + .setForwardingAddress("forward") + .setRoutingName("routiingName") + .setTransformerConfiguration(mytransformer); journal.storeDivertConfiguration(new PersistedDivertConfiguration(configuration)); @@ -80,14 +78,12 @@ public class DivertConfigurationStorageTest extends StorageManagerTestBase { @TestTemplate public void testStoreDivertConfigurationNoTransformer() throws Exception { - createStorage(); - - DivertConfiguration configuration = new DivertConfiguration(); - configuration.setName("name"); - configuration.setAddress("address"); - configuration.setExclusive(true); - configuration.setForwardingAddress("forward"); - configuration.setRoutingName("routiingName"); + DivertConfiguration configuration = new DivertConfiguration() + .setName("name") + .setAddress("address") + .setExclusive(true) + .setForwardingAddress("forward") + .setRoutingName("routiingName"); journal.storeDivertConfiguration(new PersistedDivertConfiguration(configuration)); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DuplicateCacheTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DuplicateCacheTest.java index 4eb9e1fb09..1b58d8b6c5 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DuplicateCacheTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DuplicateCacheTest.java @@ -16,9 +16,6 @@ */ package org.apache.activemq.artemis.tests.integration.persistence; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -31,10 +28,12 @@ import org.apache.activemq.artemis.core.postoffice.impl.DuplicateIDCaches; import org.apache.activemq.artemis.core.transaction.impl.TransactionImpl; import org.apache.activemq.artemis.tests.extensions.parameterized.ParameterizedTestExtension; import org.apache.activemq.artemis.utils.RandomUtil; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.ExtendWith; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + //Parameters set in super class @ExtendWith(ParameterizedTestExtension.class) public class DuplicateCacheTest extends StorageManagerTestBase { @@ -43,16 +42,8 @@ public class DuplicateCacheTest extends StorageManagerTestBase { super(storeType); } - @AfterEach - @Override - public void tearDown() throws Exception { - super.tearDown(); - } - @TestTemplate public void testDuplicate() throws Exception { - createStorage(); - DuplicateIDCache cache = DuplicateIDCaches.persistent(SimpleString.of("test"), 2000, journal); TransactionImpl tx = new TransactionImpl(journal); @@ -107,8 +98,6 @@ public class DuplicateCacheTest extends StorageManagerTestBase { @TestTemplate public void testDuplicateNonPersistent() throws Exception { - createStorage(); - DuplicateIDCache cache = DuplicateIDCaches.inMemory(SimpleString.of("test"), 2000); TransactionImpl tx = new TransactionImpl(journal); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/RolesConfigurationStorageTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/RolesConfigurationStorageTest.java index 0c7244dc5d..6c021286e0 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/RolesConfigurationStorageTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/RolesConfigurationStorageTest.java @@ -16,8 +16,6 @@ */ package org.apache.activemq.artemis.tests.integration.persistence; -import static org.junit.jupiter.api.Assertions.assertEquals; - import java.util.HashMap; import java.util.List; import java.util.Map; @@ -30,6 +28,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.ExtendWith; +import static org.junit.jupiter.api.Assertions.assertEquals; + //Parameters set in super class @ExtendWith(ParameterizedTestExtension.class) public class RolesConfigurationStorageTest extends StorageManagerTestBase { @@ -54,17 +54,13 @@ public class RolesConfigurationStorageTest extends StorageManagerTestBase { @TestTemplate public void testStoreSecuritySettings() throws Exception { - createStorage(); - addSetting(new PersistedSecuritySetting("a#", "a1", "a1", "a1", "a1", "a1", "a1", "a1", "a1", "a1", "a1", null, null)); addSetting(new PersistedSecuritySetting("a2", "a1", null, "a1", "a1", "a1", "a1", "a1", "a1", "a1", "a1", null, null)); - journal.stop(); - checkSettings(); - createStorage(); + rebootStorage(); checkSettings(); @@ -74,41 +70,26 @@ public class RolesConfigurationStorageTest extends StorageManagerTestBase { checkSettings(); - journal.stop(); - - createStorage(); + rebootStorage(); checkSettings(); - - journal.stop(); - - journal = null; - } @TestTemplate public void testStoreSecuritySettings2() throws Exception { - createStorage(); - checkSettings(); - journal.stop(); - - createStorage(); + rebootStorage(); checkSettings(); addSetting(new PersistedSecuritySetting("a#", "a1", "a1", "a1", "a1", "a1", "a1", "a1", "a1", "a1", "a1", null, null)); - journal.stop(); - - createStorage(); + rebootStorage(); checkSettings(); - journal.stop(); - - createStorage(); + rebootStorage(); checkSettings(); } @@ -128,5 +109,4 @@ public class RolesConfigurationStorageTest extends StorageManagerTestBase { assertEquals(el, el2); } } - } diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/StorageManagerTestBase.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/StorageManagerTestBase.java index a781fcaeca..860787c900 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/StorageManagerTestBase.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/StorageManagerTestBase.java @@ -70,6 +70,8 @@ public abstract class StorageManagerTestBase extends ActiveMQTestBase { execFactory = getOrderedExecutor(); scheduledExecutorService = new ScheduledThreadPoolExecutor(5); + + createStorage(); } @Override