test: prevent apptest from choosing ports in use #12580
Conversation
Apptest requires a port without a listening server to test failure cases. This port was chosen and had a chance of actually being provisioned. To prevent this accident, a port <1k is chosen, since those will never be allocated.
Emyrk
changed the title
| // Ports <1k will never be selected. 396 is for some old OS over IP. | ||
| // It will never likely be provisioned. Using quick timeout since | ||
| // it's all localhost | ||
| fakeAppURL := "http://127.1.0.1:396" |
There was a problem hiding this comment.
AFAIK ports <1024 are privileged.
Does the test suite run as root?
There was a problem hiding this comment.
They do not. We do not want a listener on this port. So the fact they are privileged is what I am hoping for 👍
There was a problem hiding this comment.
Essentially we want a port that will never be listened on in our tests. If we listen on port 0, then read the port and close the listener, there's a potential that another test will take that port that we hope nothing will listen on.
Using a port < 1024 and on an uncommon app as the URL for a "not running" app should serve our use case here. We're just dialing it to make sure it's not in use and then failing fast if it is.
There was a problem hiding this comment.
It's unfortunate, but to do this in a less potentially flakey way we would need to stub out the dialer somehow. In CI and on dev machines though I don't expect this port to ever be in use so I doubt we'll ever hit this problem
There was a problem hiding this comment.
Nice, thanks for the explanation 👍
Apptest requires a port without a listening server to test failure cases. This port was chosen and had a chance of actually being provisioned. To prevent this accident, a port <1k is chosen, since those will never be allocated.
Closes 2/3 of #12531 (comment)
One of the flakes (#12531 (comment)) looks different.
I am able to reproduce the exact error if I spin up a server on
65535. I think this fix should prevent this in future.