-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Support random test port #4140
Support random test port #4140
Conversation
@gsmet it would be nice if this PR could make it to 0.23.0. I'd appreciate if you (or someone else) could review |
This is going to conflict with #4128 (which I have already had to rebase to fix a different conflict 🤪) |
Is there a specific reason? It looks like a new feature to me so I would keep it for 0.24.0. |
@gsmet no strong reason except early feedback from users, I'm fine with that in 0.24.0 ;) |
Rebased, thanks for the warning @geoand 👍 |
FYI: #3938 |
test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tried running the MP TCKs with it and noticed that setting a -Dquarkus.http.test-port
breaks a lot of tests in MP OpenApi tests. I'll have a look what can be done about that (it's not caused by this PR).
Perhaps an additional CI check with a random port enabled would be a good thing to introduce.
Anyway very nice, this will be very useful
I like this! Let's see what other think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, the only caveat is that this will not work with native mode tests.
The only way I can see to make it work is to parse the log output to read the http port, but that kinda sucks.
By supporting random ports during test execution, this PR should allow running HTTP tests in parallel (as long as they run in different JVMs)
Fixes #2269 and closes #2324