[
https://issues.apache.org/jira/browse/ARTEMIS-3627?focusedWorklogId=713726&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-713726
]
ASF GitHub Bot logged work on ARTEMIS-3627:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 24/Jan/22 13:29
Start Date: 24/Jan/22 13:29
Worklog Time Spent: 10m
Work Description: gemmellr commented on a change in pull request #3924:
URL: https://github.com/apache/activemq-artemis/pull/3924#discussion_r790722908
##########
File path:
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/server/config/JMSServerPropertyConfigTest.java
##########
@@ -78,7 +78,31 @@ public void testConfigViaBrokerPropertiesFromClasspath()
throws Exception {
server.setConfiguration(configuration);
server.start();
- assertEquals("ConfiguredViaProperties",
server.getActiveMQServer().getConfiguration().getName());
+ assertEquals(-2,
server.getActiveMQServer().getConfiguration().getGracefulShutdownTimeout());
Review comment:
Would be good to add a constant with/or comment to explain where the -2
comes from, will be very non-obvious later (isnt even that obvious now, until
the diff following later to explain it in a totally different file).
Should probably also assert that
ActiveMQDefaultConfiguration.getDefaultGracefulShutdownTimeout() isnt -2,
otherwise this test and the next one could be checking the same thing and not
really verifying that "server.setPropertiesResourcePath(null);" does anything.
(The tests are so similar they could also share a common impl and pass a
boolean to say whether to set the value and govern the expected result..might
make them more obvious).
##########
File path: tests/integration-tests/src/test/resources/broker.properties
##########
@@ -15,4 +15,6 @@
# limitations under the License.
#
-name=ConfiguredViaProperties
\ No newline at end of file
+# use a very innocuous bit of config b/c it will be picked up by all embedded
servers in the integration tests
Review comment:
If unit testing properly verified the file config is being picked up and
used as expected I dont see a real need for the integration test using the
actual default _broker.properties_ file, and especially not doing so for
_every_ integration test even though the broker doesnt provide a default file.
That just means the tests are _always_ going on a path that actual use
typically wont use. Would seem nicer to use a non-default file or only create
the file during the test that needs it.
--
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]
Issue Time Tracking
-------------------
Worklog Id: (was: 713726)
Time Spent: 2.5h (was: 2h 20m)
> Allow properties to configure 'new' broker attributes and load from a file
> --------------------------------------------------------------------------
>
> Key: ARTEMIS-3627
> URL: https://issues.apache.org/jira/browse/ARTEMIS-3627
> Project: ActiveMQ Artemis
> Issue Type: Improvement
> Components: Configuration
> Affects Versions: 2.20.0
> Reporter: Gary Tully
> Assignee: Gary Tully
> Priority: Major
> Fix For: 2.21.0
>
> Time Spent: 2.5h
> Remaining Estimate: 0h
>
> Following up on ARTEMIS-851, where simple set attributes can be overridden
> via system properties. This enhancement will allow properties to be loaded
> from broker.properties and allow additions to lists and maps.
> The additions to the collections require some naming convention. Many of the
> elements already support a name attribute and this can be used to locate and
> or create new entries.
> The idea is that a plain broker can be augmented with the presence of a
> properties file to add a new acceptor called "tcp" using properties of the
> form:
> acceptorConfigurations.tcp.params.host=localhost
> acceptorConfigurations.tcp.params.port=61616
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
