You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by "gnodet (via GitHub)" <gi...@apache.org> on 2026/03/11 14:29:30 UTC

[PR] Fix flaky tests in Undertow, JMS, and tracing modules [cxf]

gnodet opened a new pull request, #2946:
URL: https://github.com/apache/cxf/pull/2946

   ## Summary
   
   Fixes three categories of flaky test failures observed in CI:
   
   ### UndertowHTTPServerEngineTest
   - **Root cause**: Port cleanup (`destroyForPort`) was done at the end of each test body. If a test failed mid-execution (e.g., assertion failure), ports remained bound, causing cascading "Address already in use" failures in subsequent tests.
   - **Fix**: Move port cleanup to an `@After` method that always runs, even on test failure.
   
   ### JMSDestinationTest (via AbstractJMSTester)
   - **Root cause**: Classic lost-notification race condition in `waitForReceiveInMessage()` and `waitForReceiveDestMessage()`. The null check was outside the `synchronized` block, so if a message arrived between the check and `wait()`, the `notifyAll()` fired before `wait()` started, causing the test to block for the full 10-second (or 60-second JMS-level) timeout.
   - **Fix**: Move the null check inside the `synchronized` block.
   
   ### Tracing tests (Brave, Micrometer, OpenTelemetry, OpenTracing)
   - **Root cause 1**: Awaitility timeouts of 1 second were too tight for slow CI environments. In `testThatNewSpanIsCreatedOnClientTimeout`, the `ConditionTimeoutException` from the `finally` block's await would mask the expected `ProcessingException`, causing JUnit's `ExpectedException` rule to fail.
   - **Root cause 2**: In `OpenTracingTracingTest`, the static `InMemoryReporter` could receive spans from a previous test's asynchronous server-side processing after `@After` cleared it but before the next test's assertions, causing unexpected span counts.
   - **Fix**: Increase all awaitility timeouts from 1s to 10s (awaitility polls, so fast tests still complete quickly). Add `@Before` cleanup to `OpenTracingTracingTest` to catch stale spans.
   
   ## Test plan
   - [x] Verify all modified tests still pass locally
   - [ ] CI should show reduced flaky test failures
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
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: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix flaky tests in Undertow, JMS, and tracing modules [cxf]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #2946:
URL: https://github.com/apache/cxf/pull/2946#issuecomment-4040561665

   @reta thank you for the feedback. You're right that the tracing changes were too broad.
   
   Regarding the specific points:
   - **Tracing timeouts**: Agreed — if the tests are stable on CI, increasing timeouts isn't the right fix.
   - **`@Before` cleanup**: I understand — `@After` + `OpenTelemetryRule` should be sufficient within a class, and the static `TestSpanHandler.SPANS` list shouldn't leak between classes since each test class creates its own `Tracing` instance.
   - **JMS**: The race condition fix (moving null check inside `synchronized`) is a genuine TOCTOU bug — if a message arrives between `inMessage.get() == null` and `inMessage.wait()`, the notification is lost. However, I agree that some JMS failures are caused by messages simply not arriving (I observed 2 flakes even with the fix applied locally).
   
   I've created #2947 with only the Undertow `@After` cleanup and JMS race condition fix, dropping all tracing changes. Closing this one.


-- 
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: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix flaky tests in Undertow, JMS, and tracing modules [cxf]

Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on code in PR #2946:
URL: https://github.com/apache/cxf/pull/2946#discussion_r2919330019


##########
systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/opentelemetry/OpenTelemetryTracingTest.java:
##########
@@ -122,6 +123,11 @@ private static Context fromRandom() {
                         TraceState.getDefault())));
     }
 
+    @Before
+    public void setUp() {
+        otelRule.clearSpans();

Review Comment:
   This is not need, the whole purpose of using OpenTelemetryRule  (JUnit rule) is automatic setup / teardown logic



##########
systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/opentelemetry/OpenTelemetryTracingTest.java:
##########
@@ -122,6 +123,11 @@ private static Context fromRandom() {
                         TraceState.getDefault())));
     }
 
+    @Before
+    public void setUp() {
+        otelRule.clearSpans();

Review Comment:
   This is not needed, the whole purpose of using OpenTelemetryRule  (JUnit rule) is automatic setup / teardown logic



-- 
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: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix flaky tests in Undertow, JMS, and tracing modules [cxf]

Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on PR #2946:
URL: https://github.com/apache/cxf/pull/2946#issuecomment-4040200096

   @gnodet few things here:
    - I believe increasing timeouts to some arbitrary value is a not a good fix, afaik the tracing tests are not an issue anymore, the CI (builds.apache.org) could serve as a proof
    - JMS tests, as far as I can tell, are not failing because of timeout, the message(s) are not arriving (have to JIRAs opened)
   
   I would prefer to target them one by one, thank you


-- 
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: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix flaky tests in Undertow, JMS, and tracing modules [cxf]

Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on code in PR #2946:
URL: https://github.com/apache/cxf/pull/2946#discussion_r2919320009


##########
systests/tracing/src/test/java/org/apache/cxf/systest/brave/jaxrs/AbstractBraveTracingTest.java:
##########
@@ -77,6 +78,11 @@ public abstract class AbstractBraveTracingTest extends AbstractClientServerTestB
         .build();
 
 
+    @Before

Review Comment:
   This is not needed - see please `tearDown` method



-- 
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: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix flaky tests in Undertow, JMS, and tracing modules [cxf]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet closed pull request #2946: Fix flaky tests in Undertow, JMS, and tracing modules
URL: https://github.com/apache/cxf/pull/2946


-- 
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: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org