From 84e0b11e02a038eee5e8730522a7093452d1d13e Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Wed, 22 Jul 2020 12:17:00 -0700 Subject: [PATCH] Get rid of remaining uses of init_subsystem. [ci skip-rust-tests] --- .../pants/backend/python/rules/pex_test.py | 44 +++++------ .../backend/python/rules/run_setup_py.py | 5 +- .../backend/python/rules/run_setup_py_test.py | 14 +--- src/python/pants/backend/python/rules/util.py | 12 +-- .../pants/backend/python/rules/util_test.py | 58 ++++---------- .../pants/backend/python/target_types_test.py | 25 +++--- src/python/pants/python/python_setup.py | 9 +++ src/python/pants/subsystem/subsystem.py | 5 +- src/python/pants/testutil/BUILD | 1 - src/python/pants/testutil/subsystem/BUILD | 4 - .../pants/testutil/subsystem/__init__.py | 0 src/python/pants/testutil/subsystem/util.py | 77 ------------------- .../pants_test/process/test_subprocess.py | 14 ---- 13 files changed, 68 insertions(+), 200 deletions(-) delete mode 100644 src/python/pants/testutil/subsystem/BUILD delete mode 100644 src/python/pants/testutil/subsystem/__init__.py delete mode 100644 src/python/pants/testutil/subsystem/util.py delete mode 100644 tests/python/pants_test/process/test_subprocess.py diff --git a/src/python/pants/backend/python/rules/pex_test.py b/src/python/pants/backend/python/rules/pex_test.py index cc7ba3ee7ae..16ec4b0a60e 100644 --- a/src/python/pants/backend/python/rules/pex_test.py +++ b/src/python/pants/backend/python/rules/pex_test.py @@ -31,45 +31,41 @@ from pants.testutil.engine.util import create_subsystem from pants.testutil.external_tool_test_base import ExternalToolTestBase from pants.testutil.option.util import create_options_bootstrapper -from pants.testutil.subsystem.util import init_subsystem from pants.util.frozendict import FrozenDict -from pants.util.strutil import create_path_env_var def test_merge_interpreter_constraints() -> None: - def assert_merged(*, input: List[List[str]], expected: List[str]) -> None: - assert PexInterpreterConstraints.merge_constraint_sets(input) == expected + def assert_merged(*, inp: List[List[str]], expected: List[str]) -> None: + assert PexInterpreterConstraints.merge_constraint_sets(inp) == expected # Multiple constraint sets get merged so that they are ANDed. # A & B => A & B - assert_merged( - input=[["CPython==2.7.*"], ["CPython==3.6.*"]], expected=["CPython==2.7.*,==3.6.*"] - ) + assert_merged(inp=[["CPython==2.7.*"], ["CPython==3.6.*"]], expected=["CPython==2.7.*,==3.6.*"]) # Multiple constraints within a single constraint set are kept separate so that they are ORed. # A | B => A | B assert_merged( - input=[["CPython==2.7.*", "CPython==3.6.*"]], expected=["CPython==2.7.*", "CPython==3.6.*"] + inp=[["CPython==2.7.*", "CPython==3.6.*"]], expected=["CPython==2.7.*", "CPython==3.6.*"] ) # Input constraints already were ANDed. # A => A - assert_merged(input=[["CPython>=2.7,<3"]], expected=["CPython>=2.7,<3"]) + assert_merged(inp=[["CPython>=2.7,<3"]], expected=["CPython>=2.7,<3"]) # Both AND and OR. # (A | B) & C => (A & B) | (B & C) assert_merged( - input=[["CPython>=2.7,<3", "CPython>=3.5"], ["CPython==3.6.*"]], + inp=[["CPython>=2.7,<3", "CPython>=3.5"], ["CPython==3.6.*"]], expected=["CPython>=2.7,<3,==3.6.*", "CPython>=3.5,==3.6.*"], ) # A & B & (C | D) => (A & B & C) | (A & B & D) assert_merged( - input=[["CPython==2.7.*"], ["CPython==3.6.*"], ["CPython==3.7.*", "CPython==3.8.*"]], + inp=[["CPython==2.7.*"], ["CPython==3.6.*"], ["CPython==3.7.*", "CPython==3.8.*"]], expected=["CPython==2.7.*,==3.6.*,==3.7.*", "CPython==2.7.*,==3.6.*,==3.8.*"], ) # (A | B) & (C | D) => (A & C) | (A & D) | (B & C) | (B & D) assert_merged( - input=[["CPython>=2.7,<3", "CPython>=3.5"], ["CPython==3.6.*", "CPython==3.7.*"]], + inp=[["CPython>=2.7,<3", "CPython>=3.5"], ["CPython==3.6.*", "CPython==3.7.*"]], expected=[ "CPython>=2.7,<3,==3.6.*", "CPython>=2.7,<3,==3.7.*", @@ -80,7 +76,7 @@ def assert_merged(*, input: List[List[str]], expected: List[str]) -> None: # A & (B | C | D) & (E | F) & G => # (A & B & E & G) | (A & B & F & G) | (A & C & E & G) | (A & C & F & G) | (A & D & E & G) | (A & D & F & G) assert_merged( - input=[ + inp=[ ["CPython==3.6.5"], ["CPython==2.7.14", "CPython==2.7.15", "CPython==2.7.16"], ["CPython>=3.6", "CPython==3.5.10"], @@ -102,13 +98,13 @@ def assert_merged(*, input: List[List[str]], expected: List[str]) -> None: # But, we first deduplicate each constraint_set. (A | B) & (A | B) can be rewritten as # X & X => X. assert_merged( - input=[["CPython==2.7.*", "CPython==3.6.*"], ["CPython==2.7.*", "CPython==3.6.*"]], + inp=[["CPython==2.7.*", "CPython==3.6.*"], ["CPython==2.7.*", "CPython==3.6.*"]], expected=["CPython==2.7.*", "CPython==3.6.*"], ) # (A | B) & C & (A | B) => (A & C) | (B & C). Alternatively, this can be rewritten as # X & Y & X => X & Y. assert_merged( - input=[ + inp=[ ["CPython>=2.7,<3", "CPython>=3.5"], ["CPython==3.6.*"], ["CPython>=3.5", "CPython>=2.7,<3"], @@ -117,21 +113,19 @@ def assert_merged(*, input: List[List[str]], expected: List[str]) -> None: ) # No specifiers - assert_merged(input=[["CPython"]], expected=["CPython"]) - assert_merged(input=[["CPython"], ["CPython==3.7.*"]], expected=["CPython==3.7.*"]) + assert_merged(inp=[["CPython"]], expected=["CPython"]) + assert_merged(inp=[["CPython"], ["CPython==3.7.*"]], expected=["CPython==3.7.*"]) # No interpreter is shorthand for CPython, which is how Pex behaves - assert_merged(input=[[">=3.5"], ["CPython==3.7.*"]], expected=["CPython>=3.5,==3.7.*"]) + assert_merged(inp=[[">=3.5"], ["CPython==3.7.*"]], expected=["CPython>=3.5,==3.7.*"]) # Different Python interpreters, which are guaranteed to fail when ANDed but are safe when ORed. with pytest.raises(ValueError): PexInterpreterConstraints.merge_constraint_sets([["CPython==3.7.*"], ["PyPy==43.0"]]) - assert_merged( - input=[["CPython==3.7.*", "PyPy==43.0"]], expected=["CPython==3.7.*", "PyPy==43.0"] - ) + assert_merged(inp=[["CPython==3.7.*", "PyPy==43.0"]], expected=["CPython==3.7.*", "PyPy==43.0"]) # Ensure we can handle empty input. - assert_merged(input=[], expected=[]) + assert_merged(inp=[], expected=[]) @dataclass(frozen=True) @@ -285,9 +279,9 @@ def test_pex_execution(self) -> None: assert "main.py" in pex_files assert "subdir/sub.py" in pex_files - init_subsystem(PythonSetup) - python_setup = PythonSetup.global_instance() - env = {"PATH": create_path_env_var(python_setup.interpreter_search_paths)} + # We reasonably expect there to be a python interpreter on the test-running + # process's path. + env = {"PATH": os.getenv("PATH", "")} process = Process( argv=("python", "test.pex"), diff --git a/src/python/pants/backend/python/rules/run_setup_py.py b/src/python/pants/backend/python/rules/run_setup_py.py index e240b4cfd1e..99e3f3728e6 100644 --- a/src/python/pants/backend/python/rules/run_setup_py.py +++ b/src/python/pants/backend/python/rules/run_setup_py.py @@ -314,11 +314,10 @@ async def run_setup_pys( exported_targets = list(FrozenOrderedSet(owners)) py2 = is_python2( - ( + python_setup.compatibilities_or_constraints( target_with_origin.target.get(PythonInterpreterCompatibility).value for target_with_origin in targets_with_origins - ), - python_setup, + ) ) chroots = await MultiGet( Get(SetupPyChroot, SetupPyChrootRequest(exported_target, py2)) diff --git a/src/python/pants/backend/python/rules/run_setup_py_test.py b/src/python/pants/backend/python/rules/run_setup_py_test.py index 7a8b79a1a19..1a675ffd3ef 100644 --- a/src/python/pants/backend/python/rules/run_setup_py_test.py +++ b/src/python/pants/backend/python/rules/run_setup_py_test.py @@ -45,7 +45,6 @@ from pants.python.python_requirement import PythonRequirement from pants.source.source_root import SourceRootConfig from pants.testutil.option.util import create_options_bootstrapper -from pants.testutil.subsystem.util import init_subsystem from pants.testutil.test_base import TestBase _namespace_decl = "__import__('pkg_resources').declare_namespace(__name__)" @@ -66,10 +65,6 @@ def tgt(self, addr: str) -> Target: return self.request_single_product(WrappedTarget, Params(Address.parse(addr))).target -def init_source_root(): - init_subsystem(SourceRootConfig, options={"source": {"root_patterns": ["src/python"]}}) - - class TestGenerateChroot(TestSetupPyBase): @classmethod def rules(cls): @@ -112,7 +107,6 @@ def assert_error(self, addr: str, exc_cls: Type[Exception]): assert type(ex.wrapped_exceptions[0]) == exc_cls def test_generate_chroot(self) -> None: - init_source_root() self.create_file( "src/python/foo/bar/baz/BUILD", "python_library(provides=setup_py(name='baz', version='1.1.1'))", @@ -176,7 +170,6 @@ def test_generate_chroot(self) -> None: ) def test_invalid_binary(self) -> None: - init_source_root() self.create_file( "src/python/invalid_binary/BUILD", textwrap.dedent( @@ -229,7 +222,7 @@ def assert_sources( SetupPySources, Params( SetupPySourcesRequest(Targets([self.tgt(addr) for addr in addrs]), py2=False), - SourceRootConfig.global_instance(), + create_options_bootstrapper(args=["--source-root-patterns=src/python"]), ), ) chroot_snapshot = self.request_single_product(Snapshot, Params(srcs.digest)) @@ -240,7 +233,6 @@ def assert_sources( assert expected_package_data == dict(srcs.package_data) def test_get_sources(self) -> None: - init_source_root() self.create_file( "src/python/foo/bar/baz/BUILD", textwrap.dedent( @@ -411,7 +403,8 @@ def assert_ancestor_init_py( ancestor_init_py_files = self.request_single_product( AncestorInitPyFiles, Params( - Targets([self.tgt(addr) for addr in addrs]), SourceRootConfig.global_instance(), + Targets([self.tgt(addr) for addr in addrs]), + create_options_bootstrapper(args=["--source-root-patterns=src/python"]), ), ) snapshots = [ @@ -423,7 +416,6 @@ def assert_ancestor_init_py( assert sorted(expected_init_pys) == sorted(init_py_files_found) def test_get_ancestor_init_py(self) -> None: - init_source_root() # NB: src/python/foo/bar/baz/qux/__init__.py is a target's source. self.create_file("src/python/foo/bar/baz/qux/BUILD", "python_library()") self.create_file("src/python/foo/bar/baz/qux/qux.py", "") diff --git a/src/python/pants/backend/python/rules/util.py b/src/python/pants/backend/python/rules/util.py index 4e865f18a3d..a542667abd6 100644 --- a/src/python/pants/backend/python/rules/util.py +++ b/src/python/pants/backend/python/rules/util.py @@ -4,7 +4,7 @@ import io import os from collections import abc, defaultdict -from typing import Dict, Iterable, List, Optional, Set, Tuple, cast +from typing import Dict, Iterable, List, Set, Tuple, cast from pkg_resources import Requirement @@ -13,7 +13,6 @@ from pants.core.util_rules.strip_source_roots import SourceRootStrippedSources from pants.engine.fs import DigestContents from pants.engine.target import Target -from pants.python.python_setup import PythonSetup from pants.source.source_root import SourceRootError from pants.util.strutil import ensure_text @@ -216,15 +215,12 @@ def has_args(call_node: ast.Call, required_arg_ids: Tuple[str, ...]) -> bool: return False -def is_python2( - compatibilities: Iterable[Optional[Iterable[str]]], python_setup: PythonSetup -) -> bool: +def is_python2(compatibilities_or_constraints: Iterable[str]) -> bool: """Checks if we should assume python2 code.""" def iter_reqs(): - for compatibility in compatibilities: - for constraint in python_setup.compatibility_or_constraints(compatibility): - yield parse_interpreter_constraint(constraint) + for constraint in compatibilities_or_constraints: + yield parse_interpreter_constraint(constraint) for req in iter_reqs(): for python_27_ver in range(0, 18): # The last python 2.7 version was 2.7.18. diff --git a/src/python/pants/backend/python/rules/util_test.py b/src/python/pants/backend/python/rules/util_test.py index 169ea256502..f244cdfb603 100644 --- a/src/python/pants/backend/python/rules/util_test.py +++ b/src/python/pants/backend/python/rules/util_test.py @@ -8,10 +8,6 @@ distutils_repr, is_python2, ) -from pants.option.ranked_value import Rank, RankedValue -from pants.python.python_setup import PythonSetup -from pants.subsystem.subsystem import Subsystem -from pants.testutil.subsystem.util import init_subsystem testdata = { "foo": "bar", @@ -80,49 +76,29 @@ def test_does_not_declare_pkg_resources_namespace_package(python_src: str) -> No @pytest.mark.parametrize( - ["constraints", "compatibilities"], + "constraints", [ - ([], [["CPython>=2.7,<3"]]), - (["CPython>=2.7,<3"], [None]), - (["CPython>=2.7,<3"], [["CPython>=2.7,<3"], ["CPython>=3.6"]]), - (["CPython>=2.7.13"], [None]), - (["CPython>=2.7.13,<2.7.16"], [None]), - (["CPython>=2.7.13,!=2.7.16"], [None]), - (["PyPy>=2.7,<3"], [None]), + ["CPython>=2.7,<3"], + ["CPython>=2.7,<3", "CPython>=3.6"], + ["CPython>=2.7.13"], + ["CPython>=2.7.13,<2.7.16"], + ["CPython>=2.7.13,!=2.7.16"], + ["PyPy>=2.7,<3"], ], ) -def test_is_python2(constraints, compatibilities): - Subsystem.reset() - init_subsystem( - PythonSetup, - { - PythonSetup.options_scope: { - "interpreter_constraints": RankedValue(Rank.CONFIG, constraints) - } - }, - ) - assert is_python2(compatibilities, PythonSetup.global_instance()) +def test_is_python2(constraints): + assert is_python2(constraints) @pytest.mark.parametrize( - ["constraints", "compatibilities"], + "constraints", [ - ([], [["CPython>=3.6"]]), - (["CPython>=3.6"], [None]), - (["CPython>=3.7"], [["CPython>=3.6"]]), - (["CPython>=3.7"], [["CPython>=3.6"], ["CPython>=3.8"]]), - (["CPython!=2.7.*"], [None]), - (["PyPy>=3.6"], [None]), + ["CPython>=3.6"], + ["CPython>=3.7"], + ["CPython>=3.6", "CPython>=3.8"], + ["CPython!=2.7.*"], + ["PyPy>=3.6"], ], ) -def test_is_not_python2(constraints, compatibilities): - Subsystem.reset() - init_subsystem( - PythonSetup, - { - PythonSetup.options_scope: { - "interpreter_constraints": RankedValue(Rank.CONFIG, constraints) - } - }, - ) - assert not is_python2(compatibilities, PythonSetup.global_instance()) +def test_is_not_python2(constraints): + assert not is_python2(constraints) diff --git a/src/python/pants/backend/python/target_types_test.py b/src/python/pants/backend/python/target_types_test.py index 6248b278fdb..61504fdc80b 100644 --- a/src/python/pants/backend/python/target_types_test.py +++ b/src/python/pants/backend/python/target_types_test.py @@ -8,12 +8,17 @@ from pants.backend.python.subsystems.pytest import PyTest from pants.backend.python.target_types import PythonBinarySources, PythonTestsTimeout from pants.engine.addresses import Address +from pants.engine.rules import SubsystemRule from pants.engine.target import InvalidFieldException -from pants.testutil.subsystem.util import global_subsystem_instance +from pants.testutil.option.util import create_options_bootstrapper from pants.testutil.test_base import TestBase class TestTimeout(TestBase): + @classmethod + def rules(cls): + return [*super().rules(), SubsystemRule(PyTest)] + def test_timeout_validation(self) -> None: with pytest.raises(InvalidFieldException): PythonTestsTimeout(-100, address=Address.parse(":tests")) @@ -28,18 +33,14 @@ def assert_timeout_calculated( expected: Optional[int], global_default: Optional[int] = None, global_max: Optional[int] = None, - timeouts_enabled: bool = True + timeouts_enabled: bool = True, ) -> None: - pytest = global_subsystem_instance( - PyTest, - options={ - PyTest.options_scope: { - "timeouts": timeouts_enabled, - "timeout_default": global_default, - "timeout_maximum": global_max, - } - }, - ) + args = ["--backend-packages=pants.backend.python", f"--pytest-timeouts={timeouts_enabled}"] + if global_default is not None: + args.append(f"--pytest-timeout-default={global_default}") + if global_max is not None: + args.append(f"--pytest-timeout-maximum={global_max}") + pytest = self.request_single_product(PyTest, create_options_bootstrapper(args=args)) field = PythonTestsTimeout(field_value, address=Address.parse(":tests")) assert field.calculate_from_global_options(pytest) == expected diff --git a/src/python/pants/python/python_setup.py b/src/python/pants/python/python_setup.py index ff3c32103fa..c3e54087656 100644 --- a/src/python/pants/python/python_setup.py +++ b/src/python/pants/python/python_setup.py @@ -195,6 +195,15 @@ def compatibility_or_constraints( return self.interpreter_constraints return tuple(compatibility or self.interpreter_constraints) + def compatibilities_or_constraints( + self, compatibilities: Iterable[Optional[Iterable[str]]] + ) -> Tuple[str, ...]: + return tuple( + constraint + for compatibility in compatibilities + for constraint in self.compatibility_or_constraints(compatibility) + ) + @classmethod def expand_interpreter_search_paths(cls, interpreter_search_paths, pyenv_root_func=None): special_strings = { diff --git a/src/python/pants/subsystem/subsystem.py b/src/python/pants/subsystem/subsystem.py index 82e3abd3b35..40c5736fe7b 100644 --- a/src/python/pants/subsystem/subsystem.py +++ b/src/python/pants/subsystem/subsystem.py @@ -47,10 +47,7 @@ class Subsystem(SubsystemClientMixin, Optionable): class UninitializedSubsystemError(SubsystemError): def __init__(self, class_name, scope): - super().__init__( - f'Subsystem "{class_name}" not initialized for scope "{scope}". Is subsystem missing ' - "from subsystem_dependencies() in a task? " - ) + super().__init__(f'Subsystem "{class_name}" not initialized for scope "{scope}"') @classmethod def is_subsystem_type(cls, obj): diff --git a/src/python/pants/testutil/BUILD b/src/python/pants/testutil/BUILD index 16b2b7ec98e..f1793d37e84 100644 --- a/src/python/pants/testutil/BUILD +++ b/src/python/pants/testutil/BUILD @@ -9,7 +9,6 @@ python_library( ':int-test-for-export', 'src/python/pants/testutil/engine', 'src/python/pants/testutil/option', - 'src/python/pants/testutil/subsystem', ], provides=pants_setup_py( name='pantsbuild.pants.testutil', diff --git a/src/python/pants/testutil/subsystem/BUILD b/src/python/pants/testutil/subsystem/BUILD deleted file mode 100644 index be9f51536f0..00000000000 --- a/src/python/pants/testutil/subsystem/BUILD +++ /dev/null @@ -1,4 +0,0 @@ -# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -python_library() diff --git a/src/python/pants/testutil/subsystem/__init__.py b/src/python/pants/testutil/subsystem/__init__.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/python/pants/testutil/subsystem/util.py b/src/python/pants/testutil/subsystem/util.py deleted file mode 100644 index 432a1397960..00000000000 --- a/src/python/pants/testutil/subsystem/util.py +++ /dev/null @@ -1,77 +0,0 @@ -# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -from pants.subsystem.subsystem import Subsystem -from pants.testutil.option.fakes import create_options_for_optionables - - -def global_subsystem_instance(subsystem_type, options=None): - """Returns the global instance of a subsystem, for use in tests. - - :API: public - - :param type subsystem_type: The subclass of :class:`pants.subsystem.subsystem.Subsystem` - to create. - :param options: dict of scope -> (dict of option name -> value). - The scopes may be that of the global instance of the subsystem (i.e., - subsystem_type.options_scope) and/or the scopes of instances of the - subsystems it transitively depends on. - """ - init_subsystem(subsystem_type, options) - return subsystem_type.global_instance() - - -def init_subsystems(subsystem_types, options=None): - """Initialize subsystems for use in tests. - - Does not create an instance. This function is for setting up subsystems that the code - under test creates. - - Note that there is some redundancy between this function and BaseTest.context(for_subsystems=...). - TODO: Fix that. - - :API: public - - :param list subsystem_types: The subclasses of :class:`pants.subsystem.subsystem.Subsystem` - to create. - :param options: dict of scope -> (dict of option name -> value). - The scopes may be those of the global instances of the subsystems (i.e., - subsystem_type.options_scope) and/or the scopes of instances of the - subsystems they transitively depend on. - """ - optionables = set() - for s in subsystem_types: - if not Subsystem.is_subsystem_type(s): - raise TypeError(f"{s} is not a subclass of `Subsystem`") - for si in s.known_scope_infos(): - optionables.add(si.optionable_cls) - if options: - allowed_scopes = {o.options_scope for o in optionables} - for scope in options.keys(): - if scope != "" and scope not in allowed_scopes: - raise ValueError( - "`{}` is not the scope of any of these subsystems: {}".format( - scope, optionables - ) - ) - # Don't trample existing subsystem options, in case a test has set up some - # other subsystems in some other way. - updated_options = dict(Subsystem._options.items()) if Subsystem._options else {} - if options: - updated_options.update(options) - Subsystem.set_options(create_options_for_optionables(optionables, options=updated_options)) - - -def init_subsystem(subsystem_type, options=None): - """Singular form of :func:`pants_test.subsystem.subsystem_util.init_subsystems` - - :API: public - - :param subsystem_type: The subclass of :class:`pants.subsystem.subsystem.Subsystem` - to create. - :param options: dict of scope -> (dict of option name -> value). - The scopes may be those of the global instance of the subsystem (i.e., - subsystem_type.options_scope) and/or the scopes of instance of the - subsystem it transitively depends on. - """ - init_subsystems([subsystem_type], options) diff --git a/tests/python/pants_test/process/test_subprocess.py b/tests/python/pants_test/process/test_subprocess.py deleted file mode 100644 index 40087043772..00000000000 --- a/tests/python/pants_test/process/test_subprocess.py +++ /dev/null @@ -1,14 +0,0 @@ -# Copyright 2016 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -from pants.process.subprocess import Subprocess -from pants.testutil.subsystem.util import global_subsystem_instance -from pants.testutil.test_base import TestBase - - -class SubprocessTest(TestBase): - def subprocess(self): - return global_subsystem_instance(Subprocess.Factory).create() - - def test_get_subprocess_dir(self): - self.assertTrue(self.subprocess().get_subprocess_dir().endswith("/.pids"))