Merge 4676402 into 1210b1f

src/python/pants/backend/python/tasks/BUILD

@@ -23,6 +23,7 @@ python_library(
     'src/python/pants/base:address_lookup_error',
     'src/python/pants/base:build_environment',
     'src/python/pants/base:build_graph',
+    'src/python/pants/base:deprecated',
     'src/python/pants/base:exceptions',
     'src/python/pants/base:generator',
     'src/python/pants/base:target',

src/python/pants/backend/python/tasks/pytest_run.py

@@ -25,6 +25,7 @@
 from pants.backend.python.python_setup import PythonRepos, PythonSetup
 from pants.backend.python.targets.python_tests import PythonTests
 from pants.backend.python.tasks.python_task import PythonTask
+from pants.base.deprecated import deprecated
 from pants.base.exceptions import TaskError, TestFailedTaskError
 from pants.base.target import Target
 from pants.base.workunit import WorkUnit
@@ -67,6 +68,43 @@ def failed_targets(self):
     return self._failed_targets
 
 
+def deprecated_env_accessors(removal_version, **replacement_mapping):
+  """Generates accessors for legacy env ver/replacement option pairs.
+
+  The generated accessors issue a deprecation warning when the deprecated env var is present and
+  enjoy the "compile" time removal forcing that normal @deprecated functions and methods do.
+  """
+  def create_accessor(env_name, option_name):
+    @deprecated(removal_version=removal_version,
+                hint_message=' Use the {option} option instead of the deprecated {env} environment'
+                             'variable'.format(option=option_name, env=env_name))
+    def deprecated_accessor():
+      return os.environ.get(env_name)
+
+    def accessor(self):
+      value = None
+      if env_name in os.environ:
+        value = deprecated_accessor()
+      sanitized_option_name = option_name.lstrip('-').replace('-', '_')
+      value = self.get_options()[sanitized_option_name] or value
+      return value
+    return accessor
+
+  def decorator(clazz):
+    for env_name, option_name in replacement_mapping.items():
+      setattr(clazz, 'get_DEPRECATED_{}'.format(env_name), create_accessor(env_name, option_name))
+    return clazz
+
+  return decorator
+
+
+# TODO(John Sirois): Replace this helper and use of the accessors it generates with direct options
+# access prior to releasing 0.0.35
+@deprecated_env_accessors(removal_version='0.0.35',
+                          JUNIT_XML_BASE='--junit-xml-dir',
+                          PANTS_PROFILE='--profile',
+                          PANTS_PYTHON_TEST_FAILSOFT='--fail-slow',
+                          PANTS_PY_COVERAGE='--coverage')
 class PytestRun(PythonTask):
   _TESTING_TARGETS = [
     # Note: the requirement restrictions on pytest and pytest-cov match those in requirements.txt,
@@ -90,6 +128,17 @@ def register_options(cls, register):
              help='Run all tests in a single chroot. If turned off, each test target will '
                   'create a new chroot, which will be much slower, but more correct, as the'
                   'isolation verifies that all dependencies are correctly declared.')
+    register('--fail-slow', action='store_true', default=False,
+             help='Do not fail fast on the first test failure in a suite; instead run all tests '
+                  'and report errors only after all tests complete.')
+    register('--junit-xml-dir', metavar='<DIR>',
+             help='Specifying a directory causes junit xml results files to be emitted under '
+                  'that dir for each test run.')
+    register('--profile', metavar='<FILE>',
+             help="Specifying a file path causes tests to be profiled with the profiling data "
+                  "emitted to that file (prefix). Note that tests may run in a different cwd, so "
+                  "it's best to use an absolute path to make it easy to find the subprocess "
+                  "profiles later.")
     register('--options', action='append', help='Pass these options to pytest.')
     register('--coverage',
              help='Emit coverage information for specified paths/modules. Value has two forms: '
@@ -104,10 +153,6 @@ def supports_passthru_args(cls):
 
   def execute(self):
     def is_python_test(target):
-      # Note that we ignore PythonTestSuite, because we'll see the PythonTests targets
-      # it depends on anyway,so if we don't we'll end up running the tests twice.
-      # TODO(benjy): Once we're off the 'build' command we can get rid of python_test_suite,
-      # or make it an alias of dependencies().
       return isinstance(target, PythonTests)
 
     test_targets = list(filter(is_python_test, self.context.targets()))
@@ -131,8 +176,8 @@ def run_tests(self, targets, workunit):
     else:
       results = {}
       # Coverage often throws errors despite tests succeeding, so force failsoft in that case.
-      fail_hard = ('PANTS_PYTHON_TEST_FAILSOFT' not in os.environ and
-                   'PANTS_PY_COVERAGE' not in os.environ)
+      fail_hard = (not self.get_DEPRECATED_PANTS_PYTHON_TEST_FAILSOFT() and
+                   not self.get_DEPRECATED_PANTS_PY_COVERAGE())
       for target in targets:
         if isinstance(target, PythonTests):
           rv = self._do_run_tests([target], workunit)
@@ -203,7 +248,8 @@ def pytest_collection_modifyitems(session, config, items):
   @contextmanager
   def _maybe_emit_junit_xml(self, targets):
     args = []
-    xml_base = os.getenv('JUNIT_XML_BASE')
+    xml_base = self.get_DEPRECATED_JUNIT_XML_BASE()
+    print('>>> found junit-xml of: {}'.format(xml_base))
     if xml_base and targets:
       xml_base = os.path.realpath(xml_base)
       xml_path = os.path.join(xml_base, Target.maybe_readable_identify(targets) + '.xml')
@@ -332,7 +378,7 @@ def is_python_lib(tgt):
 
   @contextmanager
   def _maybe_emit_coverage_data(self, targets, chroot, pex, workunit):
-    coverage = os.environ.get('PANTS_PY_COVERAGE')
+    coverage = self.get_DEPRECATED_PANTS_PY_COVERAGE()
     if coverage is None:
       yield []
       return
@@ -432,12 +478,9 @@ def _do_run_tests_with_args(self, pex, workunit, args):
       env = {
         'PYTHONUNBUFFERED': '1',
       }
-      # If profiling a test run, this will enable profiling on the test code itself.
-      # Note that tests may run in a different cwd, so it's best to set PANTS_PROFILE
-      # to an absolute path to make it easy to find the subprocess profiles later.
-      if 'PANTS_PROFILE' in os.environ:
-        env['PEX_PROFILE'] = '{0}.subprocess.{1:.6f}'.format(os.environ['PANTS_PROFILE'],
-                                                             time.time())
+      profile = self.get_DEPRECATED_PANTS_PROFILE()
+      if profile:
+        env['PEX_PROFILE'] = '{0}.subprocess.{1:.6f}'.format(profile, time.time())
       with environment_as(**env):
         rc = self._pex_run(pex, workunit, args=args, setsid=True)
         return PythonTestResult.rc(rc)

tests/python/pants_test/backend/python/tasks/test_pytest_run.py

@@ -158,36 +158,40 @@ def test_red(self):
   def test_mixed(self):
     self.run_failing_tests(targets=[self.green, self.red], failed_targets=[self.red])
 
-  def test_junit_xml(self):
+  def assert_expected_junit_xml(self, report_basedir, **kwargs):
     # We expect xml of the following form:
     # <testsuite errors=[Ne] failures=[Nf] skips=[Ns] tests=[Nt] ...>
     #   <testcase classname="..." name="..." .../>
     #   <testcase classname="..." name="..." ...>
     #     <failure ...>...</failure>
     #   </testcase>
     # </testsuite>
-
-    report_basedir = os.path.join(self.build_root, 'dist', 'junit')
-    with environment_as(JUNIT_XML_BASE=report_basedir):
-      self.run_failing_tests(targets=[self.red, self.green], failed_targets=[self.red])
-
-      files = glob.glob(os.path.join(report_basedir, '*.xml'))
-      self.assertEqual(1, len(files))
-      junit_xml = files[0]
-      with open(junit_xml) as fp:
-        print(fp.read())
-
-      root = DOM.parse(junit_xml).documentElement
-      self.assertEqual(2, len(root.childNodes))
-      self.assertEqual(2, int(root.getAttribute('tests')))
-      self.assertEqual(1, int(root.getAttribute('failures')))
-      self.assertEqual(0, int(root.getAttribute('errors')))
-      self.assertEqual(0, int(root.getAttribute('skips')))
-
-      children_by_test_name = dict((elem.getAttribute('name'), elem) for elem in root.childNodes)
-      self.assertEqual(0, len(children_by_test_name['test_one'].childNodes))
-      self.assertEqual(1, len(children_by_test_name['test_two'].childNodes))
-      self.assertEqual('failure', children_by_test_name['test_two'].firstChild.nodeName)
+    self.run_failing_tests(targets=[self.green, self.red], failed_targets=[self.red], **kwargs)
+
+    files = glob.glob(os.path.join(report_basedir, '*.xml'))
+    self.assertEqual(1, len(files), 'Expected 1 file, found: {}'.format(files))
+    junit_xml = files[0]
+    with open(junit_xml) as fp:
+      print(fp.read())
+    root = DOM.parse(junit_xml).documentElement
+    self.assertEqual(2, len(root.childNodes))
+    self.assertEqual(2, int(root.getAttribute('tests')))
+    self.assertEqual(1, int(root.getAttribute('failures')))
+    self.assertEqual(0, int(root.getAttribute('errors')))
+    self.assertEqual(0, int(root.getAttribute('skips')))
+    children_by_test_name = dict((elem.getAttribute('name'), elem) for elem in root.childNodes)
+    self.assertEqual(0, len(children_by_test_name['test_one'].childNodes))
+    self.assertEqual(1, len(children_by_test_name['test_two'].childNodes))
+    self.assertEqual('failure', children_by_test_name['test_two'].firstChild.nodeName)
+
+  def test_junit_xml_option(self):
+    basedir = os.path.join(self.build_root, 'dist', 'junit_option')
+    self.assert_expected_junit_xml(basedir, junit_xml_dir=basedir)
+
+  def test_junit_xml_env(self):
+    basedir = os.path.join(self.build_root, 'dist', 'junit_env')
+    with environment_as(JUNIT_XML_BASE=basedir):
+      self.assert_expected_junit_xml(basedir)
 
   def coverage_data_file(self):
     return os.path.join(self.build_root, '.coverage')
@@ -200,68 +204,113 @@ def load_coverage_data(self, path):
     _, all_statements, not_run_statements, _ = coverage_data.analysis(path)
     return all_statements, not_run_statements
 
-  def test_coverage_simple(self):
+  def assert_expected_coverage(self, **kwargs):
     self.assertFalse(os.path.isfile(self.coverage_data_file()))
     covered_file = os.path.join(self.build_root, 'lib', 'core.py')
+
+    self.run_tests(targets=[self.green], **kwargs)
+    all_statements, not_run_statements = self.load_coverage_data(covered_file)
+    self.assertEqual([1, 2, 5, 6], all_statements)
+    self.assertEqual([6], not_run_statements)
+
+    self.run_failing_tests(targets=[self.red], failed_targets=[self.red], **kwargs)
+    all_statements, not_run_statements = self.load_coverage_data(covered_file)
+    self.assertEqual([1, 2, 5, 6], all_statements)
+    self.assertEqual([2], not_run_statements)
+
+    self.run_failing_tests(targets=[self.green, self.red], failed_targets=[self.red], **kwargs)
+    all_statements, not_run_statements = self.load_coverage_data(covered_file)
+    self.assertEqual([1, 2, 5, 6], all_statements)
+    self.assertEqual([], not_run_statements)
+
+    # The all target has no coverage attribute and the code under test does not follow the
+    # auto-discover pattern so we should get no coverage.
+    self.run_failing_tests(targets=[self.all], failed_targets=[self.all], **kwargs)
+    all_statements, not_run_statements = self.load_coverage_data(covered_file)
+    self.assertEqual([1, 2, 5, 6], all_statements)
+    self.assertEqual([1, 2, 5, 6], not_run_statements)
+
+    self.run_failing_tests(targets=[self.all_with_coverage],
+                           failed_targets=[self.all_with_coverage],
+                           **kwargs)
+    all_statements, not_run_statements = self.load_coverage_data(covered_file)
+    self.assertEqual([1, 2, 5, 6], all_statements)
+    self.assertEqual([], not_run_statements)
+
+  def test_coverage_simple_option(self):
+    # TODO(John Sirois): Consider eliminating support for "simple" coverage or at least formalizing
+    # the coverage option value that turns this on to "1" or "all" or "simple" = anything formal.
+    self.assert_expected_coverage(coverage='1')
+
+  def test_coverage_simple_env(self):
     with environment_as(PANTS_PY_COVERAGE='1'):
-      self.run_tests(targets=[self.green])
-      all_statements, not_run_statements = self.load_coverage_data(covered_file)
-      self.assertEqual([1, 2, 5, 6], all_statements)
-      self.assertEqual([6], not_run_statements)
-
-      self.run_failing_tests(targets=[self.red], failed_targets=[self.red])
-      all_statements, not_run_statements = self.load_coverage_data(covered_file)
-      self.assertEqual([1, 2, 5, 6], all_statements)
-      self.assertEqual([2], not_run_statements)
-
-      self.run_failing_tests(targets=[self.green, self.red], failed_targets=[self.red])
-      all_statements, not_run_statements = self.load_coverage_data(covered_file)
-      self.assertEqual([1, 2, 5, 6], all_statements)
-      self.assertEqual([], not_run_statements)
-
-      # The all target has no coverage attribute and the code under test does not follow the
-      # auto-discover pattern so we should get no coverage.
-      self.run_failing_tests(targets=[self.all], failed_targets=[self.all])
-      all_statements, not_run_statements = self.load_coverage_data(covered_file)
-      self.assertEqual([1, 2, 5, 6], all_statements)
-      self.assertEqual([1, 2, 5, 6], not_run_statements)
-
-      self.run_failing_tests(targets=[self.all_with_coverage], failed_targets=[self.all_with_coverage])
-      all_statements, not_run_statements = self.load_coverage_data(covered_file)
-      self.assertEqual([1, 2, 5, 6], all_statements)
-      self.assertEqual([], not_run_statements)
-
-  def test_coverage_modules(self):
+      self.assert_expected_coverage()
+
+  def assert_modules_dne(self, **kwargs):
     self.assertFalse(os.path.isfile(self.coverage_data_file()))
     covered_file = os.path.join(self.build_root, 'lib', 'core.py')
+
+    # modules: should trump .coverage
+    self.run_failing_tests(targets=[self.green, self.red], failed_targets=[self.red], **kwargs)
+    all_statements, not_run_statements = self.load_coverage_data(covered_file)
+    self.assertEqual([1, 2, 5, 6], all_statements)
+    self.assertEqual([1, 2, 5, 6], not_run_statements)
+
+  def test_coverage_modules_dne_option(self):
+    self.assert_modules_dne(coverage='modules:does_not_exist,nor_does_this')
+
+  def test_coverage_modules_dne_env(self):
     with environment_as(PANTS_PY_COVERAGE='modules:does_not_exist,nor_does_this'):
-      # modules: should trump .coverage
-      self.run_failing_tests(targets=[self.green, self.red], failed_targets=[self.red])
-      all_statements, not_run_statements = self.load_coverage_data(covered_file)
-      self.assertEqual([1, 2, 5, 6], all_statements)
-      self.assertEqual([1, 2, 5, 6], not_run_statements)
+      self.assert_modules_dne()
+
+  def assert_modules(self, **kwargs):
+    self.assertFalse(os.path.isfile(self.coverage_data_file()))
+    covered_file = os.path.join(self.build_root, 'lib', 'core.py')
+
+    self.run_failing_tests(targets=[self.all], failed_targets=[self.all], **kwargs)
+    all_statements, not_run_statements = self.load_coverage_data(covered_file)
+    self.assertEqual([1, 2, 5, 6], all_statements)
+    self.assertEqual([], not_run_statements)
+
+  def test_coverage_modules_option(self):
+    self.assert_modules(coverage='modules:core')
 
+  def test_coverage_modules_env(self):
     with environment_as(PANTS_PY_COVERAGE='modules:core'):
-      self.run_failing_tests(targets=[self.all], failed_targets=[self.all])
-      all_statements, not_run_statements = self.load_coverage_data(covered_file)
-      self.assertEqual([1, 2, 5, 6], all_statements)
-      self.assertEqual([], not_run_statements)
+      self.assert_modules()
 
-  def test_coverage_paths(self):
+  def assert_paths_dne(self, **kwargs):
     self.assertFalse(os.path.isfile(self.coverage_data_file()))
     covered_file = os.path.join(self.build_root, 'lib', 'core.py')
+
+    # paths: should trump .coverage
+    self.run_failing_tests(targets=[self.green, self.red], failed_targets=[self.red], **kwargs)
+    all_statements, not_run_statements = self.load_coverage_data(covered_file)
+    self.assertEqual([1, 2, 5, 6], all_statements)
+    self.assertEqual([1, 2, 5, 6], not_run_statements)
+
+  def test_coverage_paths_dne_option(self):
+    self.assert_paths_dne(coverage='paths:does_not_exist/,nor_does_this/')
+
+  def test_coverage_paths_dne_env(self):
     with environment_as(PANTS_PY_COVERAGE='paths:does_not_exist/,nor_does_this/'):
-      # paths: should trump .coverage
-      self.run_failing_tests(targets=[self.green, self.red], failed_targets=[self.red])
-      all_statements, not_run_statements = self.load_coverage_data(covered_file)
-      self.assertEqual([1, 2, 5, 6], all_statements)
-      self.assertEqual([1, 2, 5, 6], not_run_statements)
+      self.assert_paths_dne()
+
+  def assert_paths(self, **kwargs):
+    self.assertFalse(os.path.isfile(self.coverage_data_file()))
+    covered_file = os.path.join(self.build_root, 'lib', 'core.py')
+
+    self.run_failing_tests(targets=[self.all], failed_targets=[self.all], **kwargs)
+    all_statements, not_run_statements = self.load_coverage_data(covered_file)
+    self.assertEqual([1, 2, 5, 6], all_statements)
+    self.assertEqual([], not_run_statements)
+
+  def test_coverage_paths_option(self):
+    self.assert_paths(coverage='paths:core.py')
 
+  def test_coverage_paths_env(self):
     with environment_as(PANTS_PY_COVERAGE='paths:core.py'):
-      self.run_failing_tests(targets=[self.all], failed_targets=[self.all])
-      all_statements, not_run_statements = self.load_coverage_data(covered_file)
-      self.assertEqual([1, 2, 5, 6], all_statements)
-      self.assertEqual([], not_run_statements)
+      self.assert_paths()
 
   def test_sharding(self):
     self.run_failing_tests(targets=[self.red, self.green], failed_targets=[self.red], shard='0/2')