[
https://issues.apache.org/jira/browse/ARTEMIS-3808?focusedWorklogId=773993&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-773993
]
ASF GitHub Bot logged work on ARTEMIS-3808:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 24/May/22 12:49
Start Date: 24/May/22 12:49
Worklog Time Spent: 10m
Work Description: gemmellr commented on code in PR #4093:
URL: https://github.com/apache/activemq-artemis/pull/4093#discussion_r880213344
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java:
##########
@@ -4372,6 +4372,7 @@ class FakeWebServerComponent implements ServiceComponent,
WebServerComponentMark
FakeWebServerComponent(CountDownLatch startDelay) {
this.startDelay = startDelay;
+ new Exception("startDelay = " + startDelay).printStackTrace();
Review Comment:
Leftover debug? Remove or convert to logging?
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java:
##########
@@ -4337,9 +4337,9 @@ public void testRestartEmbeddedWebServerTimeout() throws
Exception {
ActiveMQServerControl serverControl = createManagementControl();
try {
- serverControl.restartEmbeddedWebServer(10);
+ serverControl.restartEmbeddedWebServer(1);
fail();
- } catch (ActiveMQTimeoutException e) {
+ } catch (Exception e) {
Review Comment:
From looking at test results elsewhere that I think you based these changes
on, it seems like this test was passing as it was, and only the subclass
'ActiveMQServerControlUsingCoreTest' version was failing, due to some proxy it
uses, and which you have now overridden the method in that class to avoid
executing this specific test case. As such, it seems like this change
could/should be removed.
(This essentially reverses a change I suggested before it was added, to
throw an appropriate exception type (It was throwing/catching
AMQIllegalStateException). The aim of this test is to check it times out, which
it takes steps to ensure happens. As such I'd expect it to be throwing a
timeout exception and verifying it did. With this change it would no longer
actually verifies a timeout occurred, but just 'something/anything went wrong',
making it a poor test. Theres actually a separate specific test for verifying
that distinct handling, which you necessarily updated as it was broken.)
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java:
##########
@@ -4358,8 +4358,8 @@ public void testRestartEmbeddedWebServerException()
throws Exception {
try {
serverControl.restartEmbeddedWebServer(10);
fail();
- } catch (ActiveMQIllegalStateException e) {
- assertEquals(message, e.getMessage());
+ } catch (ActiveMQException e) {
+ assertEquals(message, e.getCause().getMessage());
Review Comment:
It seems ActiveMQMessageBundle.embeddedWebServerRestartFailed(Exception)
does use the failure exception as the cause of a new exetpion, meaning this was
somehow always broken and making this change in catch reasonable/necessary. I
think it should just verify now that the cause is the actual exception the test
threw though rather than just comparing the messages, that way its more closely
verifying the behaviour and still checking the type is correct as it originally
tried to.
E.g. assertSame("Unexpected cause", startException, e.getCause());
Issue Time Tracking
-------------------
Worklog Id: (was: 773993)
Time Spent: 7h (was: 6h 50m)
> Support starting/stopping the embedded web server via mangement
> ---------------------------------------------------------------
>
> Key: ARTEMIS-3808
> URL: https://issues.apache.org/jira/browse/ARTEMIS-3808
> Project: ActiveMQ Artemis
> Issue Type: Improvement
> Reporter: Justin Bertram
> Assignee: Justin Bertram
> Priority: Major
> Time Spent: 7h
> Remaining Estimate: 0h
>
> It would be useful to be able to cycle the embedded web server if, for
> example, one needed to renew the SSL certificates.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
