Zundel/test GitHub review

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 <target>

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:

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]

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"

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',

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
@@ -60,13 +60,22 @@
                                                           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.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,
@@ -189,7 +198,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)

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',

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):

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