From 5b33c5729cff98f9be164e286305318499010fb3 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 10 Jun 2020 12:54:37 -0700 Subject: [PATCH 1/3] Remove pyopenssl This was only being used to pin a transitive dep. We don't need to do this anymore because `requests` is more up-to-date. # Delete this line to force CI to run the JVM tests. [ci skip-jvm-tests] --- 3rdparty/python/requirements.txt | 1 - contrib/go/src/python/pants/contrib/go/subsystems/BUILD | 1 - src/python/pants/goal/BUILD | 1 - src/python/pants/net/BUILD | 1 - tests/python/pants_test/net/http/BUILD | 1 - 5 files changed, 5 deletions(-) diff --git a/3rdparty/python/requirements.txt b/3rdparty/python/requirements.txt index 413fe547f47..b60d5a8ca03 100644 --- a/3rdparty/python/requirements.txt +++ b/3rdparty/python/requirements.txt @@ -25,7 +25,6 @@ pathspec==0.8.0 pex==2.1.11 psutil==5.7.0 Pygments==2.6.1 -pyopenssl==19.1.0 pystache==0.5.4 python-Levenshtein==0.12.0 pywatchman==1.4.1 diff --git a/contrib/go/src/python/pants/contrib/go/subsystems/BUILD b/contrib/go/src/python/pants/contrib/go/subsystems/BUILD index 2c4eaf9a169..78043789bd8 100644 --- a/contrib/go/src/python/pants/contrib/go/subsystems/BUILD +++ b/contrib/go/src/python/pants/contrib/go/subsystems/BUILD @@ -4,7 +4,6 @@ python_library( dependencies=[ '3rdparty/python:requests', - '3rdparty/python:pyopenssl', 'contrib/go/src/python/pants/contrib/go/targets', 'src/python/pants/base:workunit', 'src/python/pants/binaries', diff --git a/src/python/pants/goal/BUILD b/src/python/pants/goal/BUILD index 205b4b8da1b..264f31a4c69 100644 --- a/src/python/pants/goal/BUILD +++ b/src/python/pants/goal/BUILD @@ -95,7 +95,6 @@ python_library( ':artifact_cache_stats', ':pantsd_stats', '3rdparty/python:requests', - '3rdparty/python:pyopenssl', 'src/python/pants/auth', 'src/python/pants/base:build_environment', 'src/python/pants/base:run_info', diff --git a/src/python/pants/net/BUILD b/src/python/pants/net/BUILD index 88b13f6570e..d6c1a42e4ed 100644 --- a/src/python/pants/net/BUILD +++ b/src/python/pants/net/BUILD @@ -5,7 +5,6 @@ python_library( sources = ['**/*.py'], dependencies = [ '3rdparty/python:requests', - '3rdparty/python:pyopenssl', 'src/python/pants/util:dirutil', ], tags = {"partially_type_checked"}, diff --git a/tests/python/pants_test/net/http/BUILD b/tests/python/pants_test/net/http/BUILD index 737ecb35787..b30c0b46871 100644 --- a/tests/python/pants_test/net/http/BUILD +++ b/tests/python/pants_test/net/http/BUILD @@ -4,7 +4,6 @@ python_tests( dependencies = [ '3rdparty/python:requests', - '3rdparty/python:pyopenssl', 'src/python/pants/net', 'src/python/pants/util:contextutil', 'src/python/pants/util:dirutil', From da1eb42d53f6af1e3f4cb17aa3356d461ba36725 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 10 Jun 2020 14:20:44 -0700 Subject: [PATCH 2/3] Remove `parametrized` It's better to use Pytest parametrization. Note that this only works with top-level modules, but that's okay because we want to encourage this style for all new tests. Tests that still need to use class-based tests can parametrize through other means like inner helper functions. # Delete this line to force CI to run the JVM tests. [ci skip-jvm-tests] --- 3rdparty/python/requirements.txt | 1 - tests/python/pants_test/init/BUILD | 1 - .../pants_test/init/test_plugin_resolver.py | 312 +++++++++--------- tests/python/pants_test/reporting/BUILD | 3 +- .../reporting/test_reporting_integration.py | 28 +- 5 files changed, 173 insertions(+), 172 deletions(-) diff --git a/3rdparty/python/requirements.txt b/3rdparty/python/requirements.txt index b60d5a8ca03..9857cdd45d6 100644 --- a/3rdparty/python/requirements.txt +++ b/3rdparty/python/requirements.txt @@ -20,7 +20,6 @@ mypy==0.780 Markdown==2.1.1 packaging==20.3 -parameterized==0.6.1 pathspec==0.8.0 pex==2.1.11 psutil==5.7.0 diff --git a/tests/python/pants_test/init/BUILD b/tests/python/pants_test/init/BUILD index 9f2b97370e2..1a1320dce93 100644 --- a/tests/python/pants_test/init/BUILD +++ b/tests/python/pants_test/init/BUILD @@ -5,7 +5,6 @@ python_tests( dependencies = [ '3rdparty/python:dataclasses', - '3rdparty/python:parameterized', '3rdparty/python:pex', '3rdparty/python:setuptools', 'src/python/pants/base:exceptions', diff --git a/tests/python/pants_test/init/test_plugin_resolver.py b/tests/python/pants_test/init/test_plugin_resolver.py index f4550bf7eb7..67e486dd4ff 100644 --- a/tests/python/pants_test/init/test_plugin_resolver.py +++ b/tests/python/pants_test/init/test_plugin_resolver.py @@ -3,13 +3,12 @@ import os import shutil -import unittest from abc import ABCMeta, abstractmethod from contextlib import contextmanager from pathlib import Path from textwrap import dedent -from parameterized import parameterized +import pytest from pex.interpreter import PythonInterpreter from pex.resolver import Unsatisfiable from pkg_resources import Requirement, WorkingSet @@ -57,164 +56,167 @@ def _create_dist(self, runner: SetupPyRunner): INSTALLERS = [("sdist", SdistInstaller), ("whl", WheelInstaller)] +DEFAULT_VERSION = "0.0.0" -class PluginResolverTest(unittest.TestCase): - DEFAULT_VERSION = "0.0.0" +def create_plugin(distribution_repo_dir, plugin, version=None, packager_cls=None): + distribution_repo_dir = Path(distribution_repo_dir) + source_dir = distribution_repo_dir.joinpath(plugin) + source_dir.mkdir(parents=True) + source_dir.joinpath("setup.py").write_text( + dedent( + f""" + from setuptools import setup - @classmethod - def create_plugin(cls, distribution_repo_dir, plugin, version=None, packager_cls=None): - distribution_repo_dir = Path(distribution_repo_dir) - source_dir = distribution_repo_dir.joinpath(plugin) - source_dir.mkdir(parents=True) - source_dir.joinpath("setup.py").write_text( - dedent( - f""" - from setuptools import setup - - - setup(name="{plugin}", version="{version or cls.DEFAULT_VERSION}") - """ - ) + setup(name="{plugin}", version="{version or DEFAULT_VERSION}") + """ ) - packager_cls = packager_cls or SdistInstaller - packager = packager_cls(source_dir=source_dir, install_dir=distribution_repo_dir) - packager.run() + ) + packager_cls = packager_cls or SdistInstaller + packager = packager_cls(source_dir=source_dir, install_dir=distribution_repo_dir) + packager.run() + +@contextmanager +def plugin_resolution(*, interpreter=None, chroot=None, plugins=None, packager_cls=None): @contextmanager - def plugin_resolution(self, *, interpreter=None, chroot=None, plugins=None, packager_cls=None): - @contextmanager - def provide_chroot(existing): - if existing: - yield existing, False - else: - with temporary_dir() as new_chroot: - yield new_chroot, True - - with provide_chroot(chroot) as (root_dir, create_artifacts): - env = {} - repo_dir = None - if plugins: - repo_dir = os.path.join(root_dir, "repo") - env.update( - PANTS_PYTHON_REPOS_REPOS=f"[{repo_dir!r}]", - PANTS_PYTHON_REPOS_INDEXES="[]", - PANTS_PYTHON_SETUP_RESOLVER_CACHE_TTL="1", + def provide_chroot(existing): + if existing: + yield existing, False + else: + with temporary_dir() as new_chroot: + yield new_chroot, True + + with provide_chroot(chroot) as (root_dir, create_artifacts): + env = {} + repo_dir = None + if plugins: + repo_dir = os.path.join(root_dir, "repo") + env.update( + PANTS_PYTHON_REPOS_REPOS=f"[{repo_dir!r}]", + PANTS_PYTHON_REPOS_INDEXES="[]", + PANTS_PYTHON_SETUP_RESOLVER_CACHE_TTL="1", + ) + plugin_list = [] + for plugin in plugins: + version = None + if isinstance(plugin, tuple): + plugin, version = plugin + plugin_list.append(f"{plugin}=={version}" if version else plugin) + if create_artifacts: + create_plugin(repo_dir, plugin, version, packager_cls=packager_cls) + env["PANTS_PLUGINS"] = f"[{','.join(map(repr, plugin_list))}]" + env["PANTS_PLUGIN_CACHE_DIR"] = os.path.join(root_dir, "plugin-cache") + + configpath = os.path.join(root_dir, "pants.toml") + if create_artifacts: + touch(configpath) + args = [f"--pants-config-files=['{configpath}']"] + + options_bootstrapper = OptionsBootstrapper.create(env=env, args=args) + plugin_resolver = PluginResolver(options_bootstrapper, interpreter=interpreter) + cache_dir = plugin_resolver.plugin_cache_dir + + working_set = plugin_resolver.resolve(WorkingSet(entries=[])) + for dist in working_set: + assert Path(cache_dir) in Path(dist.location).parents + + yield working_set, root_dir, repo_dir, cache_dir + + +def test_no_plugins(): + with plugin_resolution() as (working_set, _, _, _): + assert [] == list(working_set) + + +@pytest.mark.parametrize("unused_test_name,packager_cls", INSTALLERS) +def test_plugins(unused_test_name, packager_cls): + with plugin_resolution(plugins=[("jake", "1.2.3"), "jane"], packager_cls=packager_cls) as ( + working_set, + _, + _, + cache_dir, + ): + + def assert_dist_version(name, expected_version): + dist = working_set.find(req(name)) + assert expected_version == dist.version + + assert 2 == len(working_set.entries) + + assert_dist_version(name="jake", expected_version="1.2.3") + assert_dist_version(name="jane", expected_version=DEFAULT_VERSION) + + +@pytest.mark.parametrize("unused_test_name,packager_cls", INSTALLERS) +def test_exact_requirements(unused_test_name, packager_cls): + with plugin_resolution( + plugins=[("jake", "1.2.3"), ("jane", "3.4.5")], packager_cls=packager_cls + ) as results: + working_set, chroot, repo_dir, cache_dir = results + + # Kill the repo source dir and re-resolve. If the PluginResolver truly detects exact + # requirements it should skip any resolves and load directly from the still in-tact cache. + safe_rmtree(repo_dir) + + with plugin_resolution( + chroot=chroot, plugins=[("jake", "1.2.3"), ("jane", "3.4.5")] + ) as results2: + + working_set2, _, _, _ = results2 + + assert list(working_set) == list(working_set2) + + +@pytest.mark.parametrize("unused_test_name,packager_cls", INSTALLERS) +@skip_unless_python36_and_python37_present +def test_exact_requirements_interpreter_change(unused_test_name, packager_cls): + python36 = PythonInterpreter.from_binary(python_interpreter_path(PY_36)) + python37 = PythonInterpreter.from_binary(python_interpreter_path(PY_37)) + + with plugin_resolution( + interpreter=python36, + plugins=[("jake", "1.2.3"), ("jane", "3.4.5")], + packager_cls=packager_cls, + ) as results: + + working_set, chroot, repo_dir, cache_dir = results + + safe_rmtree(repo_dir) + with pytest.raises(Unsatisfiable): + with plugin_resolution( + interpreter=python37, chroot=chroot, plugins=[("jake", "1.2.3"), ("jane", "3.4.5")], + ): + pytest.fail( + "Plugin re-resolution is expected for an incompatible interpreter and it is " + "expected to fail since we removed the dist `repo_dir` above." ) - plugin_list = [] - for plugin in plugins: - version = None - if isinstance(plugin, tuple): - plugin, version = plugin - plugin_list.append(f"{plugin}=={version}" if version else plugin) - if create_artifacts: - self.create_plugin(repo_dir, plugin, version, packager_cls=packager_cls) - env["PANTS_PLUGINS"] = f"[{','.join(map(repr, plugin_list))}]" - env["PANTS_PLUGIN_CACHE_DIR"] = os.path.join(root_dir, "plugin-cache") - - configpath = os.path.join(root_dir, "pants.toml") - if create_artifacts: - touch(configpath) - args = [f"--pants-config-files=['{configpath}']"] - - options_bootstrapper = OptionsBootstrapper.create(env=env, args=args) - plugin_resolver = PluginResolver(options_bootstrapper, interpreter=interpreter) - cache_dir = plugin_resolver.plugin_cache_dir - - working_set = plugin_resolver.resolve(WorkingSet(entries=[])) - for dist in working_set: - self.assertIn(Path(cache_dir), Path(dist.location).parents) - - yield working_set, root_dir, repo_dir, cache_dir - - def test_no_plugins(self): - with self.plugin_resolution() as (working_set, _, _, _): - self.assertEqual([], list(working_set)) - - @parameterized.expand(INSTALLERS) - def test_plugins(self, unused_test_name, packager_cls): - with self.plugin_resolution( - plugins=[("jake", "1.2.3"), "jane"], packager_cls=packager_cls - ) as (working_set, _, _, cache_dir): - - def assert_dist_version(name, expected_version): - dist = working_set.find(req(name)) - self.assertEqual(expected_version, dist.version) - - self.assertEqual(2, len(working_set.entries)) - - assert_dist_version(name="jake", expected_version="1.2.3") - assert_dist_version(name="jane", expected_version=self.DEFAULT_VERSION) - - @parameterized.expand(INSTALLERS) - def test_exact_requirements(self, unused_test_name, packager_cls): - with self.plugin_resolution( - plugins=[("jake", "1.2.3"), ("jane", "3.4.5")], packager_cls=packager_cls - ) as results: - working_set, chroot, repo_dir, cache_dir = results - - # Kill the repo source dir and re-resolve. If the PluginResolver truly detects exact - # requirements it should skip any resolves and load directly from the still in-tact cache. - safe_rmtree(repo_dir) - - with self.plugin_resolution( - chroot=chroot, plugins=[("jake", "1.2.3"), ("jane", "3.4.5")] - ) as results2: - - working_set2, _, _, _ = results2 - - self.assertEqual(list(working_set), list(working_set2)) - - @parameterized.expand(INSTALLERS) - @skip_unless_python36_and_python37_present - def test_exact_requirements_interpreter_change(self, unused_test_name, packager_cls): - python36 = PythonInterpreter.from_binary(python_interpreter_path(PY_36)) - python37 = PythonInterpreter.from_binary(python_interpreter_path(PY_37)) - - with self.plugin_resolution( - interpreter=python36, - plugins=[("jake", "1.2.3"), ("jane", "3.4.5")], - packager_cls=packager_cls, - ) as results: - - working_set, chroot, repo_dir, cache_dir = results - - safe_rmtree(repo_dir) - with self.assertRaises(Unsatisfiable): - with self.plugin_resolution( - interpreter=python37, - chroot=chroot, - plugins=[("jake", "1.2.3"), ("jane", "3.4.5")], - ): - self.fail( - "Plugin re-resolution is expected for an incompatible interpreter and it is " - "expected to fail since we removed the dist `repo_dir` above." - ) - - # But for a compatible interpreter the exact resolve results should be re-used and load - # directly from the still in-tact cache. - with self.plugin_resolution( - interpreter=python36, chroot=chroot, plugins=[("jake", "1.2.3"), ("jane", "3.4.5")] - ) as results2: - - working_set2, _, _, _ = results2 - self.assertEqual(list(working_set), list(working_set2)) - - @parameterized.expand(INSTALLERS) - def test_inexact_requirements(self, unused_test_name, packager_cls): - with self.plugin_resolution( - plugins=[("jake", "1.2.3"), "jane"], packager_cls=packager_cls - ) as results: - - working_set, chroot, repo_dir, cache_dir = results - - # Kill the cache and the repo source dir and wait past our 1s test TTL, if the PluginResolver - # truly detects inexact plugin requirements it should skip perma-caching and fall through to - # a pex resolve and then fail. - safe_rmtree(repo_dir) - safe_rmtree(cache_dir) - - with self.assertRaises(Unsatisfiable): - with self.plugin_resolution(chroot=chroot, plugins=[("jake", "1.2.3"), "jane"]): - self.fail("Should not reach here, should raise first.") + + # But for a compatible interpreter the exact resolve results should be re-used and load + # directly from the still in-tact cache. + with plugin_resolution( + interpreter=python36, chroot=chroot, plugins=[("jake", "1.2.3"), ("jane", "3.4.5")] + ) as results2: + + working_set2, _, _, _ = results2 + assert list(working_set) == list(working_set2) + + +@pytest.mark.parametrize("unused_test_name,packager_cls", INSTALLERS) +def test_inexact_requirements(unused_test_name, packager_cls): + with plugin_resolution( + plugins=[("jake", "1.2.3"), "jane"], packager_cls=packager_cls + ) as results: + + working_set, chroot, repo_dir, cache_dir = results + + # Kill the cache and the repo source dir and wait past our 1s test TTL, if the PluginResolver + # truly detects inexact plugin requirements it should skip perma-caching and fall through to + # a pex resolve and then fail. + safe_rmtree(repo_dir) + safe_rmtree(cache_dir) + + with pytest.raises(Unsatisfiable): + with plugin_resolution(chroot=chroot, plugins=[("jake", "1.2.3"), "jane"]): + pytest.fail("Should not reach here, should raise first.") diff --git a/tests/python/pants_test/reporting/BUILD b/tests/python/pants_test/reporting/BUILD index b5e9ae28389..753bea581ff 100644 --- a/tests/python/pants_test/reporting/BUILD +++ b/tests/python/pants_test/reporting/BUILD @@ -15,8 +15,7 @@ python_tests( name = 'reporting_integration', sources = ['test_reporting_integration.py'], dependencies = [ - '3rdparty/python:parameterized', - '3rdparty/python:py-zipkin', + '3rdparty/python:psutil', 'src/python/pants/util:contextutil', 'src/python/pants/testutil:int-test', 'examples/src/java/org/pantsbuild/example:hello_directory', diff --git a/tests/python/pants_test/reporting/test_reporting_integration.py b/tests/python/pants_test/reporting/test_reporting_integration.py index 004e2039d30..591200edf7d 100644 --- a/tests/python/pants_test/reporting/test_reporting_integration.py +++ b/tests/python/pants_test/reporting/test_reporting_integration.py @@ -9,7 +9,6 @@ from pathlib import Path import psutil -from parameterized import parameterized from pants.testutil.pants_run_integration_test import PantsRunIntegrationTest from pants.util.collections import assert_single_element @@ -194,18 +193,21 @@ def test_invalid_config(self): ) self.assertIn("", pants_run.stdout_data) - @parameterized.expand(["--quiet", "--no-quiet"]) - def test_epilog_to_stderr(self, quiet_flag): - command = [ - "--time", - quiet_flag, - "bootstrap", - "examples/src/java/org/pantsbuild/example/hello::", - ] - pants_run = self.run_pants(command) - self.assert_success(pants_run) - self.assertIn("Cumulative Timings", pants_run.stderr_data) - self.assertNotIn("Cumulative Timings", pants_run.stdout_data) + def test_epilog_to_stderr(self) -> None: + def run_test(quiet_flag: str) -> None: + command = [ + "--time", + quiet_flag, + "bootstrap", + "examples/src/java/org/pantsbuild/example/hello::", + ] + pants_run = self.run_pants(command) + self.assert_success(pants_run) + self.assertIn("Cumulative Timings", pants_run.stderr_data) + self.assertNotIn("Cumulative Timings", pants_run.stdout_data) + + run_test("--quiet") + run_test("--no-quiet") def test_zipkin_reporter(self): ZipkinHandler = zipkin_handler() From 34f40e528070694bc57ecbc419ea2f9a1521c321 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 10 Jun 2020 14:28:58 -0700 Subject: [PATCH 3/3] Remove 'wheel' We don't use it for anything # Delete this line to force CI to run the JVM tests. [ci skip-jvm-tests] --- 3rdparty/python/requirements.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/3rdparty/python/requirements.txt b/3rdparty/python/requirements.txt index 9857cdd45d6..c9c33e2b337 100644 --- a/3rdparty/python/requirements.txt +++ b/3rdparty/python/requirements.txt @@ -36,5 +36,4 @@ setuptools==44.0.0 toml==0.10.1 typed-ast>=1.4.1,<1.5 typing-extensions==3.7.4.2 -wheel==0.34.2 www-authenticate==0.9.2