Skip to content

Facky tests revealed mainly with faster CI (aka Github Actions but also with Java 25)#1610

Merged
jbonofre merged 9 commits into
apache:mainfrom
jeanouii:fix/flacky-tests-github-actions
Jan 16, 2026
Merged

Facky tests revealed mainly with faster CI (aka Github Actions but also with Java 25)#1610
jbonofre merged 9 commits into
apache:mainfrom
jeanouii:fix/flacky-tests-github-actions

Conversation

@jeanouii

Copy link
Copy Markdown
Contributor

Includes various fixes related to race conditions or bugs

@jeanouii jeanouii force-pushed the fix/flacky-tests-github-actions branch 2 times, most recently from 1d892cb to 541942e Compare January 13, 2026 20:39
@jbonofre jbonofre self-requested a review January 14, 2026 16:44
@jeanouii jeanouii force-pushed the fix/flacky-tests-github-actions branch from c2a5a80 to a695119 Compare January 15, 2026 17:30
…ailover

During failover in transacted sessions with async dispatch (MessageListener),
  prefetched messages sitting in the unconsumedMessages buffer were not being
  tracked in previouslyDeliveredMessages. This caused them to be incorrectly
  identified as duplicates on redelivery and poison-acked to the DLQ.
Give the GC a bit more time to operate before failing
Add some tolerance to JVM timing variations. The test is to validate the priority ordering not the overall performance, so it's ok
Fix test stability
Add some flexibility to the test assertion
Use Ephemeral ports when possible
Fix race condition in test
Improve stability
Harden Stomp test because of timing issues/race conditions
Move the JMSCOntext.start() a bit later to avoid race conditions
@jeanouii jeanouii force-pushed the fix/flacky-tests-github-actions branch from d3cb053 to f4ebde6 Compare January 16, 2026 14:01

@jbonofre jbonofre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed all the updated test and all looks good, fixing several race conditions.

@jbonofre jbonofre merged commit 94c3c3d into apache:main Jan 16, 2026
7 checks passed
@cshannon

Copy link
Copy Markdown
Contributor

I don't understand why we are combining multiple unrelated changes in one PR? This doesn't make sense at all to combine several Jiras of changes into one commit. We need to be able to isolate issues by jira to rollback or fix

@cshannon

Copy link
Copy Markdown
Contributor

The consumer changes to the client especially need to be reviewed more they are higher risk changes and should be by itself

@jbonofre

Copy link
Copy Markdown
Member

OK, the change doesn't look problematic to me.

I propose to keep the test updates and revert the client change for broader discussion then.

@jeanouii do you mind to create a PR to revert client change ?

mattrpav added a commit to mattrpav/activemq that referenced this pull request Jan 16, 2026
…s but also with Java 25) (apache#1610)"

This reverts commit 94c3c3d.

Note: Reverting due to accidental inclusion of [AMQ-9829] ActiveMQMessageConsumer.java
@cshannon

Copy link
Copy Markdown
Contributor

@jbonofre - Those changes themselves may or may not be fine, but they need to be done in isolation so that they can be noticed. I didn't even notice it was part of this because the title of the PR only mentions flaky tests and faster CI.

Anything with a bug fix, especially I think needs to be separated out. Also, if it really is a bug we should backport the fix but we may not necessarily want to backport the rest of the commit.

mattrpav added a commit that referenced this pull request Jan 16, 2026
…s but also with Java 25) (#1610)" (#1617)

This reverts commit 94c3c3d.

Note: Reverting due to accidental inclusion of [AMQ-9829] ActiveMQMessageConsumer.java
@jbonofre

jbonofre commented Jan 16, 2026

Copy link
Copy Markdown
Member

@cshannon i agree. Sorry about that. My intent was to unblock all the freaking test failures thanks to @jeanouii . I under-estimated the change on the client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants