<div dir="ltr">On Thu, May 7, 2015 at 8:00 PM, Pierre-Yves David <span dir="ltr"><<a href="mailto:pierre-yves.david@ens-lyon.org" target="_blank">pierre-yves.david@ens-lyon.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
On 05/07/2015 07:37 PM, Gregory Szorc wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
On Thu, May 7, 2015 at 6:21 PM, Pierre-Yves David<br></span>
<<a href="mailto:pierre-yves.david@ens-lyon.org" target="_blank">pierre-yves.david@ens-lyon.org</a> <mailto:<a href="mailto:pierre-yves.david@ens-lyon.org" target="_blank">pierre-yves.david@ens-lyon.org</a>>><span class=""><br>
wrote:<br>
<br>
# HG changeset patch<br>
# User Pierre-Yves David <<a href="mailto:pierre-yves.david@fb.com" target="_blank">pierre-yves.david@fb.com</a><br></span>
<mailto:<a href="mailto:pierre-yves.david@fb.com" target="_blank">pierre-yves.david@fb.com</a>>><div><div class="h5"><br>
# Date 1431044040 25200<br>
# Thu May 07 17:14:00 2015 -0700<br>
# Node ID c25b2adb3664cd3c488e2c53aab0c64100d40af7<br>
# Parent 17ba4ccd20b48511b3d06ab47fb1b2faf31410d7<br>
run-test: ensure the test port are available before launching test<br>
<br>
I'm running into systematic issue because there is always some port<br>
taken in the<br>
1500 wide range of port used by the test (3 for each test file).<br>
<br>
diff --git a/tests/run-tests.py b/tests/run-tests.py<br>
--- a/tests/run-tests.py<br>
+++ b/tests/run-tests.py<br>
@@ -47,10 +47,11 @@ import errno<br>
import optparse<br>
import os<br>
import shutil<br>
import subprocess<br>
import signal<br>
+import socket<br>
import sys<br>
import tempfile<br>
import time<br>
import random<br>
import re<br>
@@ -76,10 +77,22 @@ processlock = threading.Lock()<br>
if sys.version_info < (2, 5):<br>
subprocess._cleanup = lambda: None<br>
<br>
wifexited = getattr(os, "WIFEXITED", lambda x: False)<br>
<br>
+def checkportisavailable(port):<br>
+ """return true is nobody seem a port seems free to bind on<br>
localhost"""<br>
+ try:<br>
+ s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)<br>
+ s.bind(('localhost', port))<br>
+ s.close()<br>
+ return True<br>
+ except socket.error, exc:<br>
+ if not exc.errno == errno.EADDRINUSE:<br>
+ raise<br>
+ return False<br>
+<br></div></div>
closefds = <a href="http://os.name" target="_blank">os.name</a> <<a href="http://os.name" target="_blank">http://os.name</a>> == 'posix'<div><div class="h5"><br>
def Popen4(cmd, wd, timeout, env=None):<br>
processlock.acquire()<br>
p = subprocess.Popen(cmd, shell=True, bufsize=-1, cwd=wd, env=env,<br>
close_fds=closefds,<br>
@@ -1601,10 +1614,12 @@ class TestRunner(object):<br>
self._tmpbinddir = None<br>
self._pythondir = None<br>
self._coveragefile = None<br>
self._createdfiles = []<br>
self._hgpath = None<br>
+ self._portoffset = 0<br>
+ self._ports = {}<br>
<br>
def run(self, args, parser=None):<br>
"""Run the test suite."""<br>
oldmask = os.umask(022)<br>
try:<br>
@@ -1805,10 +1820,28 @@ class TestRunner(object):<br>
if failed:<br>
return 1<br>
if warned:<br>
return 80<br>
<br>
+ def _getport(self, count):<br>
+ port = self._ports.get(count) # do we have a cached entry?<br>
+ if port is None:<br>
+ port = self.options.port + self._portoffset<br>
+ portneeded = 3<br>
+ # above 100 tries we just give up and let test reports<br>
failure<br>
+ for tries in xrange(100):<br>
+ allfree = True<br>
+ for idx in xrange(portneeded):<br>
+ if not checkportisavailable(port + idx):<br>
<br>
+ allfree = False<br>
+ break<br>
+ self._portoffset += portneeded<br>
+ if allfree:<br>
+ break<br>
+ self._ports[count] = port<br>
+ return port<br>
+<br>
<br>
<br>
This is hard to read. Part of me thinks there is a race condition here.<br>
The offset management in particular has me worried.<br>
</div></div></blockquote>
<br>
The test (therefore this function) is called in the main thread before we start using thread so no race condition should happen.<span class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Ideally, tests would bind to port 0 and let the OS choose an existing<br>
port. That's the only guaranteed way to prevent race conditions. But<br>
then we'd need to parse port numbers out of command output so they could<br>
be used in the rest of tests and we'd need a mechanism to register<br>
replacements in test output. Scope boat.<br>
</blockquote>
<br></span>
Would could have variable management inside the test, but I expect headache from it.<span class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
If you are going to rewrite port selection, I'd almost prefer having the<br>
test harness choose random numbers between 1024 and 65535, attempt to<br>
bind, and assign that. This seems simple and effective. There is no need<br>
to maintain state: just try binding in a loop until it works. Is this<br>
solution too simple? Is user control over the port range really that<br>
important? We're not talking about a production service here...<br>
</blockquote>
<br></span>
Choosing random number for each test does not guarantee test will not choose overlapping port number. The math says there is more than 99% change it will happen.<br></blockquote><div><br></div><div>Err, yeah.<br><br></div><div>Have the test runner choose N random ports when the test executes. Keep track of which ports are reserved. Refuse to try those until the test finishes executing. That is doable. And it doesn't require nested range loops. <br></div></div></div></div>