Skip to content

Commit

Permalink
Upgrade the python task tests to use TaskTestBase.
Browse files Browse the repository at this point in the history
1) Upgrades the PythonTaskTest to extend TaskTestBase instead of TaskTest.
2) Make all relevant tests subclass PythonTaskTest.
3) Move a bunch of common code into that base class.

I removed some rather hacky code whose ostensible purpose was to
make the code under test use the test-invoking code's InterpreterCache.
It would have been hard to make this code work under the new scheme,
but in any case, no benchmarks I ran could detect any performance
difference before and after. Even if I kept the old code but just
commented out that hack I could see no performance difference.

Let's revisit this performance issue if it crops up, or feel free
to show me that this is necessary and I'll figure something out.

Testing Done:
CI passes: https://travis-ci.org/pantsbuild/pants/builds/57187738

Reviewed at https://rbcommons.com/s/twitter/r/2031/
  • Loading branch information
benjyw authored and Benjy committed Apr 6, 2015
1 parent 63fc8ed commit 2d7fbf2
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 157 deletions.
13 changes: 5 additions & 8 deletions tests/python/pants_test/backend/python/tasks/BUILD
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
target(
name='tasks',
dependencies=[
':pytest_run',
':python_eval',
':python_repl',
':setup_py',
Expand All @@ -11,26 +12,24 @@ python_library(
name='python_task_test',
sources=['python_task_test.py'],
dependencies=[
'src/python/pants/backend/python/targets:python',
'src/python/pants/backend/python:plugin',
'src/python/pants/base:address',
'src/python/pants/base:build_file_aliases',
'src/python/pants/util:dirutil',
'tests/python/pants_test/tasks:base'
'tests/python/pants_test:task_test_base'
]
)

python_tests(
name='pytest_run',
sources=['test_pytest_run.py'],
dependencies=[
':python_task_test',
'3rdparty/python:coverage',
'3rdparty/python:pex',
'src/python/pants/backend/python:python_setup',
'src/python/pants/backend/python/targets:python',
'src/python/pants/backend/python/tasks:python',
'src/python/pants/base:build_file_aliases',
'src/python/pants/util:contextutil',
'tests/python/pants_test:task_test_base'
]
)

Expand Down Expand Up @@ -68,14 +67,12 @@ python_tests(
name='setup_py',
sources=['test_setup_py.py'],
dependencies=[
':python_task_test',
'3rdparty/python/twitter/commons:twitter.common.collections',
'3rdparty/python/twitter/commons:twitter.common.dirutil',
'3rdparty/python:mock',
'src/python/pants/backend/python:plugin',
'src/python/pants/backend/python/tasks:python',
'src/python/pants/base:address',
'src/python/pants/util:contextutil',
'src/python/pants/util:dirutil',
'tests/python/pants_test:task_test_base',
],
)
74 changes: 47 additions & 27 deletions tests/python/pants_test/backend/python/tasks/python_task_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,58 +6,78 @@
unicode_literals, with_statement)

import os
import shutil
from textwrap import dedent

from pants.backend.python.targets.python_binary import PythonBinary
from pants.backend.python.targets.python_library import PythonLibrary
from pants.backend.python.register import build_file_aliases as register_python
from pants.base.address import SyntheticAddress
from pants.base.build_file_aliases import BuildFileAliases
from pants.util.dirutil import safe_mkdir
from pants_test.tasks.test_base import TaskTest
from pants_test.task_test_base import TaskTestBase


# TODO(John Sirois): Convert to TaskTestBase - note this will require a good bit of option plumbing
# to get the main pants cache re-use for speed-ups bit in setUp below.
class PythonTaskTest(TaskTest):
class PythonTaskTest(TaskTestBase):
def setUp(self):
super(PythonTaskTest, self).setUp()

# Re-use the main pants python cache to speed up interpreter selection and artifact resolution.
safe_mkdir(os.path.join(self.build_root, '.pants.d'))
shutil.copytree(os.path.join(self.real_build_root, '.pants.d', 'python'),
os.path.join(self.build_root, '.pants.d', 'python'),
symlinks=True)
self.set_options_for_scope('', python_chroot_requirements_ttl=1000000000)

@property
def alias_groups(self):
return BuildFileAliases.create(targets={'python_library': PythonLibrary,
'python_binary': PythonBinary})
return register_python()

def create_python_library(self, relpath, name, source, contents, dependencies=()):
def create_python_library(self, relpath, name, source_contents_map=None,
dependencies=(), provides=None):
sources = ['__init__.py'] + source_contents_map.keys() if source_contents_map else None
sources_strs = ["'{0}'".format(s) for s in sources] if sources else None
self.create_file(relpath=self.build_path(relpath), contents=dedent("""
python_library(
name='{name}',
sources=['__init__.py', '{source}'],
{sources_clause}
dependencies=[
{dependencies}
]
],
{provides_clause}
)
""").format(name=name, source=source, dependencies=','.join(map(repr, dependencies))))

self.create_file(relpath=os.path.join(relpath, '__init__.py'))
self.create_file(relpath=os.path.join(relpath, source), contents=contents)
""").format(
name=name,
sources_clause='sources=[{0}],'.format(','.join(sources_strs)) if sources_strs else '',
dependencies=','.join(map(repr, dependencies)),
provides_clause='provides={0},'.format(provides) if provides else ''))
if source_contents_map:
self.create_file(relpath=os.path.join(relpath, '__init__.py'))
for source, contents in source_contents_map.items():
self.create_file(relpath=os.path.join(relpath, source), contents=contents)
return self.target(SyntheticAddress(relpath, name).spec)

def create_python_binary(self, relpath, name, entry_point, dependencies=()):
def create_python_binary(self, relpath, name, entry_point, dependencies=(), provides=None):
self.create_file(relpath=self.build_path(relpath), contents=dedent("""
python_binary(
name='{name}',
entry_point='{entry_point}',
dependencies=[
{dependencies}
]
],
{provides_clause}
)
""").format(name=name, entry_point=entry_point, dependencies=','.join(map(repr, dependencies))))
""").format(name=name, entry_point=entry_point, dependencies=','.join(map(repr, dependencies)),
provides_clause='provides={0},'.format(provides) if provides else ''))
return self.target(SyntheticAddress(relpath, name).spec)

def create_python_requirement_library(self, relpath, name, requirements):
def make_requirement(req):
return 'python_requirement("{}")'.format(req)

self.create_file(relpath=self.build_path(relpath), contents=dedent("""
python_requirement_library(
name='{name}',
requirements=[
{requirements}
]
)
""").format(name=name, requirements=','.join(map(make_requirement, requirements))))
return self.target(SyntheticAddress(relpath, name).spec)

def context(self, config='', options=None, target_roots=None, **kwargs):
# Our python tests don't pass on Python 3 yet.
# TODO: Clean up this hard-coded interpreter constraint once we have subsystems
# and can simplify InterpreterCache and PythonSetup.
self.set_options(interpreter=['CPython>=2.7,<3'])
return super(PythonTaskTest, self).context(config=config, options=options,
target_roots=target_roots, **kwargs)
21 changes: 3 additions & 18 deletions tests/python/pants_test/backend/python/tasks/test_pytest_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,18 @@

import coverage

from pants.backend.python.targets.python_library import PythonLibrary
from pants.backend.python.targets.python_tests import PythonTests
from pants.backend.python.tasks.pytest_run import PytestRun, PythonTestFailure
from pants.base.build_file_aliases import BuildFileAliases
from pants.util.contextutil import environment_as, pushd
from pants_test.task_test_base import TaskTestBase
from pants_test.backend.python.tasks.python_task_test import PythonTaskTest


class PythonTestBuilderTestBase(TaskTestBase):
class PythonTestBuilderTestBase(PythonTaskTest):
@classmethod
def task_type(cls):
return PytestRun

def setUp(self):
super(PythonTestBuilderTestBase, self).setUp()
self.set_options_for_scope('', python_chroot_requirements_ttl=1000000000)

def run_tests(self, targets):
options = {
# TODO: Clean up this hard-coded interpreter constraint once we have subsystems
# and can simplify InterpreterCache and PythonSetup.
'interpreter': ['CPython>=2.7,<3'], # These tests don't pass on Python 3 yet.
'colors': False,
'level': 'info' # When debugging a test failure it may be helpful to set this to 'debug'.
}
Expand All @@ -47,20 +37,15 @@ def run_failing_tests(self, targets):
with self.assertRaises(PythonTestFailure):
self.run_tests(targets=targets)


class PythonTestBuilderTestEmpty(PythonTestBuilderTestBase):
def test_empty(self):
self.run_tests(targets=[])


class PythonTestBuilderTest(PythonTestBuilderTestBase):
@property
def alias_groups(self):
return BuildFileAliases.create(targets={
'python_tests': PythonTests, 'python_library': PythonLibrary})

def setUp(self):
super(PythonTestBuilderTest, self).setUp()

self.create_file(
'lib/core.py',
dedent("""
Expand Down
55 changes: 28 additions & 27 deletions tests/python/pants_test/backend/python/tasks/test_python_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,33 +25,33 @@ def setUp(self):

SourceRoot.register('src', PythonBinary, PythonLibrary)

self.a_library = self.create_python_library('src/a', 'a', 'a.py', dedent("""
self.a_library = self.create_python_library('src/a', 'a', {'a.py': dedent("""
import inspect
def compile_time_check_decorator(cls):
if not inspect.isclass(cls):
raise TypeError('This decorator can only be applied to classes, given {}'.format(cls))
return cls
"""))
""")})

self.b_library = self.create_python_library('src/b', 'b', 'b.py', dedent("""
self.b_library = self.create_python_library('src/b', 'b', {'b.py': dedent("""
from a.a import compile_time_check_decorator
@compile_time_check_decorator
class BarB(object):
pass
"""))
""")})

self.b_library = self.create_python_library('src/c', 'c', 'c.py', dedent("""
self.b_library = self.create_python_library('src/c', 'c', {'c.py': dedent("""
from a.a import compile_time_check_decorator
@compile_time_check_decorator
def baz_c():
pass
"""), dependencies=['//src/a'])
""")}, dependencies=['//src/a'])

def fix_c_source():
self.create_file('src/c/c.py', contents=dedent("""
Expand All @@ -64,14 +64,14 @@ class BazC(object):
"""))
self.fix_c_source = fix_c_source

self.d_library = self.create_python_library('src/d', 'd', 'd.py', dedent("""
self.d_library = self.create_python_library('src/d', 'd', { 'd.py': dedent("""
from a.a import compile_time_check_decorator
@compile_time_check_decorator
class BazD(object):
pass
"""), dependencies=['//src/a'])
""")}, dependencies=['//src/a'])

self.e_binary = self.create_python_binary('src/e', 'e', 'a.a', dependencies=['//src/a'])
self.f_binary = self.create_python_binary('src/f', 'f', 'a.a:compile_time_check_decorator',
Expand All @@ -84,103 +84,104 @@ def tearDown(self):
super(PythonEvalTest, self).tearDown()
SourceRoot.reset()

def prepare_task(self, *args, **kwargs):
kwargs.update(build_graph=self.build_graph, build_file_parser=self.build_file_parser)
return super(PythonEvalTest, self).prepare_task(*args, **kwargs)
def _create_task(self, target_roots, options=None):
if options:
self.set_options(**options)
return self.create_task(self.context(target_roots=target_roots))

def test_noop(self):
python_eval = self.prepare_task(targets=[])
python_eval = self._create_task(target_roots=[])
compiled = python_eval.execute()
self.assertEqual([], compiled)

def test_compile(self):
python_eval = self.prepare_task(targets=[self.a_library])
python_eval = self._create_task(target_roots=[self.a_library])
compiled = python_eval.execute()
self.assertEqual([self.a_library], compiled)

def test_compile_incremental(self):
python_eval = self.prepare_task(targets=[self.a_library])
python_eval = self._create_task(target_roots=[self.a_library])
compiled = python_eval.execute()
self.assertEqual([self.a_library], compiled)

python_eval = self.prepare_task(targets=[self.a_library])
python_eval = self._create_task(target_roots=[self.a_library])
compiled = python_eval.execute()
self.assertEqual([], compiled)

def test_compile_closure(self):
python_eval = self.prepare_task(args=['--test-closure'], targets=[self.d_library])
python_eval = self._create_task(target_roots=[self.d_library], options={'closure': True})
compiled = python_eval.execute()
self.assertEqual({self.d_library, self.a_library}, set(compiled))

def test_compile_fail_closure(self):
python_eval = self.prepare_task(args=['--test-closure'], targets=[self.b_library])
python_eval = self._create_task(target_roots=[self.b_library], options={'closure': True})

with self.assertRaises(TaskError) as e:
python_eval.execute()
self.assertEqual([self.a_library], e.exception.compiled)
self.assertEqual([self.b_library], e.exception.failed)

def test_compile_incremental_progress(self):
python_eval = self.prepare_task(args=['--test-closure'], targets=[self.b_library])
python_eval = self._create_task(target_roots=[self.b_library], options={'closure': True})

with self.assertRaises(TaskError) as e:
python_eval.execute()
self.assertEqual([self.a_library], e.exception.compiled)
self.assertEqual([self.b_library], e.exception.failed)

self.fix_c_source()
python_eval = self.prepare_task(args=['--test-closure'], targets=[self.b_library])
python_eval = self._create_task(target_roots=[self.b_library], options={'closure': True})

compiled = python_eval.execute()
self.assertEqual([self.b_library], compiled)

def test_compile_fail_missing_build_dep(self):
python_eval = self.prepare_task(targets=[self.b_library])
python_eval = self._create_task(target_roots=[self.b_library])

with self.assertRaises(python_eval.Error) as e:
python_eval.execute()
self.assertEqual([], e.exception.compiled)
self.assertEqual([self.b_library], e.exception.failed)

def test_compile_fail_compile_time_check_decorator(self):
python_eval = self.prepare_task(targets=[self.b_library])
python_eval = self._create_task(target_roots=[self.b_library])

with self.assertRaises(TaskError) as e:
python_eval.execute()
self.assertEqual([], e.exception.compiled)
self.assertEqual([self.b_library], e.exception.failed)

def test_compile_failslow(self):
python_eval = self.prepare_task(args=['--test-fail-slow'],
targets=[self.a_library, self.b_library, self.d_library])
python_eval = self._create_task(target_roots=[self.a_library, self.b_library, self.d_library],
options={'fail_slow': True})

with self.assertRaises(TaskError) as e:
python_eval.execute()
self.assertEqual({self.a_library, self.d_library}, set(e.exception.compiled))
self.assertEqual([self.b_library], e.exception.failed)

def test_entry_point_module(self):
python_eval = self.prepare_task(targets=[self.e_binary])
python_eval = self._create_task(target_roots=[self.e_binary])

compiled = python_eval.execute()
self.assertEqual([self.e_binary], compiled)

def test_entry_point_function(self):
python_eval = self.prepare_task(targets=[self.f_binary])
python_eval = self._create_task(target_roots=[self.f_binary])

compiled = python_eval.execute()
self.assertEqual([self.f_binary], compiled)

def test_entry_point_does_not_exist(self):
python_eval = self.prepare_task(targets=[self.g_binary])
python_eval = self._create_task(target_roots=[self.g_binary])

with self.assertRaises(TaskError) as e:
python_eval.execute()
self.assertEqual([], e.exception.compiled)
self.assertEqual([self.g_binary], e.exception.failed)

def test_entry_point_missing_build_dep(self):
python_eval = self.prepare_task(targets=[self.h_binary])
python_eval = self._create_task(target_roots=[self.h_binary])

with self.assertRaises(TaskError) as e:
python_eval.execute()
Expand Down
Loading

0 comments on commit 2d7fbf2

Please sign in to comment.