Skip to content

Fix CI test flakes#3299

Merged
guusdk merged 6 commits intoigniterealtime:mainfrom
Fishbowler:flakes
May 5, 2026
Merged

Fix CI test flakes#3299
guusdk merged 6 commits intoigniterealtime:mainfrom
Fishbowler:flakes

Conversation

@Fishbowler
Copy link
Copy Markdown
Member

Herein lie attempts to fix the recent test flakes in CI. I believe them all to be reasonable changes or reasonable compromises.

Fishbowler added 2 commits May 3, 2026 21:49
Commit ea75775 explicitly added tolerance to the TCP test, but references BOSH. I've extended it to BOSH - no idea if TCP is in error.
@Fishbowler Fishbowler requested a review from guusdk May 4, 2026 15:48
Copy link
Copy Markdown
Member

@guusdk guusdk left a comment

Choose a reason for hiding this comment

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

I don't really like the blunt retries - it hides a problem to the point where having the test may not reliably prove much any more.

I was considering to simply disable these tests, until we can get them more stable.

Happy to test these changes to see if they make a difference, but meh - I don' have high hopes.

Copy link
Copy Markdown
Member

@guusdk guusdk left a comment

Choose a reason for hiding this comment

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

I don't really like the blunt retries - it hides a problem to the point where having the test may not reliably prove much any more.

I was considering to simply disable these tests, until we can get them more stable.

Happy to test these changes to see if they make a difference, but meh - I don' have high hopes.

@Fishbowler
Copy link
Copy Markdown
Member Author

I don't really like the blunt retries - it hides a problem to the point where having the test may not reliably prove much any more.

Me neither, but in this case, it feels like a necessary evil, and didn't feel altogether all that evil....

Retries are added to 2 places:

  • on the SSL socket layer, where the retry is dealing with a test setup race condition. We've already got the same line in RemoteReceivingServerDummy
  • for a single retry on the outgoing test, where the expectation is testing combinations of connection logic for an outcome. If something is broken, it's likely always broken. Whilst it might fail for other reasons, it's unlikely to "accidentally pass". It's likely only hiding some test-only warm-up bug.

Fishbowler added 2 commits May 4, 2026 17:51
ea75775 added a lenience, but gasve it to TCP rather than BOSH connection
Mockito was causing UnfinishedStubbingException on XMPPServer.setInstance(newMock)
@guusdk guusdk merged commit d33ef54 into igniterealtime:main May 5, 2026
61 of 62 checks passed
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.

2 participants