Skip to content

Commit

Permalink
Centralize finding target types for an alias.
Browse files Browse the repository at this point in the history
Both `ReverseDepmap` and `Filter` tasks were doing this slightly
differently, but now they share an implementation via a mixin.

Add tests for the mixin and move the test_dependees.py and
test_filter.py tests to their proper home along side it.

Also cleans up docs and whitespace along the way.

Testing Done:
CI went green here:
  https://travis-ci.org/pantsbuild/pants/builds/79766329

Bugs closed: 2169

Reviewed at https://rbcommons.com/s/twitter/r/2796/
  • Loading branch information
jsirois committed Sep 11, 2015
1 parent 4d5904d commit 908916d
Show file tree
Hide file tree
Showing 11 changed files with 157 additions and 44 deletions.
11 changes: 11 additions & 0 deletions src/python/pants/backend/core/tasks/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ python_library(
dependencies = [
':common',
':console_task',
':target_filter_task_mixin',
'3rdparty/python/twitter/commons:twitter.common.collections',
'src/python/pants/base:build_environment',
'src/python/pants/base:target',
Expand All @@ -149,6 +150,7 @@ python_library(
sources = ['filter.py'],
dependencies = [
':console_task',
':target_filter_task_mixin',
'src/python/pants/base:address_lookup_error',
'src/python/pants/base:build_environment',
'src/python/pants/base:cmd_line_spec_parser',
Expand Down Expand Up @@ -329,6 +331,15 @@ python_library(
],
)

python_library(
name = 'target_filter_task_mixin',
sources = ['target_filter_task_mixin.py'],
dependencies = [
'src/python/pants/backend/core/tasks:task',
'src/python/pants/base:exceptions',
]
)

python_library(
name = 'targets_help',
sources = ['targets_help.py'],
Expand Down
8 changes: 3 additions & 5 deletions src/python/pants/backend/core/tasks/dependees.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@
from twitter.common.collections import OrderedSet

from pants.backend.core.tasks.console_task import ConsoleTask
from pants.backend.core.tasks.target_filter_task_mixin import TargetFilterTaskMixin
from pants.base.build_environment import get_buildroot
from pants.base.exceptions import TaskError
from pants.base.source_root import SourceRoot


class ReverseDepmap(ConsoleTask):
class ReverseDepmap(TargetFilterTaskMixin, ConsoleTask):
"""Outputs all targets whose dependencies include at least one of the input targets."""

@classmethod
Expand All @@ -44,12 +45,9 @@ def console_output(self, _):
buildfiles = OrderedSet()
address_mapper = self.context.address_mapper
if self._dependees_types:
registered_aliases = self.context.build_file_parser.registered_aliases()
base_paths = OrderedSet()
for dependees_type in self._dependees_types:
target_types = registered_aliases.target_types_by_alias.get(dependees_type, None)
if not target_types:
raise TaskError('No Targets with type name: {}'.format(dependees_type))
target_types = self.target_types_for_alias(dependees_type)
# Try to find the SourceRoots for the given input type alias
for target_type in target_types:
try:
Expand Down
8 changes: 3 additions & 5 deletions src/python/pants/backend/core/tasks/filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@
import re

from pants.backend.core.tasks.console_task import ConsoleTask
from pants.backend.core.tasks.target_filter_task_mixin import TargetFilterTaskMixin
from pants.base.address_lookup_error import AddressLookupError
from pants.base.build_environment import get_buildroot
from pants.base.cmd_line_spec_parser import CmdLineSpecParser
from pants.base.exceptions import TaskError
from pants.util.filtering import create_filters, wrap_filters


class Filter(ConsoleTask):
class Filter(TargetFilterTaskMixin, ConsoleTask):
"""Filter the input targets based on various criteria.
Each of the filtering options below is a comma-separated list of filtering criteria, with an
Expand Down Expand Up @@ -69,10 +70,7 @@ def filter_for_address(spec):
self._filters.extend(create_filters(self.get_options().target, filter_for_address))

def filter_for_type(name):
registered_aliases = self.context.build_file_parser.registered_aliases()
target_types = registered_aliases.target_types_by_alias.get(name, None)
if not target_types:
raise TaskError('No Targets with type name: {}'.format(name))
target_types = self.target_types_for_alias(name)
return lambda target: isinstance(target, tuple(target_types))
self._filters.extend(create_filters(self.get_options().type, filter_for_type))

Expand Down
33 changes: 33 additions & 0 deletions src/python/pants/backend/core/tasks/target_filter_task_mixin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# coding=utf-8
# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
unicode_literals, with_statement)

from pants.backend.core.tasks.task import Task
from pants.base.exceptions import TaskError


class TargetFilterTaskMixin(Task):
"""A Task mixin that provides methods to help with filtering by target type."""

class InvalidTargetType(TaskError):
"""Indicates a target type name that is not registered or does not point to a `Target` type."""

def target_types_for_alias(self, alias):
"""Returns all the target types that might be produced by the given alias.
Normally there is 1 target type per alias, but macros can expand a single alias to several
target types.
:param string alias: The alias to look up associated target types for.
:returns: The set of target types that can be produced by the given alias.
:raises :class:`TargetFilterTaskMixin.InvalidTargetType`: when no target types correspond to
the given `alias`.
"""
registered_aliases = self.context.build_file_parser.registered_aliases()
target_types = registered_aliases.target_types_by_alias.get(alias, None)
if not target_types:
raise self.InvalidTargetType('Not a target type: {}'.format(alias))
return target_types
6 changes: 3 additions & 3 deletions src/python/pants/base/build_file_aliases.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def _is_target_macro_factory(obj):
@classmethod
def _validate_alias(cls, category, alias, obj):
if not isinstance(alias, six.string_types):
raise TypeError('Aliases must be strings, given {category} entry {alias} of type {typ} as '
raise TypeError('Aliases must be strings, given {category} entry {alias!r} of type {typ} as '
'the alias of {obj}'
.format(category=category, alias=alias, typ=type(alias).__name__, obj=obj))

Expand Down Expand Up @@ -259,7 +259,7 @@ def context_aware_object_factories(self):
def target_types_by_alias(self):
"""Returns a mapping from target alias to the target types produced for that alias.
Normally there is 1 target type per alias, but macros can expand a single alias o several
Normally there is 1 target type per alias, but macros can expand a single alias to several
target types.
:rtype: dict
Expand All @@ -278,7 +278,7 @@ def merge(self, other):
:param other: The BuildFileAliases to merge in.
:type other: :class:`BuildFileAliases`
:returns: A new BuildFileAliases containing other's aliases merged into ours.
:returns: A new BuildFileAliases containing `other`'s aliases merged into ours.
:rtype: :class:`BuildFileAliases`
"""
if not isinstance(other, BuildFileAliases):
Expand Down
45 changes: 45 additions & 0 deletions tests/python/pants_test/backend/core/tasks/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@ target(
name='tasks',
dependencies=[
':bash_completion',
':dependees',
':filter',
':markdown_to_html',
':mutex_task_mixin',
':scm_publish',
':target_filter_task_mixin',
],
)

Expand All @@ -34,6 +37,37 @@ python_tests(
]
)

python_tests(
name = 'dependees',
sources = ['test_dependees.py'],
dependencies = [
'src/python/pants/backend/codegen/targets:java',
'src/python/pants/backend/core/targets:common',
'src/python/pants/backend/core/tasks:dependees',
'src/python/pants/backend/jvm/targets:java',
'src/python/pants/backend/jvm/targets:jvm',
'src/python/pants/backend/python/targets:python',
'src/python/pants/base:build_file_aliases',
'src/python/pants/base:exceptions',
'src/python/pants/base:source_root',
'tests/python/pants_test/tasks:task_test_base',
]
)

python_tests(
name = 'filter',
sources = ['test_filter.py'],
dependencies = [
'src/python/pants/backend/core/targets:common',
'src/python/pants/backend/core/tasks:filter',
'src/python/pants/backend/jvm/targets:java',
'src/python/pants/backend/python/targets:python',
'src/python/pants/base:build_file_aliases',
'src/python/pants/base:exceptions',
'tests/python/pants_test/tasks:task_test_base',
],
)

python_tests(
name = 'markdown_to_html',
sources = ['test_markdown_to_html.py'],
Expand Down Expand Up @@ -83,3 +117,14 @@ python_tests(
'src/python/pants/backend/core/tasks:scm_publish',
]
)

python_tests(
name='target_filter_task_mixin',
sources=['test_target_filter_task_mixin.py'],
dependencies=[
'src/python/pants/backend/core/tasks:target_filter_task_mixin',
'src/python/pants/base:build_file_aliases',
'src/python/pants/base:target',
'tests/python/pants_test/tasks:task_test_base',
]
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# coding=utf-8
# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
unicode_literals, with_statement)

from pants.backend.core.tasks.target_filter_task_mixin import TargetFilterTaskMixin
from pants.base.build_file_aliases import BuildFileAliases, TargetMacro
from pants.base.target import Target
from pants_test.tasks.task_test_base import TaskTestBase


class TargetFilterTaskMixinTest(TaskTestBase):

@classmethod
def task_type(cls):
class TestTargetFilteringTask(TargetFilterTaskMixin):
def execute(self):
raise NotImplementedError()
return TestTargetFilteringTask

class RedTarget(Target):
pass

class BlueTarget(Target):
pass

class GreenTarget(Target):
pass

@property
def alias_groups(self):
purple_macro = TargetMacro.Factory.wrap(lambda ctx: None, self.RedTarget, self.BlueTarget)
return BuildFileAliases(targets={'green': self.GreenTarget, 'purple': purple_macro},
objects={'red': object()},
context_aware_object_factories={'blue': lambda ctx: None})

def setUp(self):
super(TargetFilterTaskMixinTest, self).setUp()
self.task = self.create_task(self.context())

def test_simple_alias(self):
green_targets = self.task.target_types_for_alias('green')
self.assertEqual({self.GreenTarget}, green_targets)

def test_macro_alias(self):
purple_targets = self.task.target_types_for_alias('purple')
self.assertEqual({self.RedTarget, self.BlueTarget}, purple_targets)

def test_alias_miss(self):
with self.assertRaises(self.task.InvalidTargetType):
self.task.target_types_for_alias('red') # Not a target - an object.
with self.assertRaises(self.task.InvalidTargetType):
self.task.target_types_for_alias('blue') # Not a target - a context aware object factory.
with self.assertRaises(self.task.InvalidTargetType):
self.task.target_types_for_alias('yellow') # Not a registered alias.
2 changes: 2 additions & 0 deletions tests/python/pants_test/base/test_build_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class Fred(Target):

target_call_proxy = parse_state.parse_globals['fred']
target_call_proxy(name='jake')

self.assertEqual(1, len(parse_state.registered_addressable_instances))
name, target_proxy = parse_state.registered_addressable_instances.pop()
self.assertEqual('jake', target_proxy.addressed_name)
Expand Down Expand Up @@ -88,6 +89,7 @@ def macro(self, parse_context):

target_call_proxy = parse_state.parse_globals['fred']
target_call_proxy(name='jake')

self.assertEqual(1, len(parse_state.registered_addressable_instances))
name, target_proxy = parse_state.registered_addressable_instances.pop()
self.assertEqual('frog', target_proxy.addressed_name)
Expand Down
31 changes: 0 additions & 31 deletions tests/python/pants_test/tasks/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,8 @@ target(
':cache_manager',
':check_published_deps',
':console_task',
':dependees',
':detect_duplicates',
':execution_graph',
':filter',
':group_task',
':jar_create',
':jar_publish',
Expand Down Expand Up @@ -187,22 +185,6 @@ python_tests(
]
)

python_tests(
name = 'dependees',
sources = ['test_dependees.py'],
dependencies = [
':task_test_base',
'src/python/pants/backend/codegen/targets:java',
'src/python/pants/backend/core/targets:common',
'src/python/pants/backend/core/tasks:dependees',
'src/python/pants/backend/jvm/targets:java',
'src/python/pants/backend/jvm/targets:scala',
'src/python/pants/backend/python/targets:python',
'src/python/pants/base:build_environment',
'src/python/pants/base:build_file_aliases',
]
)

python_tests(
name = 'detect_duplicates',
sources = ['test_detect_duplicates.py'],
Expand Down Expand Up @@ -235,19 +217,6 @@ python_tests(
],
)

python_tests(
name = 'filter',
sources = ['test_filter.py'],
dependencies = [
':task_test_base',
'src/python/pants/backend/jvm/targets:java',
'src/python/pants/backend/python/targets:python',
'src/python/pants/backend/core/tasks:filter',
'src/python/pants/backend/core/targets:common',
'src/python/pants/base:build_file_aliases',
],
)

python_tests(
name = 'ivy_resolve_integration',
sources = ['test_ivy_resolve_integration.py'],
Expand Down

0 comments on commit 908916d

Please sign in to comment.