Patchwork [001,of,179,tests-refactor] run-tests: create classes for representing tests

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

Comments

Gregory Szorc - May 2, 2014, 6:37 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1397935594 25200
#      Sat Apr 19 12:26:34 2014 -0700
# Branch stable
# Node ID 7dd9be16585bd36596a2f65358d36c5b039a7092
# Parent  cadad384c97c7956c98f3c9b92d8cc40fa16d93b
run-tests: create classes for representing tests

Currently, the state for an individual test is scattered across a number
of functions and variables. This patch begins a process of isolating a
single test's state into instances of a class. It does this by
establishing a new Test base class and child classes for Python tests
and T tests. The class currently has a run() API that proxies into the
existing "runner" functions. Upcoming patches will move the logic for
each test type into the class.
Pierre-Yves David - May 2, 2014, 6:45 p.m.
> [PATCH 001 of 179 tests-refactor] run-tests: create classes for representing tests

Did you really patch bombed 179 patch in one go?

If so, that is super wrong, please keep the number of inflight patches 
low (patchbomb series of around 5 changesets).
Gregory Szorc - May 2, 2014, 7:49 p.m.
On 5/2/2014 11:45 AM, Pierre-Yves David wrote:
>> [PATCH 001 of 179 tests-refactor] run-tests: create classes for
>> representing tests
> 
> Did you really patch bombed 179 patch in one go?
> 
> If so, that is super wrong, please keep the number of inflight patches
> low (patchbomb series of around 5 changesets).
> 

Well, only 85 right now because my smtp server dropped the connection
mid send and patchbomb didn't recover from that gracefully ;)

I was pretty diligent about making commits small. Most of these patches
require little cognitive ability to review. However, I wanted to have
the whole series available so reviewers could see the full direction
things are moving in.

Since you raised objections both here and in IRC, I'll hold off sending
the remaining patches until these are upstreamed.

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -572,24 +572,42 @@  def outputcoverage(options):
         htmldir = os.path.join(TESTDIR, 'htmlcov')
         covrun('-i', '-b', '"--directory=%s"' % htmldir, '"--omit=%s"' % omit)
     if options.annotate:
         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):
+        self._path = path
+        self._options = options
+
+    def run(self, testtmp, replacements, env):
+        return self._run(testtmp, replacements, env)
+
+    def _run(self, testtmp, replacements, env):
+        raise NotImplemented('Subclasses must implement Test.run()')
+
 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)
 
+class PythonTest(Test):
+    """A Python-based test."""
+    def _run(self, testtmp, replacements, env):
+        return pytest(self._path, testtmp, self._options, replacements, env)
+
 needescape = re.compile(r'[\x00-\x08\x0b-\x1f\x7f-\xff]').search
 escapesub = re.compile(r'[\x00-\x08\x0b-\x1f\\\x7f-\xff]').sub
 escapemap = dict((chr(i), r'\x%02x' % i) for i in range(256))
 escapemap.update({'\\': '\\\\', '\r': r'\r'})
 def escapef(m):
     return escapemap[m.group(0)]
 def stringescape(s):
     return escapesub(escapef, s)
@@ -836,16 +854,22 @@  def tsttest(test, wd, options, replaceme
 
     if pos in after:
         postout += after.pop(pos)
 
     if warnonly == 2:
         exitcode = False # set exitcode to warned
     return exitcode, postout
 
+class TTest(Test):
+    """A "t test" is a test backed by a .t file."""
+
+    def _run(self, testtmp, replacements, env):
+        return tsttest(self._path, testtmp, self._options, replacements, env)
+
 wifexited = getattr(os, "WIFEXITED", lambda x: False)
 def run(cmd, wd, options, replacements, env):
     """Run command in a sub-process, capturing the output (stdout and stderr).
     Return a tuple (exitcode, output).  output is None in debug mode."""
     # TODO: Use subprocess.Popen if we're running on Python 2.4
     if options.debug:
         proc = subprocess.Popen(cmd, shell=True, cwd=wd, env=env)
         ret = proc.wait()
@@ -945,29 +969,31 @@  def runone(options, test, count):
             for k in options.keywords.lower().split():
                 if k in t:
                     break
                 else:
                     return ignore("doesn't match keyword")
 
     if not os.path.basename(lctest).startswith("test-"):
         return skip("not a test file")
-    for ext, func, out in testtypes:
+    for ext, cls, out in testtypes:
         if lctest.endswith(ext):
-            runner = func
+            runner = cls
             ref = os.path.join(TESTDIR, test + out)
             break
     else:
         return skip("unknown test type")
 
     vlog("# Test", test)
 
     if os.path.exists(err):
         os.remove(err)       # Remove any previous output files
 
+    t = runner(testpath, options)
+
     # Make a tmp subdirectory to work in
     threadtmp = os.path.join(HGTMP, "child%d" % count)
     testtmp = os.path.join(threadtmp, os.path.basename(test))
     os.mkdir(threadtmp)
     os.mkdir(testtmp)
 
     port = options.port + count * 3
     replacements = [
@@ -985,17 +1011,17 @@  def runone(options, test, count):
     else:
         replacements.append((re.escape(testtmp), '$TESTTMP'))
 
     env = createenv(options, testtmp, threadtmp, port)
     createhgrc(env['HGRCPATH'], options)
 
     starttime = time.time()
     try:
-        ret, out = runner(testpath, testtmp, options, replacements, env)
+        ret, out = t.run(testtmp, replacements, 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)
 
@@ -1185,18 +1211,18 @@  def runtests(options, tests):
         failed = True
         print "\ninterrupted!"
 
     if failed:
         return 1
     if warned:
         return 80
 
-testtypes = [('.py', pytest, '.out'),
-             ('.t', tsttest, '')]
+testtypes = [('.py', PythonTest, '.out'),
+             ('.t', TTest, '')]
 
 def main(args, parser=None):
     parser = parser or getparser()
     (options, args) = parseargs(args, parser)
     os.umask(022)
 
     checktools()