gemmellr commented on code in PR #4351:
URL: https://github.com/apache/activemq-artemis/pull/4351#discussion_r1091788208
##########
artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImplTest.java:
##########
@@ -1082,6 +1083,68 @@ public void
testAddressSettingsPageLimitInvalidConfiguration2() throws Throwable
String randomString = RandomUtil.randomString();
+ properties.put("addressSettings.#.expiryAddress", randomString);
+ properties.put("addressSettings.#.pageLimitMessages", "300");
+ // properties.put("addressSettings.#.pageLimitBytes", "300000"); out on
purpose
+ //properties.put("addressSettings.#.pageFullMessagePolicy", "DROP"); //
removing the pageFull on purpose
Review Comment:
The commented out code seems out of place (and inconsistently
indented)...why not just have the comment explain pageLimitBytes and
pageFullMessagePolicy are missing to test the validation of them being missing.
Which the existing comments dont even really make clear.
##########
artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImplTest.java:
##########
@@ -1082,6 +1083,68 @@ public void
testAddressSettingsPageLimitInvalidConfiguration2() throws Throwable
String randomString = RandomUtil.randomString();
+ properties.put("addressSettings.#.expiryAddress", randomString);
+ properties.put("addressSettings.#.pageLimitMessages", "300");
+ // properties.put("addressSettings.#.pageLimitBytes", "300000"); out on
purpose
+ //properties.put("addressSettings.#.pageFullMessagePolicy", "DROP"); //
removing the pageFull on purpose
+
+ configuration.parsePrefixedProperties(properties, null);
+
+ Assert.assertEquals(1, configuration.getAddressSettings().size());
+ Assert.assertEquals(SimpleString.toSimpleString(randomString),
configuration.getAddressSettings().get("#").getExpiryAddress());
+ Assert.assertEquals((Long)300L,
configuration.getAddressSettings().get("#").getPageLimitMessages());
+ Assert.assertEquals(null,
configuration.getAddressSettings().get("#").getPageLimitBytes());
+ Assert.assertEquals(null,
configuration.getAddressSettings().get("#").getPageFullMessagePolicy());
+
+ PagingStore storeImpl = new PagingStoreImpl(new SimpleString("Test"),
(ScheduledExecutorService) null, 100L, Mockito.mock(PagingManager.class),
Mockito.mock(StorageManager.class), Mockito.mock(SequentialFileFactory.class),
Mockito.mock(PagingStoreFactory.class), new SimpleString("Test"),
configuration.getAddressSettings().get("#"), null, null, true);
+
+ Assert.assertEquals(null, storeImpl.getPageLimitMessages());
+ Assert.assertEquals(null, storeImpl.getPageLimitBytes());
+ Assert.assertEquals(null, storeImpl.getPageFullMessagePolicy());
+ Assert.assertTrue(AssertionLoggerHandler.findText("AMQ224125"));
+ }
+
+ @Test
+ public void testAddressSettingsPageLimitInvalidConfiguration3() throws
Throwable {
+ AssertionLoggerHandler.startCapture();
+ runAfter(AssertionLoggerHandler::stopCapture);
+ ConfigurationImpl configuration = new ConfigurationImpl();
+
+ Properties properties = new Properties();
+
+ String randomString = RandomUtil.randomString();
+
+ properties.put("addressSettings.#.expiryAddress", randomString);
+ //properties.put("addressSettings.#.pageLimitMessages", "300");
+ properties.put("addressSettings.#.pageLimitBytes", "300000"); //
removing this on purpose
+ //properties.put("addressSettings.#.pageFullMessagePolicy", "DROP"); //
removing the pageFull on purpose
Review Comment:
As before, just comment about pageLimitMessages and pageFullMessagePolicy
being missing to test that, without the commented-out code.
The "// removing this on purpose" comment doesnt actually match up, with it
not being removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
