Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARTEMIS-4548 refactor MQTT tests #4726

Merged
merged 1 commit into from
Jan 11, 2024
Merged

Conversation

jbertram
Copy link
Contributor

Many MQTT tests are run twice - once using TCP and once using WebSockets. This is essentially a big waste of time since once the connection is established to the broker the tests are identical. The tests should be refactored to run just once and then there can be a small number of tests specifically for WebSockets.

This should knock several minutes off the test-suite.

@jbertram jbertram force-pushed the ARTEMIS-4548 branch 2 times, most recently from db5ea3b to 8738410 Compare December 22, 2023 20:03
@gemmellr
Copy link
Member

Not clear if you ran the full test suite on this already, but probably worth a rebase and re-run at this point either way. Havent looked at it yet but will try to get to it soon.

Many MQTT tests are run twice - once using TCP and once using
WebSockets. This is essentially a big waste of time since once the
connection is established to the broker the tests are identical. The
tests should be refactored to run just once and then there can be a
small number of tests specifically for WebSockets.

This should knock several minutes off the test-suite.
@jbertram
Copy link
Contributor Author

Rebased. Test-suite is good!

@tabish121
Copy link
Contributor

Change seems to make sense as it reduces redundant test runs, something we made the mistake of doing in the 5.x code base was have all protocol tests run for every permutation of TCP, SSL, NIO, Auto, etc transports which explodes the test time. It is important to create tests that capture any cases where the protocol or transport do come into play though.

@gemmellr gemmellr merged commit 99d43da into apache:main Jan 11, 2024
6 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.

3 participants