feat(net): Add method to check if port is in LISTEN state #1043
Conversation
| this.$errors.failWithoutHelp(`Unable to check for free ports on ${platform}. Supported platforms are: ${_.keys(platformData).join(", ")}`); | ||
| } | ||
|
|
||
| while (true) { |
There was a problem hiding this comment.
Instead of while (true) you can set the condition while (new Date().getTime() < endTime) and delete the break; code below.
IMO it'd be a bit clearer than looking at what seems to be an endless loop at a first glance
There was a problem hiding this comment.
The current flow executes at least one check, no matter of the passed timeout. Changing the while statement will change this behavior.
Also the current workflow checks if a timeout is passed and performs at least one check after that. Changing the while statement will change this behavior as well, as we have sleep at the end of the while. The sleep cannot be moved at the first line of the while as we may not have to wait for it.
So the desired workflow is:
- Check if the port is in LISTEN state.
- If yes - break. If not, check if timeout is passed and in case not - sleep for specified interval.
- Check if port is in LISTEN state.
- If yes - break. If not, check if timeout is passed and in case not - sleep for specified interval.
... Repeat until timeout is reached.
services/net-service.ts
Outdated
| public async waitForPortToListen(port: number, timeout: number, interval?: number): Promise<boolean> { | ||
| interval = interval || Net.DEFAULT_INTERVAL; | ||
| const endTime = new Date().getTime() + timeout; | ||
| const platformData: IDictionary<any> = { |
There was a problem hiding this comment.
You can introduce an strongly typed interface here instead of any. Not a merge-stopper though
There was a problem hiding this comment.
No need to introduce an interface for an object that's created and used inside a single method.
declarations.d.ts
Outdated
| * @param {number} inteval @optional The amount of time between each check. | ||
| * @returns {boolean} true in case port is in LISTEN state, false otherwise. | ||
| */ | ||
| waitForPortToListen(port: number, timeout: number, interval?: number): Promise<boolean>; |
There was a problem hiding this comment.
You can optionally introduce an interface here instead of accepting three different arguments
rosen-vladimirov
force-pushed
the
vladimirov/fix-sim-debug-brk
branch
from
717b5f4 to
c3e7d93
Compare
Add method in net service to check if port is in LISTEN state. Add tests for it. The method has different implementation for macOS, Linux and Windows - added tests for each of the implementations.
rosen-vladimirov
force-pushed
the
vladimirov/fix-sim-debug-brk
branch
from
c3e7d93 to
a3beed4
Compare
|
ping @Mitko-Kerezov , comments are fixed |
Add method in net service to check if port is in LISTEN state. Add tests for it.
The method has different implementation for macOS, Linux and Windows - added tests for each of the implementations.