Python 3 - fixes to get green contrib (pantsbuild#6340)

All contrib folders should now be green on Python 2 and Python 3, excluding `googlejavaformat` and `python`. 

This fixes multiple issues causing Py3 failures + adds Py3 to Travis.

Author: Eric-Arellano

.travis.yml

@@ -175,15 +175,15 @@ matrix:
 
     - <<: *default_test_config
       env:
-        - SHARD="Python contrib tests - shard 1"
+        - SHARD="Py2 - Python contrib tests"
       script:
-        - ./build-support/bin/ci.sh -n -y 0/2 "${SHARD}"
+        - ./build-support/bin/ci.sh -n "${SHARD}"
 
     - <<: *default_test_config
       env:
-        - SHARD="Python contrib tests - shard 2"
+        - SHARD="Py3 - Python contrib tests"
       script:
-        - ./build-support/bin/ci.sh -n -y 1/2 "${SHARD}"
+        - ./build-support/bin/ci.sh -3n "${SHARD}"
 
     - <<: *default_test_config
       env:

contrib/go/src/python/pants/contrib/go/tasks/go_task.py

@@ -93,7 +93,7 @@ def go_stdlib(self):
     :rtype: frozenset of string
     """
     out = self._go_dist.create_go_cmd('list', args=['std']).check_output()
-    return frozenset(out.strip().split())
+    return frozenset(out.decode('utf-8').strip().split())
 
   # This simple regex mirrors the behavior of the relevant go code in practice (see
   # repoRootForImportDynamic and surrounding code in
@@ -154,7 +154,7 @@ def list_imports(self, pkg, gopath=None):
       if returncode != 0:
         raise self.ListDepsError('Problem listing imports for {}: {} failed with exit code {}'
                                  .format(pkg, go_cmd, returncode))
-      data = json.loads(out)
+      data = json.loads(out.decode('utf-8'))
 
       # XTestImports are for black box tests.  These test files live inside the package dir but
       # declare a different package and thus can only access the public members of the package's

contrib/go/tests/python/pants_test/contrib/go/subsystems/test_go_distribution.py

@@ -17,6 +17,14 @@
 
 class GoDistributionTest(unittest.TestCase):
 
+  @staticmethod
+  def _generate_go_command_regex(gopath, final_value):
+    goroot_env = r'GOROOT=[^ ]+'
+    gopath_env = r'GOPATH={}'.format(gopath)
+    # order of env values varies by interpreter and platform
+    env_values = r'({goroot_env} {gopath_env}|{gopath_env} {goroot_env})'.format(goroot_env=goroot_env, gopath_env=gopath_env)
+    return r'^{env_values} .*/go env {final_value}$'.format(env_values=env_values, final_value=final_value)
+
   def distribution(self):
     return global_subsystem_instance(GoDistribution)
 
@@ -46,10 +54,11 @@ def assert_no_gopath(self):
     self.assertEqual(go_env, go_cmd.env)
     self.assertEqual('go', os.path.basename(go_cmd.cmdline[0]))
     self.assertEqual(['env', 'GOPATH'], go_cmd.cmdline[1:])
-    self.assertRegexpMatches(str(go_cmd),
-                             r'^GOROOT=[^ ]+ GOPATH={} .*/go env GOPATH'.format(default_gopath))
     self.assertEqual(default_gopath, go_cmd.check_output().decode('utf-8').strip())
 
+    regex = GoDistributionTest._generate_go_command_regex(gopath=default_gopath, final_value='GOPATH')
+    self.assertRegexpMatches(str(go_cmd), regex)
+
   def test_go_command_no_gopath(self):
     self.assert_no_gopath()
 
@@ -68,4 +77,6 @@ def test_go_command_gopath(self):
                       'GOPATH': '/tmp/fred'}, go_cmd.env)
     self.assertEqual('go', os.path.basename(go_cmd.cmdline[0]))
     self.assertEqual(['env', 'GOROOT'], go_cmd.cmdline[1:])
-    self.assertRegexpMatches(str(go_cmd), r'^GOROOT=[^ ]+ GOPATH=/tmp/fred .*/go env GOROOT$')
+
+    regex = GoDistributionTest._generate_go_command_regex(gopath='/tmp/fred', final_value='GOROOT')
+    self.assertRegexpMatches(str(go_cmd), regex)

contrib/go/tests/python/pants_test/contrib/go/tasks/test_go_compile_integration.py

@@ -22,8 +22,8 @@ def test_go_compile_simple(self):
       pants_run = self.run_pants_with_workdir(args, workdir)
       self.assert_success(pants_run)
       go_dist = global_subsystem_instance(GoDistribution)
-      goos = go_dist.create_go_cmd('env', args=['GOOS']).check_output().strip()
-      goarch = go_dist.create_go_cmd('env', args=['GOARCH']).check_output().strip()
+      goos = go_dist.create_go_cmd('env', args=['GOOS']).check_output().decode('utf-8').strip()
+      goarch = go_dist.create_go_cmd('env', args=['GOARCH']).check_output().decode('utf-8').strip()
       expected_files = set('contrib.go.examples.src.go.{libname}.{libname}/'
                            'pkg/{goos}_{goarch}/{libname}.a'
                            .format(libname=libname, goos=goos, goarch=goarch)

contrib/go/tests/python/pants_test/contrib/go/tasks/test_go_fetch.py

@@ -41,7 +41,7 @@ def test_get_remote_import_paths(self):
     """)
     remote_import_ids = go_fetch._get_remote_import_paths('github.com/u/a',
                                                           gopath=self.build_root)
-    self.assertItemsEqual(remote_import_ids, ['bitbucket.org/u/b', 'github.com/u/c'])
+    self.assertEqual(sorted(remote_import_ids), sorted(['bitbucket.org/u/b', 'github.com/u/c']))
 
   def test_resolve_and_inject_explicit(self):
     r1 = self.make_target(spec='3rdparty/go/r1', target_type=GoRemoteLibrary)
@@ -219,4 +219,4 @@ def test_issues_2616(self):
     """)
     remote_import_ids = go_fetch._get_remote_import_paths('github.com/u/a',
                                                           gopath=self.build_root)
-    self.assertItemsEqual(remote_import_ids, ['bitbucket.org/u/b', 'github.com/u/c'])
+    self.assertEqual(sorted(remote_import_ids), sorted(['bitbucket.org/u/b', 'github.com/u/c']))

contrib/googlejavaformat/tests/python/pants_test/contrib/googlejavaformat/test_googlejavaformat.py

@@ -86,7 +86,7 @@ def test_lint_badformat(self):
     with self.assertRaises(TaskError) as error:
       self.execute(context)
     self.assertEqual(
-      error.exception.message,
+      str(error.exception),
       'google-java-format failed with exit code 1; to fix run: `./pants fmt <targets>`'
     )
 

contrib/node/src/python/pants/contrib/node/subsystems/command.py

@@ -63,7 +63,7 @@ def check_output(self, **kwargs):
     :raises: :class:`subprocess.CalledProcessError` if the command fails.
     """
     env, kwargs = self._prepare_env(kwargs)
-    return subprocess.check_output(self.cmd, env=env, **kwargs)
+    return subprocess.check_output(self.cmd, env=env, **kwargs).decode('utf-8')
 
   def __str__(self):
     return ' '.join(self.cmd)

contrib/node/tests/python/pants_test/contrib/node/subsystems/test_node_distribution.py

@@ -21,7 +21,7 @@ def setUp(self):
 
   def test_bootstrap(self):
     node_cmd = self.distribution.node_command(args=['--version'])
-    output = node_cmd.check_output().decode('utf-8').strip()
+    output = node_cmd.check_output().strip()
     self.assertEqual(self.distribution.version(), output)
 
   def test_node(self):
@@ -42,7 +42,7 @@ def test_node(self):
   def test_npm(self):
     npm_version_flag = self.distribution.get_package_manager('npm').run_command(
       args=['--version'])
-    raw_version = npm_version_flag.check_output().decode('utf-8').strip()
+    raw_version = npm_version_flag.check_output().strip()
 
     npm_version_cmd = self.distribution.get_package_manager('npm').run_command(
       args=['version', '--json'])
@@ -54,7 +54,7 @@ def test_npm(self):
   def test_yarnpkg(self):
     yarnpkg_version_command = self.distribution.get_package_manager('yarn').run_command(
       args=['--version'])
-    yarnpkg_version = yarnpkg_version_command.check_output().decode('utf-8').strip()
+    yarnpkg_version = yarnpkg_version_command.check_output().strip()
     yarnpkg_versions_command = self.distribution.get_package_manager('yarn').run_command(
       args=['versions', '--json'])
     yarnpkg_versions = json.loads(yarnpkg_versions_command.check_output())
@@ -67,7 +67,7 @@ def test_node_command_path_injection(self):
 
     # Test the case in which we do not pass in env,
     # which should fall back to env=os.environ.copy()
-    injected_paths = node_path_cmd.check_output().decode('utf-8').strip().split(os.pathsep)
+    injected_paths = node_path_cmd.check_output().strip().split(os.pathsep)
     self.assertEqual(node_bin_path, injected_paths[0])
 
   def test_node_command_path_injection_with_overrided_path(self):
@@ -76,7 +76,7 @@ def test_node_command_path_injection_with_overrided_path(self):
     node_bin_path = self.distribution._install_node()
     injected_paths = node_path_cmd.check_output(
       env={'PATH': '/test/path'}
-    ).decode('utf-8').strip().split(os.pathsep)
+    ).strip().split(os.pathsep)
     self.assertEqual(node_bin_path, injected_paths[0])
     self.assertListEqual([node_bin_path, '/test/path'], injected_paths)
 
@@ -86,5 +86,5 @@ def test_node_command_path_injection_with_empty_path(self):
     node_bin_path = self.distribution._install_node()
     injected_paths = node_path_cmd.check_output(
       env={'PATH': ''}
-    ).decode('utf-8').strip().split(os.pathsep)
+    ).strip().split(os.pathsep)
     self.assertListEqual([node_bin_path, ''], injected_paths)

contrib/python/src/python/pants/contrib/python/checks/tasks/python_eval.py

@@ -34,8 +34,8 @@ class Error(TaskError):
     """A richer failure exception type useful for tests."""
 
     def __init__(self, *args, **kwargs):
-      compiled = kwargs.pop(b'compiled')
-      failed = kwargs.pop(b'failed')
+      compiled = kwargs.pop('compiled')
+      failed = kwargs.pop('failed')
       super(PythonEval.Error, self).__init__(*args, **kwargs)
       self.compiled = compiled
       self.failed = failed
@@ -139,9 +139,9 @@ def _compile_target(self, vt):
       executable_file_content = self._get_executable_file_content(exec_pex_parent, modules)
 
       hasher = hashlib.sha1()
-      hasher.update(reqs_pex.path())
-      hasher.update(srcs_pex.path())
-      hasher.update(executable_file_content)
+      hasher.update(reqs_pex.path().encode('utf-8'))
+      hasher.update(srcs_pex.path().encode('utf-8'))
+      hasher.update(executable_file_content.encode('utf-8'))
       exec_file_hash = hasher.hexdigest()
       exec_pex_path = os.path.realpath(os.path.join(exec_pex_parent, exec_file_hash))
       if not os.path.isdir(exec_pex_path):
@@ -215,7 +215,7 @@ def _resolve_requirements_for_versioned_target_closure(self, interpreter, vt):
     reqs_pex_path = os.path.realpath(os.path.join(self.workdir, str(interpreter.identity),
                                                   vt.cache_key.hash))
     if not os.path.isdir(reqs_pex_path):
-      req_libs =  [t for t in vt.target.closure() if has_python_requirements(t)]
+      req_libs = [t for t in vt.target.closure() if has_python_requirements(t)]
       with safe_concurrent_creation(reqs_pex_path) as safe_path:
         builder = PEXBuilder(safe_path, interpreter=interpreter, copy=True)
         dump_requirement_libs(builder, interpreter, req_libs, self.context.log)

contrib/python/tests/python/pants_test/contrib/python/checks/tasks/checkstyle/test_checker.py

@@ -12,8 +12,8 @@
 from pants_test.backend.python.tasks.python_task_test_base import PythonTaskTestBase
 
 from pants.contrib.python.checks.tasks.checkstyle.checker import PythonCheckStyleTask
-from pants.contrib.python.checks.tasks.checkstyle.print_statements_subsystem import \
-  PrintStatementsSubsystem
+from pants.contrib.python.checks.tasks.checkstyle.variable_names_subsystem import \
+  VariableNamesSubsystem
 
 
 class PythonCheckStyleTaskTest(PythonTaskTestBase):
@@ -24,7 +24,7 @@ def task_type(cls):
   def setUp(self):
     super(PythonCheckStyleTaskTest, self).setUp()
     PythonCheckStyleTask.clear_plugins()
-    PythonCheckStyleTask.register_plugin('print-statements', PrintStatementsSubsystem)
+    PythonCheckStyleTask.register_plugin('variable-names', VariableNamesSubsystem)
 
   def tearDown(self):
     super(PythonCheckStyleTaskTest, self).tearDown()
@@ -36,7 +36,8 @@ def test_no_sources(self):
 
   def test_pass(self):
     self.create_file('a/python/pass.py', contents=dedent("""
-                       print('Print is a function')
+                       class UpperCase: 
+                         pass
                      """))
     target = self.make_target('a/python:pass', PythonLibrary, sources=['pass.py'])
     context = self.context(target_roots=[target])
@@ -45,7 +46,8 @@ def test_pass(self):
 
   def test_failure(self):
     self.create_file('a/python/fail.py', contents=dedent("""
-                         print 'Print should not be used as a statement'
+                        class lower_case:
+                          pass
                        """))
     target = self.make_target('a/python:fail', PythonLibrary, sources=['fail.py'])
     context = self.context(target_roots=[target])
@@ -56,20 +58,21 @@ def test_failure(self):
 
   def test_suppressed_file_passes(self):
     self.create_file('a/python/fail.py', contents=dedent("""
-                         print 'Print should not be used as a statement'
+                        class lower_case:
+                          pass
                        """))
     suppression_file = self.create_file('suppress.txt', contents=dedent("""
-    a/python/fail\.py::print-statements"""))
+    a/python/fail\.py::variable-names"""))
     target = self.make_target('a/python:fail', PythonLibrary, sources=['fail.py'])
     self.set_options(suppress=suppression_file)
     context = self.context(target_roots=[target], )
     task = self.create_task(context)
-
     self.assertEqual(0, task.execute())
 
   def test_failure_fail_false(self):
     self.create_file('a/python/fail.py', contents=dedent("""
-                       print 'Print should not be used as a statement'
+                        class lower_case:
+                          pass
                      """))
     target = self.make_target('a/python:fail', PythonLibrary, sources=['fail.py'])
     self.set_options(fail=False)
@@ -90,7 +93,8 @@ def test_syntax_error(self):
 
   def test_failure_print_nit(self):
     self.create_file('a/python/fail.py', contents=dedent("""
-                         print 'Print should not be used as a statement'
+                        class lower_case:
+                          pass
                        """))
     target = self.make_target('a/python:fail', PythonLibrary, sources=['fail.py'])
     context = self.context(target_roots=[target])
@@ -100,8 +104,8 @@ def test_failure_print_nit(self):
 
     self.assertEqual(1, len(nits))
     self.assertEqual(
-      """T607:ERROR   a/python/fail.py:002 Print used as a statement.\n"""
-      """     |print 'Print should not be used as a statement'""",
+      """T000:ERROR   a/python/fail.py:002 Classes must be UpperCamelCased\n"""
+      """     |class lower_case:""",
       str(nits[0]))
 
   def test_syntax_error_nit(self):
@@ -121,21 +125,3 @@ def test_syntax_error_nit(self):
                      """     |invalid python\n"""
                      """     |""",
                      str(nits[0]))
-
-  def test_multiline_nit_printed_only_once(self):
-    self.create_file('a/python/fail.py', contents=dedent("""
-                         print ('Multi'
-                                'line') + 'expression'
-                       """))
-    target = self.make_target('a/python:fail', PythonLibrary, sources=['fail.py'])
-    context = self.context(target_roots=[target])
-    task = self.create_task(context)
-
-    nits = list(task.get_nits('a/python/fail.py'))
-
-    self.assertEqual(1, len(nits))
-    self.assertEqual(
-      """T607:ERROR   a/python/fail.py:002-003 Print used as a statement.\n"""
-      """     |print ('Multi'\n"""
-      """     |       'line') + 'expression'""",
-      str(nits[0]))

contrib/python/tests/python/pants_test/contrib/python/checks/tasks/test_python_eval.py

@@ -6,13 +6,17 @@
 
 from textwrap import dedent
 
+import pytest
+from future.utils import PY3
 from pants.backend.python.subsystems.python_repos import PythonRepos
 from pants.backend.python.subsystems.python_setup import PythonSetup
 from pants_test.backend.python.tasks.python_task_test_base import PythonTaskTestBase
 
 from pants.contrib.python.checks.tasks.python_eval import PythonEval
 
 
+# TODO(python3port): https://github.com/pantsbuild/pants/issues/6354. Fix before switching fully to Py3.
+@pytest.mark.skipif(PY3, reason='PEX issue when using Python 3. https://github.com/pantsbuild/pants/issues/6354')
 class PythonEvalTest(PythonTaskTestBase):
 
   @classmethod

contrib/scrooge/src/python/pants/contrib/scrooge/tasks/java_thrift_library_fingerprint_strategy.py

@@ -29,8 +29,8 @@ def compute_fingerprint(self, target):
       return fp
 
     hasher = hashlib.sha1()
-    hasher.update(fp)
-    hasher.update(self._thrift_defaults.language(target))
+    hasher.update(fp.encode('utf-8'))
+    hasher.update(self._thrift_defaults.language(target).encode('utf-8'))
     hasher.update(str(self._thrift_defaults.compiler_args(target)).encode('utf-8'))
 
     namespace_map = self._thrift_defaults.namespace_map(target)

contrib/scrooge/tests/python/pants_test/contrib/scrooge/tasks/test_thrift_linter.py

@@ -40,15 +40,15 @@ def get_default_jvm_options():
     thrift_target = self.create_library('a', 'java_thrift_library', 'a', ['A.thrift'])
     task = self.create_task(self.context(target_roots=thrift_target))
     self._prepare_mocks(task)
-    expected_include_paths = {'src/thrift/tweet', 'src/thrift/users'}
-    expected_paths = {'src/thrift/tweet/a.thrift', 'src/thrift/tweet/b.thrift'}
+    expected_include_paths = ['src/thrift/users', 'src/thrift/tweet']
+    expected_paths = ['src/thrift/tweet/a.thrift', 'src/thrift/tweet/b.thrift']
     mock_calculate_compile_sources.return_value = (expected_include_paths, expected_paths)
     task._lint(thrift_target, task.tool_classpath('scrooge-linter'))
 
     self._run_java_mock.assert_called_once_with(
       classpath='foo_classpath',
       main='com.twitter.scrooge.linter.Main',
       args=['--ignore-errors', '--include-path', 'src/thrift/users', '--include-path',
-            'src/thrift/tweet', 'src/thrift/tweet/b.thrift', 'src/thrift/tweet/a.thrift'],
+            'src/thrift/tweet', 'src/thrift/tweet/a.thrift', 'src/thrift/tweet/b.thrift'],
       jvm_options=get_default_jvm_options(),
       workunit_labels=[WorkUnitLabel.COMPILER, WorkUnitLabel.SUPPRESS_LABEL])

pants-plugins/tests/python/internal_backend_test/sitegen/test_sitegen.py

@@ -95,10 +95,10 @@ class AllTheThingsTestCase(unittest.TestCase):
   def setUp(self):
     self.config = json.loads(CONFIG_JSON)
     self.soups = {
-      'index': bs4.BeautifulSoup(INDEX_HTML),
-      'subdir/page1': bs4.BeautifulSoup(P1_HTML),
-      'subdir/page2': bs4.BeautifulSoup(P2_HTML),
-      'subdir/page2_no_toc': bs4.BeautifulSoup(P2_HTML),
+      'index': bs4.BeautifulSoup(INDEX_HTML, 'html.parser'),
+      'subdir/page1': bs4.BeautifulSoup(P1_HTML, 'html.parser'),
+      'subdir/page2': bs4.BeautifulSoup(P2_HTML, 'html.parser'),
+      'subdir/page2_no_toc': bs4.BeautifulSoup(P2_HTML, 'html.parser'),
     }
     self.precomputed = sitegen.precompute(self.config, self.soups)
 

src/python/pants/backend/jvm/ivy_utils.py

@@ -1017,7 +1017,7 @@ def generate_fetch_ivy(cls, jars, ivyxml, confs, resolve_hash_name):
 
   @classmethod
   def _write_ivy_xml_file(cls, ivyxml, template_data, template_relpath):
-    template_text = pkgutil.get_data(__name__, template_relpath)
+    template_text = pkgutil.get_data(__name__, template_relpath).decode('utf-8')
     generator = Generator(template_text, lib=template_data)
     with safe_open(ivyxml, 'w') as output:
       generator.write(output)