Patchwork [003,of,179,tests-refactor] run-tests: move replacements generation into Test

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

Comments

Gregory Szorc - May 2, 2014, 6:37 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1397937804 25200
#      Sat Apr 19 13:03:24 2014 -0700
# Branch stable
# Node ID b5da54671ef0204c48ce54715aa547cb1766fb7d
# Parent  b81c28c2cb25a35e899faf379b3f139dc12cbd66
run-tests: move replacements generation into Test

The API is a bit funky. Things will look better once all state is
captured in Test.
Pierre-Yves David - May 7, 2014, 8:38 p.m.
On 05/02/2014 11:37 AM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1397937804 25200
> #      Sat Apr 19 13:03:24 2014 -0700
> # Branch stable
> # Node ID b5da54671ef0204c48ce54715aa547cb1766fb7d
> # Parent  b81c28c2cb25a35e899faf379b3f139dc12cbd66
> run-tests: move replacements generation into Test
>
> The API is a bit funky. Things will look better once all state is
> captured in Test.
>
> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -588,16 +588,34 @@ class Test(object):
>           os.mkdir(self.testtmp)
>
>       def run(self, replacements, env):
>           return self._run(replacements, env)
>
>       def _run(self, replacements, env):
>           raise NotImplemented('Subclasses must implement Test.run()')
>
> +    def getreplacements(self, count):

Now that you injected some understanding of what this step is. We need 
to extract it for future people looking at this code (including you 
future self).

In other word: this methods needs a docstring.

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -588,16 +588,34 @@  class Test(object):
         os.mkdir(self.testtmp)
 
     def run(self, replacements, env):
         return self._run(replacements, env)
 
     def _run(self, replacements, env):
         raise NotImplemented('Subclasses must implement Test.run()')
 
+    def getreplacements(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
+
 def pytest(test, wd, options, replacements, env):
     py3kswitch = options.py3k_warnings and ' -3' or ''
     cmd = '%s%s "%s"' % (PYTHON, py3kswitch, test)
     vlog("# Running", cmd)
     if os.name == 'nt':
         replacements.append((r'\r\n', '\n'))
     return run(cmd, wd, options, replacements, env)
 
@@ -992,33 +1010,17 @@  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)
-
-    port = options.port + count * 3
-    replacements = [
-        (r':%s\b' % port, ':$HGPORT'),
-        (r':%s\b' % (port + 1), ':$HGPORT1'),
-        (r':%s\b' % (port + 2), ':$HGPORT2'),
-        ]
-    if os.name == 'nt':
-        replacements.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 t.testtmp), '$TESTTMP'))
-    else:
-        replacements.append((re.escape(t.testtmp), '$TESTTMP'))
-
+    replacements, port = t.getreplacements(count)
     env = createenv(options, t.testtmp, threadtmp, port)
     createhgrc(env['HGRCPATH'], options)
 
     starttime = time.time()
     try:
         ret, out = t.run(replacements, env)
     except KeyboardInterrupt:
         endtime = time.time()