added check for open port before creating test server.#769
Conversation
dhaval24
left a comment
There was a problem hiding this comment.
Few best practice suggestions. It would be awesome if you can make those changes. Rest looks good.
| private int Min = 1050; | ||
| private int Max = 15000; | ||
| private int portNumber = Min + (int)(Math.random() * ((Max - Min) + 1)); | ||
| private final int Min = 1050; |
There was a problem hiding this comment.
@littleaj can you please adhere to camelCase for varable names here. I know you didn't made this variable but since you touched this part of the code better that we can clean up if possible.
|
|
||
| public JettyTestServer() { | ||
| // try 256 times for ports | ||
| int initialPortNumber = Min + (int)(Math.random() * ((Max - Min) + 1)); |
There was a problem hiding this comment.
A standard way / best practice in java to get random integer within the range is this:
import java.util.concurrent.ThreadLocalRandom;
// nextInt is normally exclusive of the top value,
// so add 1 to make it inclusive
int randomNum = ThreadLocalRandom.current().nextInt(min, max + 1);
We don't need to reproduce the results, so seed of random doesn't really matter.
@littleaj it would be great if you can change this one as bonus!
There was a problem hiding this comment.
Since this is test code, so I'm not really worried about this.
There was a problem hiding this comment.
Cool :) no worries. Please proceed with merging.
This makes the tests a bit more reliable.