Patchwork [5,of,6] run-tests: obtain replacements inside Test._runcommand

login
register
mail settings
Submitter Gregory Szorc
Date March 29, 2015, 3:15 a.m.
Message ID <f62a75b235644da799d0.1427598910@vm-ubuntu-main.gateway.sonic.net>
Download mbox | patch
Permalink /patch/8340/
State Accepted
Headers show

Comments

Gregory Szorc - March 29, 2015, 3:15 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1427579728 25200
#      Sat Mar 28 14:55:28 2015 -0700
# Node ID f62a75b235644da799d0f43e492e55aca2e66206
# Parent  ef971bec3244987328d272ee93e81a691509d63a
run-tests: obtain replacements inside Test._runcommand

Now that command running is part of Test, we no longer need to pass
a list of replacements down through various call layers.

The impetus for this change is to fetch replacements after
command execution, not before. This will allow replacements to be
defined as part of test execution.
Augie Fackler - March 30, 2015, 6:46 p.m.
On Sat, Mar 28, 2015 at 08:15:10PM -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1427579728 25200
> #      Sat Mar 28 14:55:28 2015 -0700
> # Node ID f62a75b235644da799d0f43e492e55aca2e66206
> # Parent  ef971bec3244987328d272ee93e81a691509d63a
> run-tests: obtain replacements inside Test._runcommand

I'm happy enough with the resulting code cleanup here that I've queued
this anyway.

>
> Now that command running is part of Test, we no longer need to pass
> a list of replacements down through various call layers.
>
> The impetus for this change is to fetch replacements after
> command execution, not before. This will allow replacements to be
> defined as part of test execution.
>
> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -542,16 +542,15 @@ class Test(unittest.TestCase):
>          """Run this test instance.
>
>          This will return a tuple describing the result of the test.
>          """
> -        replacements = self._getreplacements()
>          env = self._getenv()
>          self._daemonpids.append(env['DAEMON_PIDS'])
>          self._createhgrc(env['HGRCPATH'])
>
>          vlog('# Test', self.name)
>
> -        ret, out = self._run(replacements, env)
> +        ret, out = self._run(env)
>          self._finished = True
>          self._ret = ret
>          self._out = out
>
> @@ -622,9 +621,9 @@ class Test(unittest.TestCase):
>              f.close()
>
>          vlog("# Ret was:", self._ret)
>
> -    def _run(self, replacements, env):
> +    def _run(self, env):
>          # This should be implemented in child classes to run tests.
>          raise SkipTest('unknown test type')
>
>      def abort(self):
> @@ -721,9 +720,9 @@ class Test(unittest.TestCase):
>          # unittest differentiates between errored and failed.
>          # Failed is denoted by AssertionError (by default at least).
>          raise AssertionError(msg)
>
> -    def _runcommand(self, cmd, replacements, env, normalizenewlines=False):
> +    def _runcommand(self, cmd, env, normalizenewlines=False):
>          """Run command in a sub-process, capturing the output (stdout and
>          stderr).
>
>          Return a tuple (exitcode, output). output is None in debug mode.
> @@ -762,9 +761,9 @@ class Test(unittest.TestCase):
>
>          if ret:
>              killdaemons(env['DAEMON_PIDS'])
>
> -        for s, r in replacements:
> +        for s, r in self._getreplacements():
>              output = re.sub(s, r, output)
>
>          if normalizenewlines:
>              output = output.replace('\r\n', '\n')
> @@ -777,14 +776,14 @@ class PythonTest(Test):
>      @property
>      def refpath(self):
>          return os.path.join(self._testdir, '%s.out' % self.name)
>
> -    def _run(self, replacements, env):
> +    def _run(self, env):
>          py3kswitch = self._py3kwarnings and ' -3' or ''
>          cmd = '%s%s "%s"' % (PYTHON, py3kswitch, self.path)
>          vlog("# Running", cmd)
>          normalizenewlines = os.name == 'nt'
> -        result = self._runcommand(cmd, replacements, env,
> +        result = self._runcommand(cmd, env,
>                                    normalizenewlines=normalizenewlines)
>          if self._aborted:
>              raise KeyboardInterrupt()
>
> @@ -813,9 +812,9 @@ class TTest(Test):
>      @property
>      def refpath(self):
>          return os.path.join(self._testdir, self.name)
>
> -    def _run(self, replacements, env):
> +    def _run(self, env):
>          f = open(self.path, 'rb')
>          lines = f.readlines()
>          f.close()
>
> @@ -830,9 +829,9 @@ class TTest(Test):
>
>          cmd = '%s "%s"' % (self._shell, fname)
>          vlog("# Running", cmd)
>
> -        exitcode, output = self._runcommand(cmd, replacements, env)
> +        exitcode, output = self._runcommand(cmd, env)
>
>          if self._aborted:
>              raise KeyboardInterrupt()
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -542,16 +542,15 @@  class Test(unittest.TestCase):
         """Run this test instance.
 
         This will return a tuple describing the result of the test.
         """
-        replacements = self._getreplacements()
         env = self._getenv()
         self._daemonpids.append(env['DAEMON_PIDS'])
         self._createhgrc(env['HGRCPATH'])
 
         vlog('# Test', self.name)
 
-        ret, out = self._run(replacements, env)
+        ret, out = self._run(env)
         self._finished = True
         self._ret = ret
         self._out = out
 
@@ -622,9 +621,9 @@  class Test(unittest.TestCase):
             f.close()
 
         vlog("# Ret was:", self._ret)
 
-    def _run(self, replacements, env):
+    def _run(self, env):
         # This should be implemented in child classes to run tests.
         raise SkipTest('unknown test type')
 
     def abort(self):
@@ -721,9 +720,9 @@  class Test(unittest.TestCase):
         # unittest differentiates between errored and failed.
         # Failed is denoted by AssertionError (by default at least).
         raise AssertionError(msg)
 
-    def _runcommand(self, cmd, replacements, env, normalizenewlines=False):
+    def _runcommand(self, cmd, env, normalizenewlines=False):
         """Run command in a sub-process, capturing the output (stdout and
         stderr).
 
         Return a tuple (exitcode, output). output is None in debug mode.
@@ -762,9 +761,9 @@  class Test(unittest.TestCase):
 
         if ret:
             killdaemons(env['DAEMON_PIDS'])
 
-        for s, r in replacements:
+        for s, r in self._getreplacements():
             output = re.sub(s, r, output)
 
         if normalizenewlines:
             output = output.replace('\r\n', '\n')
@@ -777,14 +776,14 @@  class PythonTest(Test):
     @property
     def refpath(self):
         return os.path.join(self._testdir, '%s.out' % self.name)
 
-    def _run(self, replacements, env):
+    def _run(self, env):
         py3kswitch = self._py3kwarnings and ' -3' or ''
         cmd = '%s%s "%s"' % (PYTHON, py3kswitch, self.path)
         vlog("# Running", cmd)
         normalizenewlines = os.name == 'nt'
-        result = self._runcommand(cmd, replacements, env,
+        result = self._runcommand(cmd, env,
                                   normalizenewlines=normalizenewlines)
         if self._aborted:
             raise KeyboardInterrupt()
 
@@ -813,9 +812,9 @@  class TTest(Test):
     @property
     def refpath(self):
         return os.path.join(self._testdir, self.name)
 
-    def _run(self, replacements, env):
+    def _run(self, env):
         f = open(self.path, 'rb')
         lines = f.readlines()
         f.close()
 
@@ -830,9 +829,9 @@  class TTest(Test):
 
         cmd = '%s "%s"' % (self._shell, fname)
         vlog("# Running", cmd)
 
-        exitcode, output = self._runcommand(cmd, replacements, env)
+        exitcode, output = self._runcommand(cmd, env)
 
         if self._aborted:
             raise KeyboardInterrupt()