Patchwork [006,of,179,tests-refactor] run-tests: move replacements and port management into Test

login
register
mail settings
Submitter Gregory Szorc
Date May 2, 2014, 6:37 p.m.
Message ID <5c920a6f7ff39411402a.1399055843@vm-ubuntu-main.gateway.sonic.net>
Download mbox | patch
Permalink /patch/4501/
State Accepted
Commit a77f4c2e18987c0900fe72e872f41826fc35b1ef
Headers show

Comments

Gregory Szorc - May 2, 2014, 6:37 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1397938735 25200
#      Sat Apr 19 13:18:55 2014 -0700
# Branch stable
# Node ID 5c920a6f7ff39411402ab7910db7c1520ca550fb
# Parent  27f7274d1694f92571e68c63696cead7658271eb
run-tests: move replacements and port management into Test

replacements and ports are really implementation details of a Test. They
have been moved to instance variables.
Pierre-Yves David - May 7, 2014, 8:39 p.m.
On 05/02/2014 11:37 AM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1397938735 25200
> #      Sat Apr 19 13:18:55 2014 -0700
> # Branch stable
> # Node ID 5c920a6f7ff39411402ab7910db7c1520ca550fb
> # Parent  27f7274d1694f92571e68c63696cead7658271eb
> run-tests: move replacements and port management into Test
>
> replacements and ports are really implementation details of a Test. They
> have been moved to instance variables.
>
> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -540,57 +540,60 @@ def outputcoverage(options):
>           adir = os.path.join(TESTDIR, 'annotated')
>           if not os.path.isdir(adir):
>               os.mkdir(adir)
>           covrun('-i', '-a', '"--directory=%s"' % adir, '"--omit=%s"' % omit)
>
>   class Test(object):
>       """Encapsulates a single, runnable test."""
>
> -    def __init__(self, path, options, threadtmp):
> +    def __init__(self, path, options, threadtmp, count):

New undocumented variable. Pointing out the fact this __init__ method 
have not docstring. Can you add one?

>           self._path = path
>           self._options = options
>           self._threadtmp = threadtmp

It probably starts making sense to document each attributes too.

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -540,57 +540,60 @@  def outputcoverage(options):
         adir = os.path.join(TESTDIR, 'annotated')
         if not os.path.isdir(adir):
             os.mkdir(adir)
         covrun('-i', '-a', '"--directory=%s"' % adir, '"--omit=%s"' % omit)
 
 class Test(object):
     """Encapsulates a single, runnable test."""
 
-    def __init__(self, path, options, threadtmp):
+    def __init__(self, path, options, threadtmp, count):
         self._path = path
         self._options = options
         self._threadtmp = threadtmp
 
         self.testtmp = os.path.join(threadtmp, os.path.basename(path))
         os.mkdir(self.testtmp)
 
-    def run(self, replacements, env):
+        self._setreplacements(count)
+
+    def run(self, env):
         createhgrc(env['HGRCPATH'], self._options)
 
-        return self._run(replacements, env)
+        return self._run(self._replacements, env)
 
     def _run(self, replacements, env):
         raise NotImplemented('Subclasses must implement Test.run()')
 
-    def getreplacements(self, count):
+    def _setreplacements(self, count):
         port = self._options.port + count * 3
         r = [
             (r':%s\b' % port, ':$HGPORT'),
             (r':%s\b' % (port + 1), ':$HGPORT1'),
             (r':%s\b' % (port + 2), ':$HGPORT2'),
             ]
 
         if os.name == 'nt':
             r.append(
                 (''.join(c.isalpha() and '[%s%s]' % (c.lower(), c.upper()) or
                     c in '/\\' and r'[/\\]' or c.isdigit() and c or '\\' + c
                     for c in self.testtmp), '$TESTTMP'))
         else:
             r.append((re.escape(self.testtmp), '$TESTTMP'))
 
-        return r, port
+        self._replacements = r
+        self._port = port
 
-    def getenv(self, port):
+    def getenv(self):
         env = os.environ.copy()
         env['TESTTMP'] = self.testtmp
         env['HOME'] = self.testtmp
-        env["HGPORT"] = str(port)
-        env["HGPORT1"] = str(port + 1)
-        env["HGPORT2"] = str(port + 2)
+        env["HGPORT"] = str(self._port)
+        env["HGPORT1"] = str(self._port + 1)
+        env["HGPORT2"] = str(self._port + 2)
         env["HGRCPATH"] = os.path.join(self._threadtmp, '.hgrc')
         env["DAEMON_PIDS"] = os.path.join(self._threadtmp, 'daemon.pids')
         env["HGEDITOR"] = sys.executable + ' -c "import sys; sys.exit(0)"'
         env["HGMERGE"] = "internal:merge"
         env["HGUSER"]   = "test"
         env["HGENCODING"] = "ascii"
         env["HGENCODINGMODE"] = "strict"
 
@@ -1012,23 +1015,22 @@  def runone(options, test, count):
 
     if os.path.exists(err):
         os.remove(err)       # Remove any previous output files
 
     # Make a tmp subdirectory to work in
     threadtmp = os.path.join(HGTMP, "child%d" % count)
     os.mkdir(threadtmp)
 
-    t = runner(testpath, options, threadtmp)
-    replacements, port = t.getreplacements(count)
-    env = t.getenv(port)
+    t = runner(testpath, options, threadtmp, count)
+    env = t.getenv()
 
     starttime = time.time()
     try:
-        ret, out = t.run(replacements, env)
+        ret, out = t.run(env)
     except KeyboardInterrupt:
         endtime = time.time()
         log('INTERRUPTED: %s (after %d seconds)' % (test, endtime - starttime))
         raise
     endtime = time.time()
     times.append((test, endtime - starttime))
     vlog("# Ret was:", ret)