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

Support random test port #4140

Merged
merged 1 commit into from
Oct 3, 2019
Merged

Support random test port #4140

merged 1 commit into from
Oct 3, 2019

Conversation

gastaldi
Copy link
Contributor

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

@gastaldi
Copy link
Contributor Author

@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

@geoand
Copy link
Contributor

geoand commented Sep 21, 2019

This is going to conflict with #4128 (which I have already had to rebase to fix a different conflict 🤪)

@gsmet
Copy link
Member

gsmet commented Sep 21, 2019

Is there a specific reason? It looks like a new feature to me so I would keep it for 0.24.0.

@geoand geoand added the triage/needs-rebase This PR needs to be rebased first because it has merge conflicts label Sep 21, 2019
@gastaldi
Copy link
Contributor Author

@gsmet no strong reason except early feedback from users, I'm fine with that in 0.24.0 ;)

@geoand geoand removed the triage/needs-rebase This PR needs to be rebased first because it has merge conflicts label Sep 21, 2019
By supporting random ports during test execution, this should allow running HTTP tests in parallel (as long as they run in different JVMs)

Fixes #2269 and closes #2324
@gastaldi
Copy link
Contributor Author

Rebased, thanks for the warning @geoand 👍

@jglick
Copy link

jglick commented Sep 23, 2019

as long as they run in different JVMs

FYI: #3938

Copy link
Contributor

@jmartisk jmartisk left a 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

@geoand
Copy link
Contributor

geoand commented Sep 25, 2019

I like this! Let's see what other think

Copy link
Member

@stuartwdouglas stuartwdouglas left a 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.

@stuartwdouglas stuartwdouglas added this to the 0.24.0 milestone Oct 3, 2019
@stuartwdouglas stuartwdouglas merged commit 0c4c609 into quarkusio:master Oct 3, 2019
@gastaldi gastaldi deleted the random_test_port branch October 3, 2019 05:49
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.

Random port configuration of port in tests
6 participants