Skip to content

Commit

Permalink
Hide BuildFileParser and BuildFile, and ensure that schedulers are dr…
Browse files Browse the repository at this point in the history
…opped in tearDown.
  • Loading branch information
stuhood committed Apr 5, 2018
1 parent 2a1b0f1 commit ad6912b
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 13 deletions.
Expand Up @@ -7,6 +7,6 @@ python_tests(
dependencies = [
'contrib/buildgen/src/python/pants/contrib/buildgen:build_file_manipulator',
'src/python/pants/build_graph',
'tests/python/pants_test:test_base',
'tests/python/pants_test:base_test',
],
)
Expand Up @@ -8,13 +8,13 @@
from textwrap import dedent

from pants.build_graph.address import Address
from pants_test.test_base import TestBase
from pants_test.base_test import BaseTest

from pants.contrib.buildgen.build_file_manipulator import (BuildFileManipulator,
BuildTargetParseError)


class BuildFileManipulatorTest(TestBase):
class BuildFileManipulatorTest(BaseTest):

def setUp(self):
super(BuildFileManipulatorTest, self).setUp()
Expand Down
2 changes: 1 addition & 1 deletion tests/python/pants_test/base_test.py
Expand Up @@ -37,7 +37,7 @@
from pants_test.option.util.fakes import create_options_for_optionables


deprecated_module('1.7.0.dev0', 'Use pants_test.test_base instead')
deprecated_module('1.9.0.dev0', 'Use pants_test.test_base instead')


class TestGenerator(object):
Expand Down
31 changes: 22 additions & 9 deletions tests/python/pants_test/test_base.py
Expand Up @@ -115,10 +115,10 @@ def _invalidate_for(self, *relpaths):
Many python operations implicitly create parent directories, so we assume that touching a
file located below directories that do not currently exist will result in their creation.
"""
if self._graph_helper is None:
if self._scheduler is None:
return
files = {f for relpath in relpaths for f in recursive_dirname(relpath)}
self._graph_helper.scheduler.invalidate_files(files)
self._scheduler.invalidate_files(files)

def create_link(self, relsrc, reldst):
"""Creates a symlink within the buildroot.
Expand Down Expand Up @@ -288,12 +288,10 @@ def setUp(self):

self._build_configuration = BuildConfiguration()
self._build_configuration.register_aliases(self.alias_groups)
self.build_file_parser = BuildFileParser(self._build_configuration, self.build_root)
self._build_file_parser = BuildFileParser(self._build_configuration, self.build_root)
self.project_tree = FileSystemProjectTree(self.build_root)

self._graph_helper = None
self._build_graph = None
self._address_mapper = None
self._reset_engine()

def buildroot_files(self, relpath=None):
"""Returns the set of all files under the test build root.
Expand All @@ -310,8 +308,21 @@ def scan():
yield os.path.relpath(os.path.join(root, f), self.build_root)
return set(scan())

def _reset_engine(self):
# TODO: The BuildGraph and AddressMapper end up held a bunch of places, making garbage
# collection difficult. The Scheduler manages threads and database handles, so we try
# harder to ensure that it is dropped.
for attr in ['_build_graph', '_address_mapper']:
field = getattr(self, attr, None)
if field is not None:
field._scheduler = None

self._scheduler = None
self._build_graph = None
self._address_mapper = None

def _initialize_engine(self):
self._graph_helper = EngineInitializer.setup_legacy_graph(
graph_helper = EngineInitializer.setup_legacy_graph(
self.pants_ignore_patterns,
self.pants_workdir,
build_file_imports_behavior='allow',
Expand All @@ -320,7 +331,8 @@ def _initialize_engine(self):
build_ignore_patterns=self.build_ignore_patterns,
)

self._build_graph, self._address_mapper = self._graph_helper.create_build_graph(
self._scheduler = graph_helper.scheduler
self._build_graph, self._address_mapper = graph_helper.create_build_graph(
LiteralTargetRoots([]),
self.build_root,
)
Expand Down Expand Up @@ -404,7 +416,7 @@ def context(self, for_task_types=None, for_subsystems=None, options=None,
context = create_context_from_options(fake_options,
target_roots=target_roots,
build_graph=self.build_graph,
build_file_parser=self.build_file_parser,
build_file_parser=self._build_file_parser,
address_mapper=self.address_mapper,
console_outstream=console_outstream,
workspace=workspace)
Expand All @@ -416,6 +428,7 @@ def tearDown(self):
"""
super(TestBase, self).tearDown()
Subsystem.reset()
self._reset_engine()

def target(self, spec):
"""Resolves the given target address to a Target object.
Expand Down

0 comments on commit ad6912b

Please sign in to comment.