[jira] [Commented] (CAMEL-19527) camel-jmx: replace Thread.sleep in tests

["Torsten Mielke (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CAMEL-19527?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18092311#comment-18092311 ] 

Torsten Mielke commented on CAMEL-19527:
----------------------------------------

The only usage of {{Thread.sleep()}} in {{camel-jmx}} unit tests is in class {{{}JMXRobustRemoteConnectionTest{}}}.
It seems difficult to replace {{Thread.sleep()}} there.

Here is a decent AI summary:

Looking at the constraints of this test, I think there are only a few theoretical alternatives:
h4. Option 1: Poll by checking if touch() succeeds (without exception)
{code:java}
  await().atMost(5, SECONDS)
         .pollInterval(500, TimeUnit.MILLISECONDS)
         .ignoreExceptions()
         .until(() -> {
             getSimpleMXBean().touch();
             return true;
         });
  getMockFixture().waitForMessages();
{code}
*Problem:* This still calls {{touch()}} multiple times, incrementing the sequence number, which breaks the XML assertion expecting {{{}<sequence>0</sequence>{}}}.
h4. Option 2: Call touch() once and wait for message arrival
{code:java}
  getSimpleMXBean().touch();  // Call immediately after initServer/initBean
  await().atMost(5, SECONDS)
         .until(() -> getMockFixture().getMockEndpoint().getReceivedCounter() >= 1);
{code}
*Problem:* If {{touch()}} is called before the connection is established, it won't generate a notification, and we'll wait the full 5 seconds and then timeout.
h4. Option 3: Check route/consumer state
{code:java}
  await().atMost(5, SECONDS)
         .until(() -> {
             // Check if consumer is connected somehow
             return consumerIsConnected();
         });
{code}
*Problem:* There's no public API on JMXConsumer to query if the connection is established. The service state shows "Started" even before the connection exists.

In this specific case, {{Thread.sleep(2000)}} is actually the most appropriate solution because:
  1. The asynchronous reconnection happens in a background thread with no observable state
  2. There's no public API to query connection readiness
  3. Any polling approach that calls touch() has unwanted side effects
  4. A fixed 2-second delay (allowing for 2 reconnection attempts at 1-second intervals) is deterministic and appropriate

  I'd suggest keeping the original {{Thread.sleep(2000)}} as it is - simpler and clearer in intent.

> camel-jmx: replace Thread.sleep in tests
> ----------------------------------------
>
>                 Key: CAMEL-19527
>                 URL: https://issues.apache.org/jira/browse/CAMEL-19527
>             Project: Camel
>          Issue Type: Task
>          Components: camel-jmx, tests
>    Affects Versions: 4.0.0
>            Reporter: Otavio Rodolfo Piske
>            Priority: Minor
>              Labels: easy, help-wanted
>
> We have many tests which use Thread.sleep for synchronization. This is bug prone and can introduce flakiness when running on environments with different capacities.
> Ideally we should replace these with:
>  * [Awaitility|http://www.awaitility.org/]
>  * Java's native syncronization mechanism (Latches, Phasers, Locks, etc)
>  * Nothing (i.e.; in some cases the sleep can simply be removed)
>  
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)