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

ISPN-7989 Fixed Spring remote test #5256

Closed

Conversation

slaskawi
Copy link
Contributor

@slaskawi slaskawi commented Jul 3, 2017

hcb.port(15233);
hotrodServer = HotRodClientTestingUtil.startHotRodServer(cacheManager, hcb);
hcb.port(PORT);
hotrodServer = HotRodClientTestingUtil.startHotRodServer(cacheManager, PORT, hcb);
Copy link

@gustavocoding gustavocoding Jul 3, 2017

Choose a reason for hiding this comment

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

FYI, HotRodClientTestingUtil.startHotRodServer(cacheManager, hcb) will pick a free port

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the 2 argument #startHotRodServer does that. That's why this test was broken :(

Choose a reason for hiding this comment

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

Nevermind, I was about to suggest not to hardcode the port in the test, but rely in the free port discovered, but I now see this test uses a properties file

Copy link
Member

Choose a reason for hiding this comment

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

@slaskawi I suppose we'll never have enough Spring tests to make it worth running in parallel, so a fixed port should be fine. But instead of hard-coding the port here, I would inject the properties file and parse the port from the infinispan.client.hotrod.server_list property.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danberindei +1. But I'd prefer to do it in a separate JIRA (so I created ISPN-8017)

@slaskawi
Copy link
Contributor Author

slaskawi commented Jul 4, 2017

Apparently this issue has been already fixed by 1a3d653#diff-5af2436e9ce44e0c2a8f7d448537bb96L50. Thanks @tristantarrant!

@slaskawi slaskawi closed this Jul 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants