mqtt-streaming: test two fixes for flaky tests #460
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ions' test It appears the flakiness was caused by the two connections using the same connection id for different connections in the same session. I'm not intimately familiar with this codebase, but AFAICS it seems this connection id is an internal concern to the component, and not an MQTT concept. I think we should probably provide an API that does not bother the user with this concept at all, and manages non-overlapping connection ids entirely internal to the component. I don't get the impression that it was the purpose of the test to test the behaviour in case of overlapping connection ids. Fixes apache#454 After this change the 'unsubscribe a client session' seems to fail a lot on CI, but that seems unrelated and something to fix in a separate PR.
Previously, in this test, the server sent a 'publish' concurrent to the client sending the 'ack' for that publish. This fix makes sure the 'ack' is sent only after observing the 'publish'. Hopefully fixes apache#458
This was referenced Feb 8, 2024
|
(ran the tests a few times, they now succeeded each time, I think this sufficiently suggests those two PRs fix the main flakiness) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
this PR combines #456 and #459 to see if CI agrees they are now fixed. Not for merge, merge them both separately.