[
https://issues.apache.org/jira/browse/CAMEL-19549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18090936#comment-18090936
]
Torsten Mielke commented on CAMEL-19549:
----------------------------------------
[~orpiske] I had a look at this in the last day.
There are 56 Test classes in {{core/camel-core}} that use {{Thread.sleep()}} in
some form or another. So it is quite an extensive review that would be needed
here.
I tried to involve an AI agent but it ...
- repeatedly suggested far more complex alternatives to {{Thread.sleep()}} in
cases where the intend was truly to let some time pass and not to coordinate
concurrent actions,
- did not detect all possible improvements
So I started to review test classes more manually and found that most
occurrences of {{Thread.sleep()}} that I saw were indeed to ensure some time
passes before the next event and not to coordinate actions.
Files I reviewed where the use of Thread.sleep() seems legitimate
- {{src/test/java/org/apache/camel/impl/StopTimeoutRouteTest.java}}
sleep is used to test stopping the route while an exchange is still in
progress
-
{{src/test/java/org/apache/camel/impl/ShutdownStrategyNotSuppressLoggingOnTimeoutManualTest.java}}
manual test where sleep is used to test shut down strategy while exchange is
being processed
-
{{src/test/java/org/apache/camel/impl/ShutdownStrategySuppressLoggingOnTimeoutManualTest.java}}
manual test where sleep is used to test shut down strategy while exchange is
being processed
- {{src/test/java/org/apache/camel/impl/engine/PeriodTaskSchedulerTest.java}}
sleep used to allow some time to pass between events
- {{src/test/java/org/apache/camel/util/CaseInsensitiveMapTest.java}}
manual test with explicit sleep call to take heap dumps
- {{src/test/java/org/apache/camel/processor/PipelineConcurrentTest.java}}
sleep used to generate a more random load
-
{{src/test/java/org/apache/camel/processor/enricher/PollEnrichFile.DefaultAggregationStrategyTest.java}}
sleep used to allow some time to pass between events
- {{src/test/java/org/apache/camel/processor/enricher/PollTest.java}}
sleep used to allow some time to pass between events
- {{src/test/java/org/apache/camel/processor/enricher/PollEnricherTest.java}}
sleep used to allow some time to pass between events
-
{{src/test/java/org/apache/camel/processor/SplitterParallelBigFileManualTest.java}}
manual test with explicit sleep call to take heap dumps
I then gave up as these uses of {{Thread.sleep()}} all seem legitimate to me.
Based on the analysis so far I wonder if it's worth to manually review the
other 40+ test classes?
I would not trust AI agents too much here, so do not always understand the
concurrency of the tests correctly.
I had suggestions from the agent that when challenged was completely withdrawn
again by the agent.
Nevertheless, I do have some improvements to suggest in a few files but it
would not be a full review of all the test classes.
Does it make sense to open a PR for the changes I have so far, even though it
won't resolve this ticket entirely?
> camel-core: replace Thread.sleep in tests
> -----------------------------------------
>
> Key: CAMEL-19549
> URL: https://issues.apache.org/jira/browse/CAMEL-19549
> Project: Camel
> Issue Type: Task
> Components: camel-core, 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)
