[PATCH stable?] run-test: ensure the test port are available before launching test
Pierre-Yves David
pierre-yves.david at ens-lyon.org
Thu May 7 22:00:51 CDT 2015
On 05/07/2015 07:37 PM, Gregory Szorc wrote:
> On Thu, May 7, 2015 at 6:21 PM, Pierre-Yves David
> <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>>
> wrote:
>
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at fb.com
> <mailto:pierre-yves.david at fb.com>>
> # Date 1431044040 25200
> # Thu May 07 17:14:00 2015 -0700
> # Node ID c25b2adb3664cd3c488e2c53aab0c64100d40af7
> # Parent 17ba4ccd20b48511b3d06ab47fb1b2faf31410d7
> run-test: ensure the test port are available before launching test
>
> I'm running into systematic issue because there is always some port
> taken in the
> 1500 wide range of port used by the test (3 for each test file).
>
> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -47,10 +47,11 @@ import errno
> import optparse
> import os
> import shutil
> import subprocess
> import signal
> +import socket
> import sys
> import tempfile
> import time
> import random
> import re
> @@ -76,10 +77,22 @@ processlock = threading.Lock()
> if sys.version_info < (2, 5):
> subprocess._cleanup = lambda: None
>
> wifexited = getattr(os, "WIFEXITED", lambda x: False)
>
> +def checkportisavailable(port):
> + """return true is nobody seem a port seems free to bind on
> localhost"""
> + try:
> + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> + s.bind(('localhost', port))
> + s.close()
> + return True
> + except socket.error, exc:
> + if not exc.errno == errno.EADDRINUSE:
> + raise
> + return False
> +
> closefds = os.name <http://os.name> == 'posix'
> def Popen4(cmd, wd, timeout, env=None):
> processlock.acquire()
> p = subprocess.Popen(cmd, shell=True, bufsize=-1, cwd=wd, env=env,
> close_fds=closefds,
> @@ -1601,10 +1614,12 @@ class TestRunner(object):
> self._tmpbinddir = None
> self._pythondir = None
> self._coveragefile = None
> self._createdfiles = []
> self._hgpath = None
> + self._portoffset = 0
> + self._ports = {}
>
> def run(self, args, parser=None):
> """Run the test suite."""
> oldmask = os.umask(022)
> try:
> @@ -1805,10 +1820,28 @@ class TestRunner(object):
> if failed:
> return 1
> if warned:
> return 80
>
> + def _getport(self, count):
> + port = self._ports.get(count) # do we have a cached entry?
> + if port is None:
> + port = self.options.port + self._portoffset
> + portneeded = 3
> + # above 100 tries we just give up and let test reports
> failure
> + for tries in xrange(100):
> + allfree = True
> + for idx in xrange(portneeded):
> + if not checkportisavailable(port + idx):
>
> + allfree = False
> + break
> + self._portoffset += portneeded
> + if allfree:
> + break
> + self._ports[count] = port
> + return port
> +
>
>
> This is hard to read. Part of me thinks there is a race condition here.
> The offset management in particular has me worried.
The test (therefore this function) is called in the main thread before
we start using thread so no race condition should happen.
> Ideally, tests would bind to port 0 and let the OS choose an existing
> port. That's the only guaranteed way to prevent race conditions. But
> then we'd need to parse port numbers out of command output so they could
> be used in the rest of tests and we'd need a mechanism to register
> replacements in test output. Scope boat.
Would could have variable management inside the test, but I expect
headache from it.
> If you are going to rewrite port selection, I'd almost prefer having the
> test harness choose random numbers between 1024 and 65535, attempt to
> bind, and assign that. This seems simple and effective. There is no need
> to maintain state: just try binding in a loop until it works. Is this
> solution too simple? Is user control over the port range really that
> important? We're not talking about a production service here...
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.
>
> def _gettest(self, test, count):
> """Obtain a Test by looking at its filename.
>
> Returns a Test instance. The Test may not be runnable if
> it doesn't
> map to a known type.
> @@ -1826,11 +1859,11 @@ class TestRunner(object):
>
> t = testcls(refpath, tmpdir,
> keeptmpdir=self.options.keep_tmpdir,
> debug=self.options.debug,
> timeout=self.options.timeout,
> - startport=self.options.port + count * 3,
> + startport=self._getport(count),
> extraconfigopts=self.options.extra_config_opt,
> py3kwarnings=self.options.py3k_warnings,
> shell=self.options.shell)
> t.should_reload = True
> return t
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com <mailto:Mercurial-devel at selenic.com>
> http://selenic.com/mailman/listinfo/mercurial-devel
>
>
--
Pierre-Yves David
More information about the Mercurial-devel
mailing list