From 0779a823cae955c67948108d2a9e58a1f02db561 Mon Sep 17 00:00:00 2001 From: Caitie McCaffrey Date: Tue, 18 Oct 2016 16:24:34 -0700 Subject: [PATCH 1/7] Adding scalafmt formatting to fmt goal Adds scalafmt formatting to the fmt command for scala files. Refactored ScalaFmt class into a base class, with two sub classes ScalaFmtCheckFormat (checks if files are formatted correctly) and ScalaFmtFormat (formats the files). This ensures that the same version of scalafmt is used for both. Both of these are currently turned off in pants.ini. Skip=True Testing Done: New Integration Test Case CI passes [#3936](https://github.com/pantsbuild/pants/pull/3963) Reviewed at https://rbcommons.com/s/twitter/r/4312/ --- pants.ini | 4 +- src/python/pants/backend/jvm/register.py | 5 +- src/python/pants/backend/jvm/tasks/BUILD | 1 + .../pants/backend/jvm/tasks/scalafmt.py | 97 +++++++++++++++---- .../python/pants_test/backend/jvm/tasks/BUILD | 1 + .../backend/jvm/tasks/test_scalafmt.py | 43 +++++++- 6 files changed, 126 insertions(+), 25 deletions(-) diff --git a/pants.ini b/pants.ini index c8ba00d5581..c802ab8fbac 100644 --- a/pants.ini +++ b/pants.ini @@ -124,7 +124,9 @@ deps: ["3rdparty:thrift-0.9.2"] configuration: %(pants_supportdir)s/checkstyle/coding_style.xml [compile.scalafmt] -configuration: %(pants_supportdir)s/scalafmt/config +skip: True + +[fmt.scalafmt] skip: True [compile.findbugs] diff --git a/src/python/pants/backend/jvm/register.py b/src/python/pants/backend/jvm/register.py index 4fe60ff224c..6538af5e5c4 100644 --- a/src/python/pants/backend/jvm/register.py +++ b/src/python/pants/backend/jvm/register.py @@ -60,7 +60,7 @@ RunTestJvmPrepCommand) from pants.backend.jvm.tasks.scala_repl import ScalaRepl from pants.backend.jvm.tasks.scaladoc_gen import ScaladocGen -from pants.backend.jvm.tasks.scalafmt import ScalaFmt +from pants.backend.jvm.tasks.scalafmt import ScalaFmtCheckFormat, ScalaFmtFormat from pants.backend.jvm.tasks.unpack_jars import UnpackJars from pants.build_graph.build_file_aliases import BuildFileAliases from pants.goal.goal import Goal @@ -189,7 +189,8 @@ def register_goals(): task(name='jvm-dirty', action=JvmRun, serialize=False).install('run-dirty') task(name='scala', action=ScalaRepl, serialize=False).install('repl') task(name='scala-dirty', action=ScalaRepl, serialize=False).install('repl-dirty') - task(name='scalafmt', action=ScalaFmt, serialize=False).install('compile', first=True) + task(name='scalafmt', action=ScalaFmtCheckFormat, serialize=False).install('compile', first=True) + task(name='scalafmt', action=ScalaFmtFormat, serialize=False).install('fmt') task(name='test-jvm-prep-command', action=RunTestJvmPrepCommand).install('test', first=True) task(name='binary-jvm-prep-command', action=RunBinaryJvmPrepCommand).install('binary', first=True) task(name='compile-jvm-prep-command', action=RunCompileJvmPrepCommand).install('compile', first=True) diff --git a/src/python/pants/backend/jvm/tasks/BUILD b/src/python/pants/backend/jvm/tasks/BUILD index 51278492e0b..1e9cffce1da 100644 --- a/src/python/pants/backend/jvm/tasks/BUILD +++ b/src/python/pants/backend/jvm/tasks/BUILD @@ -619,6 +619,7 @@ python_library( 'src/python/pants/base:exceptions', 'src/python/pants/build_graph', 'src/python/pants/option', + 'src/python/pants/util:meta', ] ) diff --git a/src/python/pants/backend/jvm/tasks/scalafmt.py b/src/python/pants/backend/jvm/tasks/scalafmt.py index 2db01b0d584..37ea9ba4884 100644 --- a/src/python/pants/backend/jvm/tasks/scalafmt.py +++ b/src/python/pants/backend/jvm/tasks/scalafmt.py @@ -5,15 +5,21 @@ from __future__ import (absolute_import, division, generators, nested_scopes, print_function, unicode_literals, with_statement) +from abc import abstractproperty + from pants.backend.jvm.targets.jar_dependency import JarDependency from pants.backend.jvm.tasks.nailgun_task import NailgunTask from pants.base.exceptions import TaskError from pants.build_graph.target import Target from pants.option.custom_types import file_option +from pants.util.meta import AbstractClass + +class ScalaFmt(NailgunTask, AbstractClass): + """Abstract class to run ScalaFmt commands. -class ScalaFmt(NailgunTask): - """Checks Scala code matches scalafmt style. + Classes that inherit from this should override get_command_args and + process_results to run different scalafmt commands :API: public """ @@ -23,8 +29,8 @@ class ScalaFmt(NailgunTask): @classmethod def register_options(cls, register): super(ScalaFmt, cls).register_options(register) - register('--skip', type=bool, fingerprint=True, help='Skip Scalafmt Check') - register('--configuration', advanced=True, type=file_option, fingerprint=True, + register('--skip', type=bool, fingerprint=False, help='Skip Scalafmt Check') + register('--configuration', advanced=True, type=file_option, fingerprint=False, help='Path to scalafmt config file, if not specified default scalafmt config used') cls.register_jvm_tool(register, 'scalafmt', @@ -45,24 +51,29 @@ def execute(self): if sources: files = ",".join(sources) - config_file = self.get_options().configuration - - """If no config file is specified use default scalafmt config""" - args = ['--test', '--files', files] - if config_file != None: - args.extend(['--config', config_file]) - result = self.runjava(classpath=self.tool_classpath('scalafmt'), main=self._SCALAFMT_MAIN, - args=args, + args=self.get_command_args(files), workunit_name='scalafmt') - if result != 0: - scalafmt_cmd = 'scalafmt -i --files {}'.format(files) - if config_file != None: - scalafmt_cmd += ' --config {}'.format(config_file) - - raise TaskError('Scalafmt failed with exit code {} to fix run: `{}`'.format(result, scalafmt_cmd), exit_code=result) + self.process_results(result) + + @abstractproperty + def get_command_args(self, files): + """Returns the arguments used to run Scalafmt command. + + The return value should be an array of strings. For + example, to run the Scalafmt help command: + ['--help'] + """ + + @abstractproperty + def process_results(self, result): + """This method processes the results of the scalafmt command. + + No return value is expected. If an error occurs running + Scalafmt raising a TaskError is recommended. + """ def get_non_synthetic_scala_targets(self, targets): return filter( @@ -77,3 +88,53 @@ def calculate_sources(self, targets): sources.update(source for source in target.sources_relative_to_buildroot() if source.endswith(self._SCALA_SOURCE_EXTENSION)) return sources + + +class ScalaFmtCheckFormat(ScalaFmt): + """This Task checks that all scala files in the target are formatted + correctly. + + If the files are not formatted correctly an error is raised + including the command to run to format the files correctly + + :API: public + """ + + def get_command_args(self, files): + # If no config file is specified use default scalafmt config. + config_file = self.get_options().configuration + args = ['--test', '--files', files] + if config_file!= None: + args.extend(['--config', config_file]) + + return args + + def process_results(self, result): + if result != 0: + raise TaskError('Scalafmt failed with exit code {} to fix run: `./pants fmt ` config{}'.format(result, self.get_options().configuration), exit_code=result) + + +class ScalaFmtFormat(ScalaFmt): + """This Task reads all scala files in the target and emits + the source in a standard style as specified by the configuration + file. + + This task mutates the underlying flies. + + :API: public + """ + + def get_command_args(self, files): + # If no config file is specified use default scalafmt config. + config_file = self.get_options().configuration + args = ['-i', '--files', files] + if config_file!= None: + args.extend(['--config', config_file]) + + return args + + def process_results(self, result): + # Processes the results of running the scalafmt command. + if result != 0: + raise TaskError('Scalafmt failed to format files', exit_code=result) + diff --git a/tests/python/pants_test/backend/jvm/tasks/BUILD b/tests/python/pants_test/backend/jvm/tasks/BUILD index ad59f9ae1bb..013cbbb1dc4 100644 --- a/tests/python/pants_test/backend/jvm/tasks/BUILD +++ b/tests/python/pants_test/backend/jvm/tasks/BUILD @@ -596,6 +596,7 @@ python_tests( 'src/python/pants/backend/jvm/tasks:scalafmt', 'tests/python/pants_test:int-test', ], + timeout=120, tags = {'integration'}, ) diff --git a/tests/python/pants_test/backend/jvm/tasks/test_scalafmt.py b/tests/python/pants_test/backend/jvm/tasks/test_scalafmt.py index a94d9a0d350..6a9cc5b414d 100644 --- a/tests/python/pants_test/backend/jvm/tasks/test_scalafmt.py +++ b/tests/python/pants_test/backend/jvm/tasks/test_scalafmt.py @@ -12,18 +12,53 @@ class ScalaFmtIntegrationTests(PantsRunIntegrationTest): - def test_scalafmt_fail(self): + def test_scalafmt_fail_default_config(self): target = '{}/badscalastyle::'.format(TEST_DIR) # test should fail because of style error. failing_test = self.run_pants(['compile', target], - {'compile.scalafmt':{'skip': 'False'}}) + {'compile.scalafmt':{'skip':'False'}}) + self.assert_failure(failing_test) + def test_scalafmt_fail(self): + target = '{}/badscalastyle::'.format(TEST_DIR) + # test should fail because of style error. + failing_test = self.run_pants(['compile', target], + {'compile.scalafmt':{'skip':'False', + 'configuration':'%(pants_supportdir)s/scalafmt/config'}}) self.assert_failure(failing_test) def test_scalafmt_disabled(self): target = '{}/badscalastyle::'.format(TEST_DIR) # test should pass because of scalafmt disabled. failing_test = self.run_pants(['compile', target], - {'compile.scalafmt':{'skip': 'True'}}) - + {'compile.scalafmt': {'skip':'True'}}) self.assert_success(failing_test) + + def test_scalafmt_format_default_config(self): + self.format_file_and_verify_fmt({'skip':'False'}) + + def test_scalafmt_format(self): + self.format_file_and_verify_fmt({'skip':'False', + 'configuration':'%(pants_supportdir)s/scalafmt/config'}) + + def format_file_and_verify_fmt(self, options): + # take a snapshot of the file which we can write out + # after the test finishes executing. + test_file_name = '{}/badscalastyle/BadScalaStyle.scala'.format(TEST_DIR) + f = open(test_file_name, 'r') + contents = f.read() + f.close() + + # format an incorrectly formatted file. + target = '{}/badscalastyle::'.format(TEST_DIR) + fmt_result = self.run_pants(['fmt', target], {'fmt.scalafmt':options}) + self.assert_success(fmt_result) + + # verify that the compile check not passes. + test_fmt = self.run_pants(['compile', target], {'compile.scalafmt':options}) + self.assert_success(test_fmt) + + # restore the file to its original state. + f = open(test_file_name, 'w') + f.write(contents) + f.close() From 0e43562657ddfb16d594abb4e48b8b471b6c08ff Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Tue, 18 Oct 2016 22:17:07 -0700 Subject: [PATCH 2/7] Prepare 1.2.0rc2 release As described in the [updated release docs](http://www.pantsbuild.org/release.html#preparation-for-the-release-from-the-stable-branch), this review adds the 1.2.0rc2 notes in master, which will then be cherry-picked and released from the `1.2.x` branch. - Update the release notes for a 1.2.0rc2 release. Testing Done: https://travis-ci.org/pantsbuild/pants/builds/168717837 Reviewed at https://rbcommons.com/s/twitter/r/4326/ --- src/python/pants/notes/1.2.x.rst | 40 ++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/python/pants/notes/1.2.x.rst b/src/python/pants/notes/1.2.x.rst index 289ba552ecf..84444ddb36c 100644 --- a/src/python/pants/notes/1.2.x.rst +++ b/src/python/pants/notes/1.2.x.rst @@ -4,6 +4,46 @@ This document describes releases from the ``1.2.x`` ``stable`` branch. +1.2.0rc2 (10/18/2016) +--------------------- + +The third, and perhaps final, release candidate for stable 1.2.0. + +Notably: this release candidate restores JDK7 compatibility for junit tests. + +API Changes +~~~~~~~~~~~ + +* Update junit runner to 1.0.15 to get java 7 compatibility + `RB #4324 `_ + +* Back down the minimum required java version for running Pants tools to java 7 + `RB #4314 `_ + +Bugfixes +~~~~~~~~ + +* Fix exclude_target_regexp breakage in test-changed and --files option breakage in changed with diffspec + `RB #4321 `_ + +* Prevent cleanup error at end of pants test with --test-junit-html-report option, update safe_rmtree to be symlink aware + `RB #4319 `_ + +* fix --changed-files option + `RB #4309 `_ + +New Features +~~~~~~~~~~~~ + +* Adding scalafmt formatting to fmt goal and enable by default + `RB #4312 `_ + +Refactoring, Improvements, and Tooling +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +* migrate changed integration tests to isolated temp git repos and add an environment variable to override buildroot + `RB #4295 `_ + 1.2.0rc1 (10/13/2016) --------------------- From e80c77bdaf58578057e8f871b584d340999bb85a Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Wed, 19 Oct 2016 16:32:47 -0700 Subject: [PATCH 3/7] Add a target-types option to scalafmt to avoid formatting all targets - Add and use a target-types option to avoid formatting all targets. - Fix output of suggested `./pants fmt` command. Testing Done: https://travis-ci.org/pantsbuild/pants/builds/169065521 Bugs closed: 3957, 3990 Reviewed at https://rbcommons.com/s/twitter/r/4328/ --- src/python/pants/backend/jvm/tasks/BUILD | 1 + .../pants/backend/jvm/tasks/scalafmt.py | 21 +++++++++++++++---- .../testproject/badscalastyle/BUILD | 6 ++++++ .../backend/jvm/tasks/test_scalafmt.py | 16 ++++++++++---- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/python/pants/backend/jvm/tasks/BUILD b/src/python/pants/backend/jvm/tasks/BUILD index 1e9cffce1da..436c9aedf4e 100644 --- a/src/python/pants/backend/jvm/tasks/BUILD +++ b/src/python/pants/backend/jvm/tasks/BUILD @@ -620,6 +620,7 @@ python_library( 'src/python/pants/build_graph', 'src/python/pants/option', 'src/python/pants/util:meta', + 'src/python/pants/util:memo', ] ) diff --git a/src/python/pants/backend/jvm/tasks/scalafmt.py b/src/python/pants/backend/jvm/tasks/scalafmt.py index 37ea9ba4884..e173ff7364d 100644 --- a/src/python/pants/backend/jvm/tasks/scalafmt.py +++ b/src/python/pants/backend/jvm/tasks/scalafmt.py @@ -10,8 +10,8 @@ from pants.backend.jvm.targets.jar_dependency import JarDependency from pants.backend.jvm.tasks.nailgun_task import NailgunTask from pants.base.exceptions import TaskError -from pants.build_graph.target import Target from pants.option.custom_types import file_option +from pants.util.memo import memoized_property from pants.util.meta import AbstractClass @@ -32,6 +32,11 @@ def register_options(cls, register): register('--skip', type=bool, fingerprint=False, help='Skip Scalafmt Check') register('--configuration', advanced=True, type=file_option, fingerprint=False, help='Path to scalafmt config file, if not specified default scalafmt config used') + register('--target-types', + default={'scala_library', 'junit_tests', 'java_tests'}, + advanced=True, + type=set, + help='The target types to apply formatting to.') cls.register_jvm_tool(register, 'scalafmt', classpath=[ @@ -40,6 +45,14 @@ def register_options(cls, register): rev='0.2.11') ]) + @memoized_property + def _formatted_target_types(self): + aliases = self.get_options().target_types + registered_aliases = self.context.build_file_parser.registered_aliases() + return tuple({target_type + for alias in aliases + for target_type in registered_aliases.target_types_by_alias[alias]}) + def execute(self): """Runs Scalafmt on all found Scala Source Files.""" if self.get_options().skip: @@ -77,7 +90,7 @@ def process_results(self, result): def get_non_synthetic_scala_targets(self, targets): return filter( - lambda target: isinstance(target, Target) + lambda target: isinstance(target, self._formatted_target_types) and target.has_sources(self._SCALA_SOURCE_EXTENSION) and (not target.is_synthetic), targets) @@ -111,7 +124,8 @@ def get_command_args(self, files): def process_results(self, result): if result != 0: - raise TaskError('Scalafmt failed with exit code {} to fix run: `./pants fmt ` config{}'.format(result, self.get_options().configuration), exit_code=result) + raise TaskError('Scalafmt failed with exit code {}; to fix run: ' + '`./pants fmt `'.format(result), exit_code=result) class ScalaFmtFormat(ScalaFmt): @@ -137,4 +151,3 @@ def process_results(self, result): # Processes the results of running the scalafmt command. if result != 0: raise TaskError('Scalafmt failed to format files', exit_code=result) - diff --git a/testprojects/src/scala/org/pantsbuild/testproject/badscalastyle/BUILD b/testprojects/src/scala/org/pantsbuild/testproject/badscalastyle/BUILD index dff9fc540f4..e8d5b6fecbf 100644 --- a/testprojects/src/scala/org/pantsbuild/testproject/badscalastyle/BUILD +++ b/testprojects/src/scala/org/pantsbuild/testproject/badscalastyle/BUILD @@ -1,3 +1,9 @@ scala_library( sources=globs('*') ) + +resources( + name='as_resources', + sources=globs('*'), + description='Depends on the same sources as the target above, but as resources.' +) diff --git a/tests/python/pants_test/backend/jvm/tasks/test_scalafmt.py b/tests/python/pants_test/backend/jvm/tasks/test_scalafmt.py index 6a9cc5b414d..de03e5a0b8e 100644 --- a/tests/python/pants_test/backend/jvm/tasks/test_scalafmt.py +++ b/tests/python/pants_test/backend/jvm/tasks/test_scalafmt.py @@ -13,14 +13,14 @@ class ScalaFmtIntegrationTests(PantsRunIntegrationTest): def test_scalafmt_fail_default_config(self): - target = '{}/badscalastyle::'.format(TEST_DIR) + target = '{}/badscalastyle'.format(TEST_DIR) # test should fail because of style error. failing_test = self.run_pants(['compile', target], {'compile.scalafmt':{'skip':'False'}}) self.assert_failure(failing_test) def test_scalafmt_fail(self): - target = '{}/badscalastyle::'.format(TEST_DIR) + target = '{}/badscalastyle'.format(TEST_DIR) # test should fail because of style error. failing_test = self.run_pants(['compile', target], {'compile.scalafmt':{'skip':'False', @@ -28,7 +28,7 @@ def test_scalafmt_fail(self): self.assert_failure(failing_test) def test_scalafmt_disabled(self): - target = '{}/badscalastyle::'.format(TEST_DIR) + target = '{}/badscalastyle'.format(TEST_DIR) # test should pass because of scalafmt disabled. failing_test = self.run_pants(['compile', target], {'compile.scalafmt': {'skip':'True'}}) @@ -50,7 +50,7 @@ def format_file_and_verify_fmt(self, options): f.close() # format an incorrectly formatted file. - target = '{}/badscalastyle::'.format(TEST_DIR) + target = '{}/badscalastyle'.format(TEST_DIR) fmt_result = self.run_pants(['fmt', target], {'fmt.scalafmt':options}) self.assert_success(fmt_result) @@ -62,3 +62,11 @@ def format_file_and_verify_fmt(self, options): f = open(test_file_name, 'w') f.write(contents) f.close() + + def test_scalafmt_ignore_resources(self): + target = '{}/badscalastyle:as_resources'.format(TEST_DIR) + # test should succeed because the target is not in `target-types`. + run = self.run_pants(['compile', target], + {'compile.scalafmt':{'skip':'False', + 'configuration':'%(pants_supportdir)s/scalafmt/config'}}) + self.assert_success(run) From 02dd1876b452ab30bf6c224fbfe831d5c61b1cae Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Wed, 19 Oct 2016 22:09:28 -0700 Subject: [PATCH 4/7] Prepare 1.2.0rc3 Prepare the 1.2.0rc3 release. As with 1.2.0rc2, the actual release will go out from the `1.2.x` branch. Testing Done: https://travis-ci.org/pantsbuild/pants/builds/169095919 Reviewed at https://rbcommons.com/s/twitter/r/4329/ --- src/python/pants/notes/1.2.x.rst | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/python/pants/notes/1.2.x.rst b/src/python/pants/notes/1.2.x.rst index 84444ddb36c..4ac2a4a9828 100644 --- a/src/python/pants/notes/1.2.x.rst +++ b/src/python/pants/notes/1.2.x.rst @@ -4,6 +4,30 @@ This document describes releases from the ``1.2.x`` ``stable`` branch. +1.2.0rc3 (10/20/2016) +--------------------- + +The fourth release candidate for stable 1.2.0, which limits the default targets affected +by scalafmt and includes Benjy's lovely "default source globs" patch. + +Bugfixes +~~~~~~~~ + +* Fix erroneous deprecated scope warnings. + `RB #4323 `_ + +New Features +~~~~~~~~~~~~ + +* Allow targets to have sensible defaults for sources=. + `RB #4300 `_ + +Refactoring, Improvements, and Tooling +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +* Add a target-types option to scalafmt to avoid formatting all targets + `RB #4328 `_ + 1.2.0rc2 (10/18/2016) --------------------- From 3a022b967a5c9023c4f746e595dccb7351033573 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Thu, 20 Oct 2016 20:09:16 +0100 Subject: [PATCH 5/7] Deprecate the `java_tests` alias in favor of `junit_tests`. Testing Done: CI passed: https://travis-ci.org/pantsbuild/pants/builds/168784588 Reviewed at https://rbcommons.com/s/twitter/r/4322/ --- contrib/findbugs/README.md | 2 +- .../pants/contrib/findbugs/tasks/findbugs.py | 4 +- src/docs/export.md | 2 +- src/python/pants/backend/jvm/BUILD | 1 + src/python/pants/backend/jvm/register.py | 15 +- src/python/pants/backend/jvm/targets/BUILD | 5 +- .../pants/backend/jvm/targets/java_library.py | 4 +- .../pants/backend/jvm/targets/java_tests.py | 123 +--------------- .../pants/backend/jvm/targets/junit_tests.py | 137 ++++++++++++++++++ .../backend/jvm/targets/scala_library.py | 4 +- .../pants/backend/jvm/tasks/junit_run.py | 32 ++-- .../maven_layout/provided_patching/leaf/BUILD | 2 +- .../testproject/junit/earlyexit/BUILD | 2 +- .../tests/org/pantsbuild/tmp/tests/BUILD | 6 +- .../tests/org/pantsbuild/tmp/tests/BUILD | 2 +- .../testproject/junit/testscope/BUILD | 2 +- .../org/pantsbuild/testproject/empty/BUILD | 2 +- .../org/pantsbuild/tools/junit/impl/BUILD | 2 +- .../pants_test/backend/jvm/targets/BUILD | 4 +- ...test_java_tests.py => test_junit_tests.py} | 38 ++--- .../backend/jvm/tasks/test_junit_run.py | 26 ++-- .../jvm/tasks/test_scope_test_integration.py | 4 +- .../backend/project_info/tasks/test_export.py | 4 +- tests/python/pants_test/test_maven_layout.py | 4 +- 24 files changed, 234 insertions(+), 193 deletions(-) create mode 100644 src/python/pants/backend/jvm/targets/junit_tests.py rename tests/python/pants_test/backend/jvm/targets/{test_java_tests.py => test_junit_tests.py} (59%) diff --git a/contrib/findbugs/README.md b/contrib/findbugs/README.md index 4ba17858ab8..e3c75b220e4 100644 --- a/contrib/findbugs/README.md +++ b/contrib/findbugs/README.md @@ -26,7 +26,7 @@ plugins: [ ## Running When you run `./pants compile` the plugin is executed after the compile step and will run FindBugs -on any `java_library` or `java_tests` targets. +on any `java_library` or `junit_tests` targets. ``` ./pants compile diff --git a/contrib/findbugs/src/python/pants/contrib/findbugs/tasks/findbugs.py b/contrib/findbugs/src/python/pants/contrib/findbugs/tasks/findbugs.py index e9906fb0966..28f733d751d 100644 --- a/contrib/findbugs/src/python/pants/contrib/findbugs/tasks/findbugs.py +++ b/contrib/findbugs/src/python/pants/contrib/findbugs/tasks/findbugs.py @@ -11,7 +11,7 @@ from pants.backend.jvm.subsystems.shader import Shader from pants.backend.jvm.targets.jar_dependency import JarDependency from pants.backend.jvm.targets.java_library import JavaLibrary -from pants.backend.jvm.targets.java_tests import JavaTests +from pants.backend.jvm.targets.junit_tests import JUnitTests from pants.backend.jvm.tasks.nailgun_task import NailgunTask from pants.base.build_environment import get_buildroot from pants.base.exceptions import TaskError @@ -86,7 +86,7 @@ def _exclude_patterns(self): return "(" + ")|(".join(self.get_options().exclude_patterns) + ")" def _is_findbugs_target(self, target): - if not isinstance(target, (JavaLibrary, JavaTests)): + if not isinstance(target, (JavaLibrary, JUnitTests)): self.context.log.debug('Skipping [{}] because it is not a java library or java test'.format(target.address.spec)) return False if target.is_synthetic: diff --git a/src/docs/export.md b/src/docs/export.md index f5224ff43fc..f03f22902c1 100644 --- a/src/docs/export.md +++ b/src/docs/export.md @@ -118,7 +118,7 @@ The following is an abbreviated export file from a command in the pants repo: "junit:junit:latest.integration" ], "platform": "java6", - "pants_target_type": "java_tests", + "pants_target_type": "junit_tests", "globs": { "globs": [ "examples/tests/java/org/pantsbuild/example/usethrift/UseThriftTest.java" diff --git a/src/python/pants/backend/jvm/BUILD b/src/python/pants/backend/jvm/BUILD index 65e9faec60b..75f952bd384 100644 --- a/src/python/pants/backend/jvm/BUILD +++ b/src/python/pants/backend/jvm/BUILD @@ -40,6 +40,7 @@ python_library( 'src/python/pants/backend/jvm/subsystems:shader', 'src/python/pants/backend/jvm/targets:all', 'src/python/pants/backend/jvm/tasks:all', + 'src/python/pants/base:deprecated', 'src/python/pants/build_graph', 'src/python/pants/goal', 'src/python/pants/goal:task_registrar', diff --git a/src/python/pants/backend/jvm/register.py b/src/python/pants/backend/jvm/register.py index 6538af5e5c4..1a3c40fc6d3 100644 --- a/src/python/pants/backend/jvm/register.py +++ b/src/python/pants/backend/jvm/register.py @@ -21,8 +21,8 @@ from pants.backend.jvm.targets.jar_library import JarLibrary from pants.backend.jvm.targets.java_agent import JavaAgent from pants.backend.jvm.targets.java_library import JavaLibrary -from pants.backend.jvm.targets.java_tests import JavaTests from pants.backend.jvm.targets.javac_plugin import JavacPlugin +from pants.backend.jvm.targets.junit_tests import JUnitTests from pants.backend.jvm.targets.jvm_app import Bundle, DirectoryReMapper, JvmApp from pants.backend.jvm.targets.jvm_binary import Duplicate, JarRules, JvmBinary, Skip from pants.backend.jvm.targets.jvm_prep_command import JvmPrepCommand @@ -62,11 +62,20 @@ from pants.backend.jvm.tasks.scaladoc_gen import ScaladocGen from pants.backend.jvm.tasks.scalafmt import ScalaFmtCheckFormat, ScalaFmtFormat from pants.backend.jvm.tasks.unpack_jars import UnpackJars +from pants.base.deprecated import warn_or_error from pants.build_graph.build_file_aliases import BuildFileAliases from pants.goal.goal import Goal from pants.goal.task_registrar import TaskRegistrar as task +class DeprecatedJavaTests(JUnitTests): + def __init__(self, *args, **kwargs): + super(DeprecatedJavaTests, self).__init__(*args, **kwargs) + warn_or_error('1.4.0', + 'java_tests(...) target type', + 'Use junit_tests(...) instead for target {}.'.format(self.address.spec)) + + def build_file_aliases(): return BuildFileAliases( targets={ @@ -77,8 +86,8 @@ def build_file_aliases(): 'java_agent': JavaAgent, 'java_library': JavaLibrary, 'javac_plugin': JavacPlugin, - 'java_tests': JavaTests, - 'junit_tests': JavaTests, + 'java_tests': DeprecatedJavaTests, + 'junit_tests': JUnitTests, 'jvm_app': JvmApp, 'jvm_binary': JvmBinary, 'jvm_prep_command' : JvmPrepCommand, diff --git a/src/python/pants/backend/jvm/targets/BUILD b/src/python/pants/backend/jvm/targets/BUILD index b2191953aaa..eee27ff16a5 100644 --- a/src/python/pants/backend/jvm/targets/BUILD +++ b/src/python/pants/backend/jvm/targets/BUILD @@ -19,6 +19,7 @@ python_library( 'jar_dependency.py', 'jar_library.py', 'jarable.py', + 'junit_tests.py', 'jvm_app.py', 'jvm_binary.py', 'jvm_prep_command.py', @@ -55,14 +56,15 @@ python_library( 'annotation_processor.py', 'java_agent.py', 'java_library.py', - 'javac_plugin.py', 'java_tests.py', + 'javac_plugin.py', ], dependencies = [ '3rdparty/python:six', ':jvm', 'src/python/pants/backend/jvm/subsystems:java', 'src/python/pants/backend/jvm/subsystems:junit', + 'src/python/pants/base:deprecated', 'src/python/pants/base:exceptions', 'src/python/pants/build_graph', ], @@ -77,7 +79,6 @@ python_library( ], dependencies = [ '3rdparty/python/twitter/commons:twitter.common.collections', - ':java', # For a dep on java_tests.py, which we might want to move to a separate target. ':jvm', 'src/python/pants/backend/jvm/subsystems:scala_platform', 'src/python/pants/base:exceptions', diff --git a/src/python/pants/backend/jvm/targets/java_library.py b/src/python/pants/backend/jvm/targets/java_library.py index 55450d5af7e..e199d2ea3dd 100644 --- a/src/python/pants/backend/jvm/targets/java_library.py +++ b/src/python/pants/backend/jvm/targets/java_library.py @@ -6,7 +6,7 @@ unicode_literals, with_statement) from pants.backend.jvm.targets.exportable_jvm_library import ExportableJvmLibrary -from pants.backend.jvm.targets.java_tests import JavaTests +from pants.backend.jvm.targets.junit_tests import JUnitTests class JavaLibrary(ExportableJvmLibrary): @@ -22,7 +22,7 @@ class JavaLibrary(ExportableJvmLibrary): """ default_sources_globs = '*.java' - default_sources_exclude_globs = JavaTests.java_test_globs + default_sources_exclude_globs = JUnitTests.java_test_globs @classmethod def subsystems(cls): diff --git a/src/python/pants/backend/jvm/targets/java_tests.py b/src/python/pants/backend/jvm/targets/java_tests.py index 5ac8533f2d8..fd018748eb9 100644 --- a/src/python/pants/backend/jvm/targets/java_tests.py +++ b/src/python/pants/backend/jvm/targets/java_tests.py @@ -5,121 +5,14 @@ from __future__ import (absolute_import, division, generators, nested_scopes, print_function, unicode_literals, with_statement) -from pants.backend.jvm.subsystems.junit import JUnit -from pants.backend.jvm.subsystems.jvm_platform import JvmPlatform -from pants.backend.jvm.targets.jvm_target import JvmTarget -from pants.base.exceptions import TargetDefinitionException -from pants.base.payload import Payload -from pants.base.payload_field import PrimitiveField +from pants.backend.jvm.targets.junit_tests import DeprecatedJavaTestsAlias +from pants.base.deprecated import warn_or_error -# TODO: Rename this to JUnitTests. It isn't just for Java language tests. -class JavaTests(JvmTarget): - """JUnit tests. +# Warn if any code imports this module. +# There's a separate deprecation warning in register.py for targets that use the old alias. +warn_or_error('1.4.0', + 'pants.backend.jvm.targets.java_tests.JavaTests', + 'Use pants.backend.jvm.targets.junit_tests.JUnitTests instead.') - :API: public - """ - - java_test_globs = ('*Test.java',) - scala_test_globs = ('*Test.scala', '*Spec.scala') - - default_sources_globs = java_test_globs + scala_test_globs - - CONCURRENCY_SERIAL = 'SERIAL' - CONCURRENCY_PARALLEL_CLASSES = 'PARALLEL_CLASSES' - CONCURRENCY_PARALLEL_METHODS = 'PARALLEL_METHODS' - CONCURRENCY_PARALLEL_CLASSES_AND_METHODS = 'PARALLEL_CLASSES_AND_METHODS' - VALID_CONCURRENCY_OPTS = [CONCURRENCY_SERIAL, - CONCURRENCY_PARALLEL_CLASSES, - CONCURRENCY_PARALLEL_METHODS, - CONCURRENCY_PARALLEL_CLASSES_AND_METHODS] - - @classmethod - def subsystems(cls): - return super(JavaTests, cls).subsystems() + (JUnit, ) - - def __init__(self, cwd=None, test_platform=None, payload=None, timeout=None, - extra_jvm_options=None, extra_env_vars=None, concurrency=None, - threads=None, **kwargs): - """ - :param str cwd: working directory (relative to the build root) for the tests under this - target. If unspecified (None), the working directory will be controlled by junit_run's --cwd. - :param str test_platform: The name of the platform (defined under the jvm-platform subsystem) to - use for running tests (that is, a key into the --jvm-platform-platforms dictionary). If - unspecified, the platform will default to the same one used for compilation. - :param int timeout: A timeout (in seconds) which covers the total runtime of all tests in this - target. Only applied if `--test-junit-timeouts` is set to True. - :param list extra_jvm_options: A list of key value pairs of jvm options to use when running the - tests. Example: ['-Dexample.property=1'] If unspecified, no extra jvm options will be added. - :param dict extra_env_vars: A map of environment variables to set when running the tests, e.g. - { 'FOOBAR': 12 }. Using `None` as the value will cause the variable to be unset. - :param string concurrency: One of 'SERIAL', 'PARALLEL_CLASSES', 'PARALLEL_METHODS', - or 'PARALLEL_CLASSES_AND_METHODS'. Overrides the setting of --test-junit-default-concurrency. - :param int threads: Use the specified number of threads when running the test. Overrides - the setting of --test-junit-parallel-threads. - """ - - payload = payload or Payload() - - if extra_env_vars is None: - extra_env_vars = {} - for key, value in extra_env_vars.items(): - if value is not None: - extra_env_vars[key] = str(value) - - payload.add_fields({ - 'test_platform': PrimitiveField(test_platform), - # TODO(zundel): Do extra_jvm_options and extra_env_vars really need to be fingerprinted? - 'extra_jvm_options': PrimitiveField(tuple(extra_jvm_options or ())), - 'extra_env_vars': PrimitiveField(tuple(extra_env_vars.items())), - }) - super(JavaTests, self).__init__(payload=payload, **kwargs) - - # These parameters don't need to go into the fingerprint: - self._concurrency = concurrency - self._cwd = cwd - self._threads = None - self._timeout = timeout - - try: - if threads is not None: - self._threads = int(threads) - except ValueError: - raise TargetDefinitionException(self, - "The value for 'threads' must be an integer, got " + threads) - if concurrency and concurrency not in self.VALID_CONCURRENCY_OPTS: - raise TargetDefinitionException(self, - "The value for 'concurrency' must be one of " - + repr(self.VALID_CONCURRENCY_OPTS) + " got: " + concurrency) - - # TODO(John Sirois): These could be scala, clojure, etc. 'jvm' and 'tests' are the only truly - # applicable labels - fixup the 'java' misnomer. - self.add_labels('java', 'tests') - - @property - def traversable_dependency_specs(self): - for spec in super(JavaTests, self).traversable_dependency_specs: - yield spec - yield JUnit.global_instance().library_spec(self._build_graph) - - @property - def test_platform(self): - if self.payload.test_platform: - return JvmPlatform.global_instance().get_platform_by_name(self.payload.test_platform) - return self.platform - - @property - def concurrency(self): - return self._concurrency - - @property - def cwd(self): - return self._cwd - - @property - def threads(self): - return self._threads - - @property - def timeout(self): - return self._timeout +JavaTests = DeprecatedJavaTestsAlias diff --git a/src/python/pants/backend/jvm/targets/junit_tests.py b/src/python/pants/backend/jvm/targets/junit_tests.py new file mode 100644 index 00000000000..fba386f920f --- /dev/null +++ b/src/python/pants/backend/jvm/targets/junit_tests.py @@ -0,0 +1,137 @@ +# coding=utf-8 +# Copyright 2014 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.jvm.subsystems.junit import JUnit +from pants.backend.jvm.subsystems.jvm_platform import JvmPlatform +from pants.backend.jvm.targets.jvm_target import JvmTarget +from pants.base.exceptions import TargetDefinitionException +from pants.base.payload import Payload +from pants.base.payload_field import PrimitiveField + + +class DeprecatedJavaTestsAlias(JvmTarget): + pass + + +# Subclasses DeprecatedJavaTestsAlias so that checks for +# isinstance(pants.backend.jvm.targets.java_tests.JavaTests) will +# return true for these instances during the migration. +# +# To migrate such code, change all BUILD files to use junit_tests instead of java_tests, +# to get rid of deprecation warnings for the java_tests BUILD file aliases. +# Then change the checks to isinstance(pants.backend.jvm.targets.junit_tests.JUnitTests), +# to get rid of the module-level deprecation warning for java_tests. +class JUnitTests(DeprecatedJavaTestsAlias): + """JUnit tests. + + :API: public + """ + + java_test_globs = ('*Test.java',) + scala_test_globs = ('*Test.scala', '*Spec.scala') + + default_sources_globs = java_test_globs + scala_test_globs + + + CONCURRENCY_SERIAL = 'SERIAL' + CONCURRENCY_PARALLEL_CLASSES = 'PARALLEL_CLASSES' + CONCURRENCY_PARALLEL_METHODS = 'PARALLEL_METHODS' + CONCURRENCY_PARALLEL_CLASSES_AND_METHODS = 'PARALLEL_CLASSES_AND_METHODS' + VALID_CONCURRENCY_OPTS = [CONCURRENCY_SERIAL, + CONCURRENCY_PARALLEL_CLASSES, + CONCURRENCY_PARALLEL_METHODS, + CONCURRENCY_PARALLEL_CLASSES_AND_METHODS] + + @classmethod + def subsystems(cls): + return super(JUnitTests, cls).subsystems() + (JUnit,) + + def __init__(self, cwd=None, test_platform=None, payload=None, timeout=None, + extra_jvm_options=None, extra_env_vars=None, concurrency=None, + threads=None, **kwargs): + """ + :param str cwd: working directory (relative to the build root) for the tests under this + target. If unspecified (None), the working directory will be controlled by junit_run's --cwd. + :param str test_platform: The name of the platform (defined under the jvm-platform subsystem) to + use for running tests (that is, a key into the --jvm-platform-platforms dictionary). If + unspecified, the platform will default to the same one used for compilation. + :param int timeout: A timeout (in seconds) which covers the total runtime of all tests in this + target. Only applied if `--test-junit-timeouts` is set to True. + :param list extra_jvm_options: A list of key value pairs of jvm options to use when running the + tests. Example: ['-Dexample.property=1'] If unspecified, no extra jvm options will be added. + :param dict extra_env_vars: A map of environment variables to set when running the tests, e.g. + { 'FOOBAR': 12 }. Using `None` as the value will cause the variable to be unset. + :param string concurrency: One of 'SERIAL', 'PARALLEL_CLASSES', 'PARALLEL_METHODS', + or 'PARALLEL_CLASSES_AND_METHODS'. Overrides the setting of --test-junit-default-concurrency. + :param int threads: Use the specified number of threads when running the test. Overrides + the setting of --test-junit-parallel-threads. + """ + + payload = payload or Payload() + + if extra_env_vars is None: + extra_env_vars = {} + for key, value in extra_env_vars.items(): + if value is not None: + extra_env_vars[key] = str(value) + + payload.add_fields({ + 'test_platform': PrimitiveField(test_platform), + # TODO(zundel): Do extra_jvm_options and extra_env_vars really need to be fingerprinted? + 'extra_jvm_options': PrimitiveField(tuple(extra_jvm_options or ())), + 'extra_env_vars': PrimitiveField(tuple(extra_env_vars.items())), + }) + super(JUnitTests, self).__init__(payload=payload, **kwargs) + + # These parameters don't need to go into the fingerprint: + self._concurrency = concurrency + self._cwd = cwd + self._threads = None + self._timeout = timeout + + try: + if threads is not None: + self._threads = int(threads) + except ValueError: + raise TargetDefinitionException(self, + "The value for 'threads' must be an integer, got " + threads) + if concurrency and concurrency not in self.VALID_CONCURRENCY_OPTS: + raise TargetDefinitionException(self, + "The value for 'concurrency' must be one of " + + repr(self.VALID_CONCURRENCY_OPTS) + " got: " + concurrency) + + # TODO(John Sirois): These could be scala, clojure, etc. 'jvm' and 'tests' are the only truly + # applicable labels - fixup the 'java' misnomer. + self.add_labels('java', 'tests') + + @property + def traversable_dependency_specs(self): + for spec in super(JUnitTests, self).traversable_dependency_specs: + yield spec + yield JUnit.global_instance().library_spec(self._build_graph) + + @property + def test_platform(self): + if self.payload.test_platform: + return JvmPlatform.global_instance().get_platform_by_name(self.payload.test_platform) + return self.platform + + @property + def concurrency(self): + return self._concurrency + + @property + def cwd(self): + return self._cwd + + @property + def threads(self): + return self._threads + + @property + def timeout(self): + return self._timeout diff --git a/src/python/pants/backend/jvm/targets/scala_library.py b/src/python/pants/backend/jvm/targets/scala_library.py index 2375cc941b9..4af453fcec4 100644 --- a/src/python/pants/backend/jvm/targets/scala_library.py +++ b/src/python/pants/backend/jvm/targets/scala_library.py @@ -7,7 +7,7 @@ from pants.backend.jvm.subsystems.scala_platform import ScalaPlatform from pants.backend.jvm.targets.exportable_jvm_library import ExportableJvmLibrary -from pants.backend.jvm.targets.java_tests import JavaTests +from pants.backend.jvm.targets.junit_tests import JUnitTests from pants.base.exceptions import TargetDefinitionException from pants.build_graph.address import Address @@ -25,7 +25,7 @@ class ScalaLibrary(ExportableJvmLibrary): """ default_sources_globs = '*.scala' - default_sources_exclude_globs = JavaTests.scala_test_globs + default_sources_exclude_globs = JUnitTests.scala_test_globs @classmethod def subsystems(cls): diff --git a/src/python/pants/backend/jvm/tasks/junit_run.py b/src/python/pants/backend/jvm/tasks/junit_run.py index dc2e7ae7d54..cddb8d916ed 100644 --- a/src/python/pants/backend/jvm/tasks/junit_run.py +++ b/src/python/pants/backend/jvm/tasks/junit_run.py @@ -16,7 +16,7 @@ from pants.backend.jvm import argfile from pants.backend.jvm.subsystems.junit import JUnit from pants.backend.jvm.subsystems.jvm_platform import JvmPlatform -from pants.backend.jvm.targets.java_tests import JavaTests as junit_tests +from pants.backend.jvm.targets.junit_tests import JUnitTests from pants.backend.jvm.targets.jvm_target import JvmTarget from pants.backend.jvm.tasks.classpath_util import ClasspathUtil from pants.backend.jvm.tasks.coverage.base import Coverage @@ -140,7 +140,7 @@ def register_options(cls, register): '[classname], [classname]#[methodname], [filename] or [filename]#[methodname]') register('--per-test-timer', type=bool, help='Show progress and timer for each test.') register('--default-concurrency', advanced=True, - choices=junit_tests.VALID_CONCURRENCY_OPTS, default=junit_tests.CONCURRENCY_SERIAL, + choices=JUnitTests.VALID_CONCURRENCY_OPTS, default=JUnitTests.CONCURRENCY_SERIAL, help='Set the default concurrency mode for running tests not annotated with' ' @TestParallel or @TestSerial.') register('--parallel-threads', advanced=True, type=int, default=0, @@ -232,13 +232,13 @@ def _args(self, output_dir): if options.per_test_timer: args.append('-per-test-timer') - if options.default_concurrency == junit_tests.CONCURRENCY_PARALLEL_CLASSES_AND_METHODS: + if options.default_concurrency == JUnitTests.CONCURRENCY_PARALLEL_CLASSES_AND_METHODS: if not options.use_experimental_runner: self.context.log.warn('--default-concurrency=PARALLEL_CLASSES_AND_METHODS is ' 'experimental, use --use-experimental-runner.') args.append('-default-concurrency') args.append('PARALLEL_CLASSES_AND_METHODS') - elif options.default_concurrency == junit_tests.CONCURRENCY_PARALLEL_METHODS: + elif options.default_concurrency == JUnitTests.CONCURRENCY_PARALLEL_METHODS: if not options.use_experimental_runner: self.context.log.warn('--default-concurrency=PARALLEL_METHODS is experimental, use ' '--use-experimental-runner.') @@ -249,10 +249,10 @@ def _args(self, output_dir): 'run classes in parallel too.') args.append('-default-concurrency') args.append('PARALLEL_METHODS') - elif options.default_concurrency == junit_tests.CONCURRENCY_PARALLEL_CLASSES: + elif options.default_concurrency == JUnitTests.CONCURRENCY_PARALLEL_CLASSES: args.append('-default-concurrency') args.append('PARALLEL_CLASSES') - elif options.default_concurrency == junit_tests.CONCURRENCY_SERIAL: + elif options.default_concurrency == JUnitTests.CONCURRENCY_SERIAL: args.append('-default-concurrency') args.append('SERIAL') @@ -390,13 +390,13 @@ def _run_tests(self, test_registry, output_dir, coverage=None): if concurrency is not None: args = remove_arg(args, '-default-parallel') - if concurrency == junit_tests.CONCURRENCY_SERIAL: + if concurrency == JUnitTests.CONCURRENCY_SERIAL: args = ensure_arg(args, '-default-concurrency', param='SERIAL') - elif concurrency == junit_tests.CONCURRENCY_PARALLEL_CLASSES: + elif concurrency == JUnitTests.CONCURRENCY_PARALLEL_CLASSES: args = ensure_arg(args, '-default-concurrency', param='PARALLEL_CLASSES') - elif concurrency == junit_tests.CONCURRENCY_PARALLEL_METHODS: + elif concurrency == JUnitTests.CONCURRENCY_PARALLEL_METHODS: args = ensure_arg(args, '-default-concurrency', param='PARALLEL_METHODS') - elif concurrency == junit_tests.CONCURRENCY_PARALLEL_CLASSES_AND_METHODS: + elif concurrency == JUnitTests.CONCURRENCY_PARALLEL_CLASSES_AND_METHODS: args = ensure_arg(args, '-default-concurrency', param='PARALLEL_CLASSES_AND_METHODS') if threads is not None: @@ -434,7 +434,7 @@ def error_handler(parse_error): .format(path=parse_error.junit_xml_path, cause=parse_error.cause)) target_to_failed_test = parse_failed_targets(test_registry, output_dir, error_handler) - failed_targets = sorted(target_to_failed_test, key=lambda target: target.address.spec) + failed_targets = sorted(target_to_failed_test, key=lambda t: t.address.spec) error_message_lines = [] if self._failure_summary: for target in failed_targets: @@ -478,20 +478,20 @@ def _calculate_tests_from_targets(self, targets): def _test_target_filter(self): def target_filter(target): - return isinstance(target, junit_tests) + return isinstance(target, JUnitTests) return target_filter def _validate_target(self, target): # TODO: move this check to an optional phase in goal_runner, so # that missing sources can be detected early. if not target.payload.sources.source_paths and not self.get_options().allow_empty_sources: - msg = 'JavaTests target must include a non-empty set of sources.' + msg = 'JUnitTests target must include a non-empty set of sources.' raise TargetDefinitionException(target, msg) def _execute(self, all_targets): - # NB: We only run tests within java_tests/junit_tests targets, but if coverage options are - # specified, we want to instrument and report on all the original targets, not just the test - # targets. + # NB: We only run tests within junit_tests targets, but if coverage options are + # specified, we want to instrument and report on all the original targets, not + # just the test targets. test_registry = self._collect_test_targets(self._get_test_targets()) if test_registry.empty: diff --git a/testprojects/maven_layout/provided_patching/leaf/BUILD b/testprojects/maven_layout/provided_patching/leaf/BUILD index b792f5f9606..7e953513ef8 100644 --- a/testprojects/maven_layout/provided_patching/leaf/BUILD +++ b/testprojects/maven_layout/provided_patching/leaf/BUILD @@ -42,7 +42,7 @@ jvm_binary(name='fail', ], ) -java_tests(name='test', +junit_tests(name='test', sources=[ 'src/test/java/org/pantsbuild/testproject/provided_patching/ShadowTest.java', ], diff --git a/testprojects/src/java/org/pantsbuild/testproject/junit/earlyexit/BUILD b/testprojects/src/java/org/pantsbuild/testproject/junit/earlyexit/BUILD index a42934a74b2..f9bbc5f842f 100644 --- a/testprojects/src/java/org/pantsbuild/testproject/junit/earlyexit/BUILD +++ b/testprojects/src/java/org/pantsbuild/testproject/junit/earlyexit/BUILD @@ -1,4 +1,4 @@ -java_tests(name='tests', +junit_tests(name='tests', sources=['AllTests.java'], dependencies=['3rdparty:junit'], ) diff --git a/testprojects/src/java/org/pantsbuild/testproject/junit/failing/tests/org/pantsbuild/tmp/tests/BUILD b/testprojects/src/java/org/pantsbuild/testproject/junit/failing/tests/org/pantsbuild/tmp/tests/BUILD index 858e8264cb0..1c5bd81fe7f 100644 --- a/testprojects/src/java/org/pantsbuild/testproject/junit/failing/tests/org/pantsbuild/tmp/tests/BUILD +++ b/testprojects/src/java/org/pantsbuild/testproject/junit/failing/tests/org/pantsbuild/tmp/tests/BUILD @@ -10,19 +10,19 @@ java_library(name='base', dependencies=['3rdparty:junit'], ) -java_tests(name='one', +junit_tests(name='one', sources=['OneTest.java'], dependencies=[':base'], strict_deps=False, ) -java_tests(name='two', +junit_tests(name='two', sources=['TwoTest.java'], dependencies=[':base'], strict_deps=False, ) -java_tests(name='three', +junit_tests(name='three', sources=['subtest/ThreeTest.java'], dependencies=[':base'], strict_deps=False, diff --git a/testprojects/src/java/org/pantsbuild/testproject/junit/mixed/tests/org/pantsbuild/tmp/tests/BUILD b/testprojects/src/java/org/pantsbuild/testproject/junit/mixed/tests/org/pantsbuild/tmp/tests/BUILD index 16af047a6f6..f972531043d 100644 --- a/testprojects/src/java/org/pantsbuild/testproject/junit/mixed/tests/org/pantsbuild/tmp/tests/BUILD +++ b/testprojects/src/java/org/pantsbuild/testproject/junit/mixed/tests/org/pantsbuild/tmp/tests/BUILD @@ -1,4 +1,4 @@ -java_tests( +junit_tests( sources=[ 'AllTestsBase.java', 'AllTests.java', diff --git a/testprojects/src/java/org/pantsbuild/testproject/junit/testscope/BUILD b/testprojects/src/java/org/pantsbuild/testproject/junit/testscope/BUILD index b438b30191d..7d2965fee40 100644 --- a/testprojects/src/java/org/pantsbuild/testproject/junit/testscope/BUILD +++ b/testprojects/src/java/org/pantsbuild/testproject/junit/testscope/BUILD @@ -1,7 +1,7 @@ # Copyright 2016 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -java_tests(name='tests', +junit_tests(name='tests', sources=['AllTests.java'], dependencies=[ '3rdparty:junit', diff --git a/testprojects/tests/java/org/pantsbuild/testproject/empty/BUILD b/testprojects/tests/java/org/pantsbuild/testproject/empty/BUILD index 2967780262c..bdb2c0a0861 100644 --- a/testprojects/tests/java/org/pantsbuild/testproject/empty/BUILD +++ b/testprojects/tests/java/org/pantsbuild/testproject/empty/BUILD @@ -1,4 +1,4 @@ # These sources intentionally missing. -java_tests( +junit_tests( sources=globs('*.java'), ) diff --git a/tests/java/org/pantsbuild/tools/junit/impl/BUILD b/tests/java/org/pantsbuild/tools/junit/impl/BUILD index 6a595d20f2f..66151e51210 100644 --- a/tests/java/org/pantsbuild/tools/junit/impl/BUILD +++ b/tests/java/org/pantsbuild/tools/junit/impl/BUILD @@ -1,7 +1,7 @@ # Copyright 2015 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -java_tests( +junit_tests( name='junit', dependencies=[ '3rdparty:guava', diff --git a/tests/python/pants_test/backend/jvm/targets/BUILD b/tests/python/pants_test/backend/jvm/targets/BUILD index 40b81267f73..6abb1b3522a 100644 --- a/tests/python/pants_test/backend/jvm/targets/BUILD +++ b/tests/python/pants_test/backend/jvm/targets/BUILD @@ -30,8 +30,8 @@ python_tests( ) python_tests( - name='java_tests', - sources=['test_java_tests.py'], + name='junit_tests', + sources=['test_junit_tests.py'], dependencies=[ 'src/python/pants/backend/jvm/subsystems:junit', 'src/python/pants/backend/jvm/targets:jvm', diff --git a/tests/python/pants_test/backend/jvm/targets/test_java_tests.py b/tests/python/pants_test/backend/jvm/targets/test_junit_tests.py similarity index 59% rename from tests/python/pants_test/backend/jvm/targets/test_java_tests.py rename to tests/python/pants_test/backend/jvm/targets/test_junit_tests.py index f0c5e9f51b8..5527a924575 100644 --- a/tests/python/pants_test/backend/jvm/targets/test_java_tests.py +++ b/tests/python/pants_test/backend/jvm/targets/test_junit_tests.py @@ -6,70 +6,70 @@ unicode_literals, with_statement) from pants.backend.jvm.subsystems.junit import JUnit -from pants.backend.jvm.targets.java_tests import JavaTests +from pants.backend.jvm.targets.junit_tests import JUnitTests from pants.base.exceptions import TargetDefinitionException from pants.build_graph.target import Target from pants_test.base_test import BaseTest from pants_test.subsystem.subsystem_util import init_subsystem -class JavaTestsTest(BaseTest): +class JUnitTestsTest(BaseTest): def test_validation(self): init_subsystem(JUnit) target = self.make_target('//:mybird', Target) # A plain invocation with no frills - test1 = self.make_target('//:test1', JavaTests, sources=['Test.java'], dependencies=[target]) + test1 = self.make_target('//:test1', JUnitTests, sources=['Test.java'], dependencies=[target]) self.assertIsNone(test1.cwd) self.assertIsNone(test1.concurrency) self.assertIsNone(test1.threads) self.assertIsNone(test1.timeout) # cwd parameter - testcwd = self.make_target('//:testcwd1', JavaTests, sources=['Test.java'], + testcwd = self.make_target('//:testcwd1', JUnitTests, sources=['Test.java'], concurrency='SERIAL', cwd='/foo/bar') self.assertEquals('/foo/bar', testcwd.cwd) # concurrency parameter - tc1 = self.make_target('//:testconcurrency1', JavaTests, sources=['Test.java'], + tc1 = self.make_target('//:testconcurrency1', JUnitTests, sources=['Test.java'], concurrency='SERIAL') - self.assertEquals(JavaTests.CONCURRENCY_SERIAL, tc1.concurrency) - tc2 = self.make_target('//:testconcurrency2', JavaTests, sources=['Test.java'], + self.assertEquals(JUnitTests.CONCURRENCY_SERIAL, tc1.concurrency) + tc2 = self.make_target('//:testconcurrency2', JUnitTests, sources=['Test.java'], concurrency='PARALLEL_CLASSES') - self.assertEquals(JavaTests.CONCURRENCY_PARALLEL_CLASSES, tc2.concurrency) - tc3 = self.make_target('//:testconcurrency3', JavaTests, sources=['Test.java'], + self.assertEquals(JUnitTests.CONCURRENCY_PARALLEL_CLASSES, tc2.concurrency) + tc3 = self.make_target('//:testconcurrency3', JUnitTests, sources=['Test.java'], concurrency='PARALLEL_METHODS') - self.assertEquals(JavaTests.CONCURRENCY_PARALLEL_METHODS, tc3.concurrency) - tc4 = self.make_target('//:testconcurrency4', JavaTests, sources=['Test.java'], + self.assertEquals(JUnitTests.CONCURRENCY_PARALLEL_METHODS, tc3.concurrency) + tc4 = self.make_target('//:testconcurrency4', JUnitTests, sources=['Test.java'], concurrency='PARALLEL_CLASSES_AND_METHODS') - self.assertEquals(JavaTests.CONCURRENCY_PARALLEL_CLASSES_AND_METHODS, tc4.concurrency) + self.assertEquals(JUnitTests.CONCURRENCY_PARALLEL_CLASSES_AND_METHODS, tc4.concurrency) with self.assertRaisesRegexp(TargetDefinitionException, r'concurrency'): - self.make_target('//:testconcurrency5', JavaTests, sources=['Test.java'], + self.make_target('//:testconcurrency5', JUnitTests, sources=['Test.java'], concurrency='nonsense') # threads parameter - tt1 = self.make_target('//:testthreads1', JavaTests, sources=['Test.java'], threads=99) + tt1 = self.make_target('//:testthreads1', JUnitTests, sources=['Test.java'], threads=99) self.assertEquals(99, tt1.threads) - tt2 = self.make_target('//:testthreads2', JavaTests, sources=['Test.java'], threads="123") + tt2 = self.make_target('//:testthreads2', JUnitTests, sources=['Test.java'], threads="123") self.assertEquals(123, tt2.threads) with self.assertRaisesRegexp(TargetDefinitionException, r'threads'): - self.make_target('//:testthreads3', JavaTests, sources=['Test.java'], threads="abc") + self.make_target('//:testthreads3', JUnitTests, sources=['Test.java'], threads="abc") # timeout parameter - timeout = self.make_target('//:testtimeout1', JavaTests, sources=['Test.java'], timeout=999) + timeout = self.make_target('//:testtimeout1', JUnitTests, sources=['Test.java'], timeout=999) self.assertEquals(999, timeout.timeout) def test_implicit_junit_dep(self): init_subsystem(JUnit) # Check that the implicit dep is added, and doesn't replace other deps. target = self.make_target('//:target', Target) - test1 = self.make_target('//:test1', JavaTests, sources=[], dependencies=[target]) + test1 = self.make_target('//:test1', JUnitTests, sources=[], dependencies=[target]) self.assertEquals(['JarLibrary(//:junit_library)', 'Target(//:target)'], sorted(str(x) for x in test1.dependencies)) # Check that having an explicit dep doesn't cause problems. junit_target = self.build_graph.get_target_from_spec('//:junit_library') - test2 = self.make_target('//:test2', JavaTests, sources=[], + test2 = self.make_target('//:test2', JUnitTests, sources=[], dependencies=[junit_target, target]) self.assertEquals(['JarLibrary(//:junit_library)', 'Target(//:target)'], sorted(str(x) for x in test2.dependencies)) diff --git a/tests/python/pants_test/backend/jvm/tasks/test_junit_run.py b/tests/python/pants_test/backend/jvm/tasks/test_junit_run.py index 87d01bf9f74..0fcd72544e7 100644 --- a/tests/python/pants_test/backend/jvm/tasks/test_junit_run.py +++ b/tests/python/pants_test/backend/jvm/tasks/test_junit_run.py @@ -12,7 +12,7 @@ from mock import patch from pants.backend.jvm.subsystems.junit import JUnit -from pants.backend.jvm.targets.java_tests import JavaTests +from pants.backend.jvm.targets.junit_tests import JUnitTests from pants.backend.jvm.tasks.junit_run import JUnitRun from pants.backend.python.targets.python_tests import PythonTests from pants.base.exceptions import TargetDefinitionException, TaskError @@ -40,7 +40,7 @@ def task_type(cls): def alias_groups(self): return super(JUnitRunnerTest, self).alias_groups.merge(BuildFileAliases( targets={ - 'java_tests': JavaTests, + 'junit_tests': JUnitTests, 'python_tests': PythonTests, }, )) @@ -181,11 +181,11 @@ def _execute_junit_runner(self, list_of_filename_content_tuples, create_some_res subprocess.check_call( [javac, '-d', test_classes_abs_path, '-cp', classpath] + test_java_file_abs_paths) - # If a target_name is specified create a target with it, otherwise create a java_tests target. + # If a target_name is specified create a target with it, otherwise create a junit_tests target. if 'target_name' in kwargs: target = self.target(kwargs['target_name']) else: - target = self.create_library(test_rel_path, 'java_tests', 'foo_test', ['FooTest.java']) + target = self.create_library(test_rel_path, 'junit_tests', 'foo_test', ['FooTest.java']) target_roots = [] if create_some_resources: @@ -193,7 +193,7 @@ def _execute_junit_runner(self, list_of_filename_content_tuples, create_some_res target_roots.append(self.make_target('some_resources', Resources)) target_roots.append(target) - # Set the context with the two targets, one java_tests target and + # Set the context with the two targets, one junit_tests target and # one synthetic resources target. # The synthetic resources target is to make sure we won't regress # in the future with bug like https://github.com/pantsbuild/pants/issues/508. Note @@ -226,7 +226,7 @@ def test_junit_runner_raises_no_error_on_non_junit_target(self): def test_empty_sources(self): self.add_to_build_file('foo', dedent(""" - java_tests( + junit_tests( name='empty', sources=[], ) @@ -239,7 +239,7 @@ def test_empty_sources(self): def test_allow_empty_sources(self): self.add_to_build_file('foo', dedent(""" - java_tests( + junit_tests( name='empty', sources=[], ) @@ -266,7 +266,7 @@ def test_request_classes_by_source(self): def test_junit_runner_extra_jvm_options(self): self.make_target( spec='foo:foo_test', - target_type=JavaTests, + target_type=JUnitTests, sources=['FooTest.java'], extra_jvm_options=['-Dexample.property=1'], ) @@ -285,7 +285,7 @@ def test_junit_runner_extra_jvm_options(self): def test_junit_runner_multiple_extra_jvm_options(self): self.make_target( spec='foo:foo_test', - target_type=JavaTests, + target_type=JUnitTests, sources=['FooTest.java'], extra_jvm_options=['-Dexample.property1=1','-Dexample.property2=2'], ) @@ -308,7 +308,7 @@ def test_junit_runner_multiple_extra_jvm_options(self): def test_junit_runner_extra_env_vars(self): self.make_target( spec='foo:foo_test', - target_type=JavaTests, + target_type=JUnitTests, sources=['FooTest.java'], extra_env_vars={ 'HELLO': 27, @@ -318,7 +318,7 @@ def test_junit_runner_extra_env_vars(self): self.make_target( spec='bar:bar_test', - target_type=JavaTests, + target_type=JUnitTests, sources=['FooTest.java'], extra_env_vars={ 'THE_ANSWER': 42, @@ -360,7 +360,7 @@ def test_junit_runner_extra_env_vars_none(self): with environment_as(THIS_VARIABLE="12", THAT_VARIABLE="This is a variable."): self.make_target( spec='foo:foo_test', - target_type=JavaTests, + target_type=JUnitTests, sources=['FooTest.java'], extra_env_vars={ 'HELLO': None, @@ -404,7 +404,7 @@ def test_junt_run_with_too_many_args(self): self.make_target( spec='foo:foo_test', - target_type=JavaTests, + target_type=JUnitTests, sources=[name for name, _ in list_of_filename_content_tuples], ) self.set_options(max_subprocess_args=max_subprocess_args) diff --git a/tests/python/pants_test/backend/jvm/tasks/test_scope_test_integration.py b/tests/python/pants_test/backend/jvm/tasks/test_scope_test_integration.py index 6be874bca49..3024aa21d67 100644 --- a/tests/python/pants_test/backend/jvm/tasks/test_scope_test_integration.py +++ b/tests/python/pants_test/backend/jvm/tasks/test_scope_test_integration.py @@ -14,7 +14,7 @@ class ScopeTestIntegrationTest(PantsRunIntegrationTest): These tests involve a library which has the 'test' scope, meaning it should only be available at runtime, and only for junit tests. - There is a java_tests() target which depends on it, and checks for its existence at runtime (not + There is a junit_tests() target which depends on it, and checks for its existence at runtime (not compile time!) by using Class.forName(). A binary is configured to also check for the existence of the class at runtime. The binary should @@ -27,7 +27,7 @@ def _spec(cls, name): return 'testprojects/src/java/org/pantsbuild/testproject/junit/testscope:{}'.format(name) def test_tests_pass(self): - """This java_tests() target tests for the presence of a particular class at runtime. + """This junit_tests() target tests for the presence of a particular class at runtime. It should be included because it has the 'test' scope. """ diff --git a/tests/python/pants_test/backend/project_info/tasks/test_export.py b/tests/python/pants_test/backend/project_info/tasks/test_export.py index ecb81a8bc52..d717598097a 100644 --- a/tests/python/pants_test/backend/project_info/tasks/test_export.py +++ b/tests/python/pants_test/backend/project_info/tasks/test_export.py @@ -18,7 +18,7 @@ from pants.backend.jvm.targets.jar_dependency import JarDependency from pants.backend.jvm.targets.jar_library import JarLibrary from pants.backend.jvm.targets.java_library import JavaLibrary -from pants.backend.jvm.targets.java_tests import JavaTests +from pants.backend.jvm.targets.junit_tests import JUnitTests from pants.backend.jvm.targets.jvm_app import JvmApp from pants.backend.jvm.targets.jvm_binary import JvmBinary from pants.backend.jvm.targets.jvm_target import JvmTarget @@ -117,7 +117,7 @@ def setUp(self): self.make_target( 'project_info:java_test', - target_type=JavaTests, + target_type=JUnitTests, dependencies=[jar_lib], sources=['this/is/a/test/source/FooTest.scala'], resources=[test_resource.address.spec], diff --git a/tests/python/pants_test/test_maven_layout.py b/tests/python/pants_test/test_maven_layout.py index 6b0b1b69546..84bbbdf58e8 100644 --- a/tests/python/pants_test/test_maven_layout.py +++ b/tests/python/pants_test/test_maven_layout.py @@ -7,7 +7,7 @@ from pants.backend.jvm.subsystems.junit import JUnit from pants.backend.jvm.targets.java_library import JavaLibrary -from pants.backend.jvm.targets.java_tests import JavaTests +from pants.backend.jvm.targets.junit_tests import JUnitTests from pants.build_graph.build_file_aliases import BuildFileAliases from pants.source.source_root import SourceRootConfig from pants_test.base_test import BaseTest @@ -22,7 +22,7 @@ def alias_groups(self): return BuildFileAliases( targets={ 'java_library': JavaLibrary, - 'junit_tests': JavaTests, + 'junit_tests': JUnitTests, }, ) From 0abade1c40309ad63d22d17cef9e301aa9b8f8c6 Mon Sep 17 00:00:00 2001 From: John Sirois Date: Sat, 22 Oct 2016 16:23:10 -0400 Subject: [PATCH 6/7] Fix the SetupPy target ownership check. Previously the check was too restrictive and considered targets that did not own files when the single publish ownership check is only intended to prevent publishing the same file in more than one package. Testing Done: Tested this over in Aurora ad-hoc and it solved the publish problem there as well as passing the new test emulating the Aurora `prep_command` arrangement. CI went green here: https://travis-ci.org/pantsbuild/pants/builds/169610485 Bugs closed: 3968, 3969 Reviewed at https://rbcommons.com/s/twitter/r/4315/ --- .../pants/backend/python/tasks/setup_py.py | 20 +++--- .../backend/python/tasks/test_setup_py.py | 61 +++++++++++++++++-- 2 files changed, 68 insertions(+), 13 deletions(-) diff --git a/src/python/pants/backend/python/tasks/setup_py.py b/src/python/pants/backend/python/tasks/setup_py.py index f767d1d2e71..f3f1926e82b 100644 --- a/src/python/pants/backend/python/tasks/setup_py.py +++ b/src/python/pants/backend/python/tasks/setup_py.py @@ -131,11 +131,12 @@ def __init__(self, build_graph): self._ancestor_iterator = TargetAncestorIterator(build_graph) @abstractmethod - def is_third_party(self, target): - """Identifies targets that are exported by third parties. + def requires_export(self, target): + """Identifies targets that need to be exported (are internal targets owning source code). :param target: The target to identify. - :returns: `True` if the given `target` represents a third party dependency. + :returns: `True` if the given `target` owns files that should be included in exported packages + when the target is a member of an exported target's dependency graph. """ @abstractmethod @@ -228,14 +229,13 @@ def reduced_dependencies(self, exported_target): owner_by_owned_python_target = OrderedDict() def collect_potentially_owned_python_targets(current): - if (current != exported_target) and not self.is_third_party(current): - owner_by_owned_python_target[current] = None # We can't know the owner in the 1st pass. + owner_by_owned_python_target[current] = None # We can't know the owner in the 1st pass. return (current == exported_target) or not self.is_exported(current) self._walk(exported_target, collect_potentially_owned_python_targets) for owned in owner_by_owned_python_target: - if not self.is_exported(owned): + if self.requires_export(owned) and not self.is_exported(owned): potential_owners = set() for potential_owner in self._ancestor_iterator.iter_target_siblings_and_ancestors(owned): if self.is_exported(potential_owner) and owned in self._closure(potential_owner): @@ -271,7 +271,7 @@ def collect_reduced_dependencies(current): reduced_dependencies.add(current) else: reduced_dependencies.add(owner) - return owner == exported_target + return owner == exported_target or not self.requires_export(current) self._walk(exported_target, collect_reduced_dependencies) return reduced_dependencies @@ -301,8 +301,10 @@ def has_provides(cls, target): class DependencyCalculator(ExportedTargetDependencyCalculator): """Calculates reduced dependencies for exported python targets.""" - def is_third_party(self, target): - return SetupPy.is_requirements(target) + def requires_export(self, target): + # TODO(John Sirois): Consider switching to the more general target.has_sources() once Benjy's + # change supporting default globs is in (that change will smooth test migration). + return SetupPy.is_python_target(target) or SetupPy.is_resources_target(target) def is_exported(self, target): return SetupPy.has_provides(target) diff --git a/tests/python/pants_test/backend/python/tasks/test_setup_py.py b/tests/python/pants_test/backend/python/tasks/test_setup_py.py index 3b826d8c975..f43c83bf8a7 100644 --- a/tests/python/pants_test/backend/python/tasks/test_setup_py.py +++ b/tests/python/pants_test/backend/python/tasks/test_setup_py.py @@ -21,6 +21,7 @@ from pants.backend.python.tasks.setup_py import SetupPy from pants.base.exceptions import TaskError from pants.build_graph.build_file_aliases import BuildFileAliases +from pants.build_graph.prep_command import PrepCommand from pants.build_graph.resources import Resources from pants.build_graph.target import Target from pants.fs.archive import TGZ @@ -45,8 +46,10 @@ def setUp(self): @property def alias_groups(self): - resources = BuildFileAliases(targets={'resources': Resources}) - return super(TestSetupPy, self).alias_groups.merge(resources) + extra_aliases = BuildFileAliases(targets={'prep_command': PrepCommand, + 'resources': Resources, + 'target': Target}) + return super(TestSetupPy, self).alias_groups.merge(extra_aliases) def create_dependencies(self, depmap): target_map = {} @@ -307,7 +310,7 @@ def test_no_owner(self): def test_ambiguous_owner(self): self.create_python_library(relpath='foo/bar', name='bar') - self.create_file(relpath=self.build_path('foo'), contents=dedent(""" + self.add_to_build_file('foo', dedent(""" python_library( name='foo1', dependencies=[ @@ -337,7 +340,7 @@ def test_ambiguous_owner(self): self.dependency_calculator.reduced_dependencies(self.target('foo:foo2')) @contextmanager - def extracted_sdist(src, sdist, expected_prefix, collect_suffixes=None): + def extracted_sdist(self, sdist, expected_prefix, collect_suffixes=None): collect_suffixes = collect_suffixes or ('.py',) def collect(path): @@ -552,6 +555,56 @@ def test_exported_thrift_issues_2005(self): with open(requirements) as fp: self.assertEqual('test.exported==0.0.0', fp.read().strip()) + def test_prep_command_case(self): + PrepCommand.add_allowed_goal('compile') + PrepCommand.add_allowed_goal('test') + self.add_to_build_file('build-support/thrift', + dedent(""" + prep_command( + name='prepare_binary_compile', + goal='compile', + prep_executable='/bin/true', + ) + + prep_command( + name='prepare_binary_test', + goal='test', + prep_executable='/bin/true', + ) + + target( + name='prepare_binary', + dependencies=[ + ':prepare_binary_compile', + ':prepare_binary_test', + ], + ) + """)) + prepare_binary_compile = self.make_target(spec='build-support/thrift:prepare_binary_compile', + target_type=PrepCommand, + prep_executable='/bin/true', + goal='compile') + prepare_binary_test = self.make_target(spec='build-support/thrift:prepare_binary_test', + target_type=PrepCommand, + prep_executable='/bin/true', + goal='test') + self.make_target(spec='build-support/thrift:prepare_binary', + dependencies=[ + prepare_binary_compile, + prepare_binary_test + ]) + + pants = self.create_python_library( + relpath='src/python/pants', + name='pants', + provides="setup_py(name='pants', version='0.0.0')", + dependencies=[ + 'build-support/thrift:prepare_binary' + ] + ) + with self.run_execute(pants) as created: + self.assertEqual([pants], created.keys()) + def test_detect_namespace_packages(): def has_ns(stmt): From 28bbb0f92a18e52db518e0b2d97137b4054f8df4 Mon Sep 17 00:00:00 2001 From: Eric Ayers Date: Mon, 24 Oct 2016 07:26:40 -0400 Subject: [PATCH 7/7] bogus fix for issue 261 --- src/python/pants/backend/python/code_generator.py | 5 +++-- src/python/pants/backend/python/sdist_builder.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/python/pants/backend/python/code_generator.py b/src/python/pants/backend/python/code_generator.py index 297d6ae0ef0..dd5823f820f 100644 --- a/src/python/pants/backend/python/code_generator.py +++ b/src/python/pants/backend/python/code_generator.py @@ -90,7 +90,8 @@ def sdist_root(self): def package_root(self): return os.path.join(self.sdist_root, self.package_dir) - def build(self, interpreter=None): + def build(self, interpreter=None, distdir='dist'): self.generate() self.dump_setup_py() - return SdistBuilder.build(self.sdist_root, self.target, interpreter=interpreter) + return SdistBuilder.build(self.sdist_root, self.target, interpreter=interpreter, + distdir) diff --git a/src/python/pants/backend/python/sdist_builder.py b/src/python/pants/backend/python/sdist_builder.py index 27cd940a3e1..b10db902206 100644 --- a/src/python/pants/backend/python/sdist_builder.py +++ b/src/python/pants/backend/python/sdist_builder.py @@ -17,9 +17,9 @@ class Error(Exception): pass class SetupError(Error): pass @classmethod - def build(cls, setup_root, target, interpreter=None): + def build(cls, setup_root, target, interpreter=None, distdir='dist'): packager = Packager(setup_root, interpreter=interpreter, - install_dir=os.path.join(setup_root, 'dist')) + install_dir=os.path.join(setup_root, distdir)) try: return packager.sdist() except Packager.InstallFailure as e: