From 496a680ef3179f44ca8603a978c7b87eb54d507a Mon Sep 17 00:00:00 2001 From: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Date: Sat, 22 Aug 2020 22:35:07 -0700 Subject: [PATCH] Change `PantsIntegrationTest` to be hermetic by default (#10672) Several refactors: * Make `hermetic` a class property rather than classmethod. Default to True. * Rename `use_pantsd_env_var` to a class property `use_pantsd`. Removes the env var mechanism as we no longer use that. Defaults to pantsd being used. * Rename `hermetic_env_whitelist` to `hermetic_env_allowlist`, and make it a class property. * Add type hints. * Enforce kwargs. * Remove `do_command()` in favor of `self.run_pants()`. It does the same thing. We want to simplify. * Remove the unused `file_renamed`. We generally are moving in the direction of our integration tests creating their own environment, rather than mutating the build root (e.g. testprojects). This wasn't being used. [ci skip-rust] [ci skip-build-wheels] --- build-support/bin/generate_docs.py | 2 +- pants.toml | 1 - .../python/rules/coverage_integration_test.py | 1 + .../run_python_binary_integration_test.py | 1 + .../base/exception_sink_integration_test.py | 2 + .../pants/base/exiter_integration_test.py | 10 +- src/python/pants/bin/BUILD | 1 - .../subproject_integration_test.py | 19 +- .../pants/core/goals/fmt_integration_test.py | 16 +- src/python/pants/engine/internals/BUILD | 1 + .../internals/scheduler_integration_test.py | 2 + .../pants/help/help_integration_test.py | 2 +- .../pants/option/options_integration_test.py | 9 +- .../pants/testutil/pants_integration_test.py | 225 +++++++----------- .../integration/changed_integration_test.py | 10 +- .../integration/goal_rule_integration_test.py | 25 +- .../integration/graph_integration_test.py | 8 - .../integration/list_integration_test.py | 32 +-- .../log_output_integration_test.py | 8 +- .../integration/test_prelude_integration.py | 1 + .../native_engine_logging_integration_test.py | 13 +- .../pantsd/pantsd_integration_test.py | 40 ++-- .../pantsd/pantsd_integration_test_base.py | 13 +- 23 files changed, 216 insertions(+), 226 deletions(-) diff --git a/build-support/bin/generate_docs.py b/build-support/bin/generate_docs.py index e0ccec68c3c..1ab0f69d2df 100644 --- a/build-support/bin/generate_docs.py +++ b/build-support/bin/generate_docs.py @@ -25,7 +25,7 @@ class ReferenceGenerator: To run use: ./pants \ - --backend-packages="-['internal_backend.rules_for_testing', 'internal_backend.utilities']" \ + --backend-packages="-['internal_backend.utilities']" \ --backend-packages="+['pants.backend.python.lint.bandit', \ 'pants.backend.python.lint.pylint', 'pants.backend.codegen.protobuf.python', \ 'pants.backend.awslambda.python']" \ diff --git a/pants.toml b/pants.toml index 92e8da51027..c92d89ebe8d 100644 --- a/pants.toml +++ b/pants.toml @@ -11,7 +11,6 @@ backend_packages.add = [ "pants.backend.python.lint.flake8", "pants.backend.python.lint.isort", "pants.backend.python.typecheck.mypy", - "internal_backend.rules_for_testing", "internal_backend.utilities", ] diff --git a/src/python/pants/backend/python/rules/coverage_integration_test.py b/src/python/pants/backend/python/rules/coverage_integration_test.py index 580bd03549a..5edb221e14f 100644 --- a/src/python/pants/backend/python/rules/coverage_integration_test.py +++ b/src/python/pants/backend/python/rules/coverage_integration_test.py @@ -131,6 +131,7 @@ def test_arithmetic(): def _run_tests(self, tmpdir_relative, *more_args: str) -> PantsResult: command = [ + "--backend-packages=pants.backend.python", "test", "--use-coverage", f"{tmpdir_relative}/src/python/project:tests", diff --git a/src/python/pants/backend/python/rules/run_python_binary_integration_test.py b/src/python/pants/backend/python/rules/run_python_binary_integration_test.py index d1c8d25d70a..7d6bdcbf7c9 100644 --- a/src/python/pants/backend/python/rules/run_python_binary_integration_test.py +++ b/src/python/pants/backend/python/rules/run_python_binary_integration_test.py @@ -52,6 +52,7 @@ def upper_case(s): (src_root2 / "BUILD").write_text("python_library()") result = self.run_pants( [ + "--backend-packages=pants.backend.python", ( f"--source-root-patterns=['/{tmpdir_relative}/src_root1', " f"'/{tmpdir_relative}/src_root2']" diff --git a/src/python/pants/base/exception_sink_integration_test.py b/src/python/pants/base/exception_sink_integration_test.py index c035c600d54..b86104a3ded 100644 --- a/src/python/pants/base/exception_sink_integration_test.py +++ b/src/python/pants/base/exception_sink_integration_test.py @@ -14,6 +14,8 @@ class ExceptionSinkIntegrationTest(PantsDaemonIntegrationTestBase): + hermetic = False + def _assert_unhandled_exception_log_matches(self, pid, file_contents, namespace): self.assertRegex( file_contents, diff --git a/src/python/pants/base/exiter_integration_test.py b/src/python/pants/base/exiter_integration_test.py index 81172f70cde..1aa96a0034d 100644 --- a/src/python/pants/base/exiter_integration_test.py +++ b/src/python/pants/base/exiter_integration_test.py @@ -9,8 +9,12 @@ class ExiterIntegrationTest(PantsIntegrationTest): @ensure_daemon def test_unicode_containing_exception(self): - """Test whether we can run a single target without special flags.""" - pants_run = self.run_pants(["run", "testprojects/src/python/unicode/compilation_failure"]) + pants_run = self.run_pants( + [ + "--backend-packages=pants.backend.python", + "run", + "testprojects/src/python/unicode/compilation_failure", + ] + ) self.assert_failure(pants_run) - self.assertIn("import sys¡", pants_run.stderr) diff --git a/src/python/pants/bin/BUILD b/src/python/pants/bin/BUILD index 7a703e8a88d..70ab38b1021 100644 --- a/src/python/pants/bin/BUILD +++ b/src/python/pants/bin/BUILD @@ -48,7 +48,6 @@ python_binary( 'build-support/flake8', 'build-support/mypy', 'build-support/regexes', - 'pants-plugins/src/python/internal_backend/rules_for_testing', 'pants-plugins/src/python/internal_backend/utilities', ], # We depend on twitter.common libraries that trigger pex warnings for not properly declaring their diff --git a/src/python/pants/build_graph/subproject_integration_test.py b/src/python/pants/build_graph/subproject_integration_test.py index ed98143cbf5..34bba867b8f 100644 --- a/src/python/pants/build_graph/subproject_integration_test.py +++ b/src/python/pants/build_graph/subproject_integration_test.py @@ -77,12 +77,22 @@ class SubprojectIntegrationTest(PantsIntegrationTest): def test_subproject(self) -> None: with harness(): # If `--subproject-roots` are not specified, we expect a failure. - self.assert_failure(self.run_pants(["dependencies", "--transitive", SUBPROJ_SPEC])) + self.assert_failure( + self.run_pants( + [ + "--backend-packages=pants.backend.python", + "dependencies", + "--transitive", + SUBPROJ_SPEC, + ] + ) + ) # The same command should succeed when `--subproject-roots` are specified. self.assert_success( self.run_pants( [ + "--backend-packages=pants.backend.python", f"--subproject-roots={SUBPROJ_ROOT}", "dependencies", "--transitive", @@ -94,6 +104,11 @@ def test_subproject(self) -> None: # Both relative and absolute dependencies should work. self.assert_success( self.run_pants( - [f"--subproject-roots={SUBPROJ_ROOT}", "dependencies", f"{SUBPROJ_ROOT}:local"] + [ + "--backend-packages=pants.backend.python", + f"--subproject-roots={SUBPROJ_ROOT}", + "dependencies", + f"{SUBPROJ_ROOT}:local", + ] ) ) diff --git a/src/python/pants/core/goals/fmt_integration_test.py b/src/python/pants/core/goals/fmt_integration_test.py index e363b077829..65c91219fba 100644 --- a/src/python/pants/core/goals/fmt_integration_test.py +++ b/src/python/pants/core/goals/fmt_integration_test.py @@ -12,10 +12,20 @@ class FmtIntegrationTest(PantsIntegrationTest): def test_fmt_then_edit(self): f = "examples/src/python/example/hello/greet/greet.py" with self.temporary_workdir() as workdir: - run = lambda: self.run_pants_with_workdir(["fmt", f], workdir=workdir) + + def run() -> None: + result = self.run_pants_with_workdir( + [ + "--backend-packages=['pants.backend.python', 'pants.backend.python.lint.black']", + "fmt", + f, + ], + workdir=workdir, + ) + self.assert_success(result) # Run once to start up, and then capture the file content. - self.assert_success(run()) + run() good_content = read_file(f) # Edit the file. @@ -25,5 +35,5 @@ def test_fmt_then_edit(self): assert good_content != read_file(f) # Re-run and confirm that the file was fixed. - self.assert_success(run()) + run() assert good_content == read_file(f) diff --git a/src/python/pants/engine/internals/BUILD b/src/python/pants/engine/internals/BUILD index 733e2eb1025..8337b49c870 100644 --- a/src/python/pants/engine/internals/BUILD +++ b/src/python/pants/engine/internals/BUILD @@ -16,6 +16,7 @@ python_integration_tests( uses_pants_run=True, dependencies=[ 'examples/src/python/example:hello_directory', + 'pants-plugins/src/python/internal_backend/rules_for_testing', ], timeout=180, ) diff --git a/src/python/pants/engine/internals/scheduler_integration_test.py b/src/python/pants/engine/internals/scheduler_integration_test.py index b7ad5fd0c7d..2ed8a596e74 100644 --- a/src/python/pants/engine/internals/scheduler_integration_test.py +++ b/src/python/pants/engine/internals/scheduler_integration_test.py @@ -15,6 +15,7 @@ def test_visualize_to(self): with temporary_dir() as destdir: args = [ f"--native-engine-visualize-to={destdir}", + "--backend-packages=pants.backend.python", "list", "examples/src/python/example/hello/greet", ] @@ -25,6 +26,7 @@ def test_visualize_to(self): @ensure_daemon def test_graceful_termination(self): args = [ + "--backend-packages=['pants.backend.python', 'internal_backend.rules_for_testing']", "list-and-die-for-testing", "examples/src/python/example/hello/greet", ] diff --git a/src/python/pants/help/help_integration_test.py b/src/python/pants/help/help_integration_test.py index 2e6d6f54b3e..80a3736bf6e 100644 --- a/src/python/pants/help/help_integration_test.py +++ b/src/python/pants/help/help_integration_test.py @@ -25,7 +25,7 @@ def test_help_advanced(self): assert "--pants-bootstrapdir" in pants_run.stdout def test_help_all(self): - command = ["help-all"] + command = ["--backend-packages=pants.backend.python", "help-all"] pants_run = self.run_pants(command=command) self.assert_success(pants_run) all_help = json.loads(pants_run.stdout) diff --git a/src/python/pants/option/options_integration_test.py b/src/python/pants/option/options_integration_test.py index 8bab7b69432..a75e28cdb38 100644 --- a/src/python/pants/option/options_integration_test.py +++ b/src/python/pants/option/options_integration_test.py @@ -14,10 +14,6 @@ @pytest.mark.skip("Skip until https://github.com/pantsbuild/pants/issues/10206") class TestOptionsIntegration(PantsIntegrationTest): - @classmethod - def hermetic(cls) -> bool: - return True - def test_options_works_at_all(self) -> None: self.assert_success(self.run_pants(["options"])) @@ -302,7 +298,8 @@ def test_pants_symlink_workdirs(self) -> None: physical_workdir = f"{physical_workdir_base}/{safe_filename_from_path(symlink_workdir)}" pants_run = self.run_pants_with_workdir( - [f"--pants-physical-workdir-base={physical_workdir_base}", "help"], symlink_workdir + [f"--pants-physical-workdir-base={physical_workdir_base}", "help"], + workdir=symlink_workdir, ) self.assert_success(pants_run) # Make sure symlink workdir is pointing to physical workdir @@ -310,7 +307,7 @@ def test_pants_symlink_workdirs(self) -> None: self.run_pants_with_workdir( [f"--pants-physical-workdir-base={physical_workdir_base}", "clean-all"], - symlink_workdir, + workdir=symlink_workdir, ) # Make sure both physical_workdir and symlink_workdir are empty after running clean-all self.assertTrue(not os.listdir(symlink_workdir) and not os.listdir(physical_workdir)) diff --git a/src/python/pants/testutil/pants_integration_test.py b/src/python/pants/testutil/pants_integration_test.py index 92310444b48..2d97e0df67e 100644 --- a/src/python/pants/testutil/pants_integration_test.py +++ b/src/python/pants/testutil/pants_integration_test.py @@ -8,17 +8,15 @@ import unittest from contextlib import contextmanager, suppress from dataclasses import dataclass -from operator import eq, ne from pathlib import Path from threading import Lock -from typing import Callable, Iterator, List, Optional, Union +from typing import Any, Callable, Iterator, List, Mapping, Optional, Union from pants.base.build_environment import get_buildroot from pants.base.deprecated import warn_or_error from pants.base.exiter import PANTS_SUCCEEDED_EXIT_CODE from pants.option.config import TomlSerializer from pants.option.options_bootstrapper import OptionsBootstrapper -from pants.option.subsystem import Subsystem from pants.pantsd.pants_daemon_client import PantsDaemonClient from pants.testutil.process_handler import SubprocessProcessHandler from pants.util.contextutil import environment_as, temporary_dir @@ -141,7 +139,7 @@ def wrapper(self, *args, **kwargs): return wrapper -def render_logs(workdir): +def render_logs(workdir: str) -> None: """Renders all potentially relevant logs from the given workdir to stdout.""" filenames = list(glob.glob(os.path.join(workdir, "logs/exceptions*log"))) + list( glob.glob(os.path.join(workdir, "pantsd/pantsd.log")) @@ -154,14 +152,14 @@ def render_logs(workdir): print(f"{rel_filename} --- ") -def read_pantsd_log(workdir): +def read_pantsd_log(workdir: str) -> Iterator[str]: """Yields all lines from the pantsd log under the given workdir.""" # Surface the pantsd log for easy viewing via pytest's `-s` (don't capture stdio) option. for line in _read_log(f"{workdir}/pantsd/pantsd.log"): yield line -def _read_log(filename): +def _read_log(filename: str) -> Iterator[str]: with open(filename, "r") as f: for line in f: yield line.rstrip() @@ -170,62 +168,15 @@ def _read_log(filename): class PantsIntegrationTest(unittest.TestCase): """A base class for integration tests that run Pants.""" - class InvalidTestEnvironmentError(Exception): - """Raised when the external environment is not set up properly to run integration tests.""" - - @classmethod - def use_pantsd_env_var(cls): - """Subclasses may override to acknowledge that the tests cannot run when pantsd is enabled, - or they want to configure pantsd themselves. - - In those cases, --pantsd will not be added to their configuration. - This approach is coarsely grained, meaning we disable pantsd in some tests that actually run - when pantsd is enabled. However: - - The number of mislabeled tests is currently small (~20 tests). - - Those tests will still run, just with pantsd disabled. - - N.B. Currently, this doesn't interact with test hermeticity. - This means that, if the test coordinator has set PANTS_PANTSD, and a test is not marked - as hermetic, it will run under pantsd regardless of the value of this function. - """ - should_pantsd = os.getenv("USE_PANTSD_FOR_INTEGRATION_TESTS") - return should_pantsd in ["True", "true", "1"] - - @classmethod - def hermetic(cls): - """Subclasses may override to acknowledge that they are hermetic. - - That is, that they should run without reading the real pants.toml. - """ - return False - - @classmethod - def hermetic_env_whitelist(cls): - """A whitelist of environment variables to propagate to tests when hermetic=True.""" - return [ - # Used in the wrapper script to locate a rust install. - "HOME", - # Needed to find python interpreters and other binaries. - "PATH", - "PANTS_PROFILE", - # Ensure that the underlying ./pants invocation doesn't run from sources - # (and therefore bootstrap) if we don't want it to. - "RUN_PANTS_FROM_PEX", - ] - - def setUp(self): - super().setUp() - # Some integration tests rely on clean subsystem state (e.g., to set up a DistributionLocator). - Subsystem.reset() - - def temporary_workdir(self, cleanup=True): - # We can hard-code '.pants.d' here because we know that will always be its value - # in the pantsbuild/pants repo (e.g., that's what we .gitignore in that repo). - # Grabbing the pants_workdir config would require this pants's config object, - # which we don't have a reference to here. - root = os.path.join(get_buildroot(), ".pants.d", "tmp") - safe_mkdir(root) - return temporary_dir(root_dir=root, cleanup=cleanup, suffix=".pants.d") + # Classes can optionally override these. + hermetic = True # If False, pants.toml will be used. + hermetic_env_allowlist = ( + "HOME", + "PATH", # Needed to find Python interpreters and other binaries. + "PANTS_PROFILE", + "RUN_PANTS_FROM_PEX", + ) + use_pantsd = True # Incremented each time we spawn a pants subprocess. # Appended to PANTS_PROFILE in the called pants process, so that each subprocess @@ -234,7 +185,7 @@ def temporary_workdir(self, cleanup=True): _profile_disambiguator_lock = Lock() @classmethod - def _get_profile_disambiguator(cls): + def _get_profile_disambiguator(cls) -> int: with cls._profile_disambiguator_lock: ret = cls._profile_disambiguator cls._profile_disambiguator += 1 @@ -242,13 +193,13 @@ def _get_profile_disambiguator(cls): def run_pants_with_workdir_without_waiting( self, - command, - workdir, - config=None, - extra_env=None, - build_root=None, - print_exception_stacktrace=True, - **kwargs, + command: Command, + *, + workdir: str, + config: Optional[Mapping] = None, + extra_env: Optional[Mapping[str, str]] = None, + print_exception_stacktrace: bool = True, + **kwargs: Any, ) -> PantsJoinHandle: args = [ "--no-pantsrc", @@ -256,48 +207,41 @@ def run_pants_with_workdir_without_waiting( f"--print-exception-stacktrace={print_exception_stacktrace}", ] - if self.hermetic(): - args.extend( - [ - "--pants-config-files=[]", - # Turn off cache globally. A hermetic integration test shouldn't rely on cache, - # or we have no idea if it's actually testing anything. - "--no-cache-read", - "--no-cache-write", - # Turn cache on just for tool bootstrapping, for performance. - "--cache-bootstrap-read", - "--cache-bootstrap-write", - ] - ) + pantsd_in_command = "--no-pantsd" in command or "--pantsd" in command + pantsd_in_config = config and "GLOBAL" in config and "pantsd" in config["GLOBAL"] + if not pantsd_in_command and not pantsd_in_config: + args.append("--pantsd" if self.use_pantsd else "--no-pantsd") - if self.use_pantsd_env_var(): - args.append("--pantsd") + if self.hermetic: + args.append("--pants-config-files=[]") if config: toml_file_name = os.path.join(workdir, "pants.toml") with safe_open(toml_file_name, mode="w") as fp: - fp.write(TomlSerializer(config).serialize()) - args.append("--pants-config-files=" + toml_file_name) + fp.write(TomlSerializer(config).serialize()) # type: ignore[arg-type] + args.append(f"--pants-config-files={toml_file_name}") pants_script = [sys.executable, "-m", "pants"] # Permit usage of shell=True and string-based commands to allow e.g. `./pants | head`. + pants_command: Command if kwargs.get("shell") is True: assert not isinstance( command, list ), "must pass command as a string when using shell=True" pants_command = " ".join([*pants_script, " ".join(args), command]) else: - pants_command = pants_script + args + command + pants_command = [*pants_script, *args, *command] - # Only whitelisted entries will be included in the environment if hermetic=True. - if self.hermetic(): - env = dict() + # Only allow-listed entries will be included in the environment if hermetic=True. Note that + # the env will already be fairly hermetic thanks to the v2 engine; this provides an + # additional layer of hermiticity. + if self.hermetic: # With an empty environment, we would generally get the true underlying system default # encoding, which is unlikely to be what we want (it's generally ASCII, still). So we # explicitly set an encoding here. - env["LC_ALL"] = "en_US.UTF-8" - for h in self.hermetic_env_whitelist(): + env = {"LC_ALL": "en_US.UTF-8"} + for h in self.hermetic_env_allowlist: value = os.getenv(h) if value is not None: env[h] = value @@ -322,7 +266,8 @@ def run_pants_with_workdir_without_waiting( if env.get("PANTS_PROFILE"): prof = f"{env['PANTS_PROFILE']}.{self._get_profile_disambiguator()}" env["PANTS_PROFILE"] = prof - # Make a note the subprocess command, so the user can correctly interpret the profile files. + # Make a note of the subprocess command, so the user can correctly interpret the + # profile files. with open(f"{prof}.cmd", "w") as fp: fp.write(" ".join(pants_command)) @@ -340,42 +285,66 @@ def run_pants_with_workdir_without_waiting( ) def run_pants_with_workdir( - self, command, workdir, config=None, stdin_data=None, tee_output=False, **kwargs + self, + command: Command, + *, + workdir: str, + config: Optional[Mapping] = None, + stdin_data: Optional[Union[bytes, str]] = None, + tee_output: bool = False, + **kwargs: Any, ) -> PantsResult: if config: kwargs["config"] = config - handle = self.run_pants_with_workdir_without_waiting(command, workdir, **kwargs) + handle = self.run_pants_with_workdir_without_waiting(command, workdir=workdir, **kwargs) return handle.join(stdin_data=stdin_data, tee_output=tee_output) def run_pants( - self, command, config=None, stdin_data=None, extra_env=None, cleanup_workdir=True, **kwargs + self, + command: Command, + config: Optional[Mapping] = None, + stdin_data: Optional[Union[bytes, str]] = None, + extra_env: Optional[Mapping[str, str]] = None, + **kwargs: Any, ) -> PantsResult: - """Runs pants in a subprocess. + """Runs Pants in a subprocess. - :param list command: A list of command line arguments coming after `./pants`. + :param command: A list of command line arguments coming after `./pants`. :param config: Optional data for a generated TOML file. A map of -> - map of key -> value. + map of key -> value. :param kwargs: Extra keyword args to pass to `subprocess.Popen`. """ with self.temporary_workdir() as workdir: return self.run_pants_with_workdir( - command, workdir, config, stdin_data=stdin_data, extra_env=extra_env, **kwargs + command, + workdir=workdir, + config=config, + stdin_data=stdin_data, + extra_env=extra_env, + **kwargs, ) - def assert_success(self, pants_run: PantsResult, msg=None): - self.assert_result(pants_run, PANTS_SUCCEEDED_EXIT_CODE, expected=True, msg=msg) + def assert_success(self, pants_run: PantsResult, msg: Optional[str] = None) -> None: + self._assert_result(pants_run, success_expected=True, msg=msg) - def assert_failure(self, pants_run: PantsResult, msg=None): - self.assert_result(pants_run, PANTS_SUCCEEDED_EXIT_CODE, expected=False, msg=msg) + def assert_failure(self, pants_run: PantsResult, msg: Optional[str] = None) -> None: + self._assert_result(pants_run, success_expected=False, msg=msg) - def assert_result(self, pants_run: PantsResult, value, expected=True, msg=None): - check, assertion = (eq, self.assertEqual) if expected else (ne, self.assertNotEqual) - if check(pants_run.exit_code, value): + @staticmethod + def _assert_result( + pants_run: PantsResult, *, success_expected: bool, msg: Optional[str] + ) -> None: + is_expected_exit_code = ( + pants_run.exit_code == PANTS_SUCCEEDED_EXIT_CODE + if success_expected + else pants_run.exit_code != PANTS_SUCCEEDED_EXIT_CODE + ) + if is_expected_exit_code: return details = [msg] if msg else [] details.append(" ".join(pants_run.command)) - details.append(f"returncode: {pants_run.exit_code}") + details.append(f"exit_code: {pants_run.exit_code}") def indent(content): return "\n\t".join(content.splitlines()) @@ -384,20 +353,20 @@ def indent(content): details.append(f"stderr:\n\t{indent(pants_run.stderr)}") error_msg = "\n".join(details) - assertion(value, pants_run.exit_code, error_msg) + assert is_expected_exit_code, error_msg - @contextmanager - def file_renamed(self, prefix, test_name, real_name): - real_path = os.path.join(prefix, real_name) - test_path = os.path.join(prefix, test_name) - try: - os.rename(test_path, real_path) - yield - finally: - os.rename(real_path, test_path) + @staticmethod + def temporary_workdir(cleanup: bool = True): + # We can hard-code '.pants.d' here because we know that will always be its value + # in the pantsbuild/pants repo (e.g., that's what we .gitignore in that repo). + # Grabbing the pants_workdir config would require this pants's config object, + # which we don't have a reference to here. + root = os.path.join(get_buildroot(), ".pants.d", "tmp") + safe_mkdir(root) + return temporary_dir(root_dir=root, cleanup=cleanup, suffix=".pants.d") @contextmanager - def temporary_directory_literal(self, path: Union[str, Path],) -> Iterator[None]: + def temporary_directory_literal(self, path: Union[str, Path]) -> Iterator[None]: """Temporarily create the given literal directory under the buildroot. The path being created must not already exist. Any parent directories will also be created @@ -423,7 +392,7 @@ def temporary_directory_literal(self, path: Union[str, Path],) -> Iterator[None] @contextmanager def temporary_file_content( - self, path: Union[str, Path], content, binary_mode=True + self, path: Union[str, Path], content, binary_mode: bool = True ) -> Iterator[None]: """Temporarily write content to a file for the purpose of an integration test.""" path = os.path.realpath(path) @@ -478,17 +447,3 @@ def with_overwritten_file_content( finally: with open(file_path, "wb") as f: f.write(file_original_content) - - def do_command(self, *args, **kwargs) -> PantsResult: - """Wrapper around run_pants method. - - :param args: command line arguments used to run pants - """ - cmd = list(args) - success = kwargs.pop("success", True) - pants_run = self.run_pants(cmd, **kwargs) - if success: - self.assert_success(pants_run) - else: - self.assert_failure(pants_run) - return pants_run diff --git a/tests/python/pants_test/integration/changed_integration_test.py b/tests/python/pants_test/integration/changed_integration_test.py index 5c4a5eb00e9..80c0a95a3c3 100644 --- a/tests/python/pants_test/integration/changed_integration_test.py +++ b/tests/python/pants_test/integration/changed_integration_test.py @@ -5,6 +5,7 @@ import shutil from contextlib import contextmanager from textwrap import dedent +from typing import List, Optional import pytest @@ -134,6 +135,8 @@ def add_to_git(commit_msg, *files): class ChangedIntegrationTest(PantsIntegrationTest, AbstractTestGenerator): + # Git isn't detected if hermetic=True for some reason. + hermetic = False TEST_MAPPING = { # A `python_binary` with `sources=['file.name']`. @@ -210,8 +213,9 @@ def inner_integration_coverage_test( inner_integration_coverage_test, ) - def run_list(self, extra_args, success=True): - pants_run = self.do_command("list", *extra_args, success=success) + def run_list(self, extra_args: Optional[List[str]] = None) -> str: + pants_run = self.run_pants(["list", *(extra_args or ())]) + self.assert_success(pants_run) return pants_run.stdout def test_changed_exclude_root_targets_only(self): @@ -280,7 +284,7 @@ def test_changed_with_multiple_build_files(self): new_build_file = "src/python/python_targets/BUILD.new" with create_isolated_git_repo() as worktree: touch(os.path.join(worktree, new_build_file)) - stdout_data = self.run_list([]) + stdout_data = self.run_list() self.assertEqual(stdout_data.strip(), "") def test_changed_with_deleted_source(self): diff --git a/tests/python/pants_test/integration/goal_rule_integration_test.py b/tests/python/pants_test/integration/goal_rule_integration_test.py index b5be0d2b37a..c29a6eba7b8 100644 --- a/tests/python/pants_test/integration/goal_rule_integration_test.py +++ b/tests/python/pants_test/integration/goal_rule_integration_test.py @@ -16,11 +16,15 @@ @pytest.mark.skip(reason="Flaky test. https://github.com/pantsbuild/pants/issues/10478") class TestGoalRuleIntegration(PantsDaemonIntegrationTestBase): + # TODO: Set hermetic=True after rewriting this test to stop using an example project. + hermetic = False + target = "examples/src/python/example/hello::" @ensure_daemon def test_list(self): - result = self.do_command("list", self.target, success=True) + result = self.run_pants(["list", self.target]) + self.assert_success(result) output_lines = result.stdout.splitlines() self.assertEqual(len(output_lines), 4) self.assertIn("examples/src/python/example/hello/main", output_lines) @@ -39,8 +43,8 @@ def run_list(): @ensure_daemon def test_goal_validation(self): - result = self.do_command("blah", "::", success=False) - + result = self.run_pants(["blah", "::"]) + self.assert_failure(result) self.assertIn("Unknown goals: blah", result.stdout) def test_list_loop(self): @@ -60,7 +64,7 @@ def dump(content): # Launch the loop as a background process. handle = self.run_pants_with_workdir_without_waiting( - ["--loop", "--loop-max=3", "list", f"{tmpdir}:",], workdir, config, + ["--loop", "--loop-max=3", "list", f"{tmpdir}:"], workdir=workdir, config=config ) # Wait for pantsd to come up and for the loop to stabilize. @@ -84,11 +88,10 @@ def test_unimplemented_goals_noop(self) -> None: # and will fail when given the glob `::`. command_prefix = ["--pants-config-files=[]"] target = "testprojects/tests/python/pants/dummies::" - self.do_command(*command_prefix, "--backend-packages=[]", "run", target, success=True) - self.do_command( - *command_prefix, - "--backend-packages='pants.backend.python'", - "run", - target, - success=False, + result = self.run_pants([*command_prefix, "--backend-packages=[]", "run", target]) + self.assert_success(result) + + result = self.run_pants( + [*command_prefix, "--backend-packages='pants.backend.python'", "run", target] ) + self.assert_failure(result) diff --git a/tests/python/pants_test/integration/graph_integration_test.py b/tests/python/pants_test/integration/graph_integration_test.py index 9716f8de601..ce01eb27a70 100644 --- a/tests/python/pants_test/integration/graph_integration_test.py +++ b/tests/python/pants_test/integration/graph_integration_test.py @@ -13,14 +13,6 @@ class GraphIntegrationTest(PantsIntegrationTest): - @classmethod - def use_pantsd_env_var(cls): - """Some of the tests here expect to read the standard error after an intentional failure. - - However, when pantsd is enabled, these errors are logged to logs/exceptions..log So - stderr appears empty. (see #7320) - """ - return False _NO_BUILD_FILE_TARGET_BASE = "testprojects/src/python/no_build_file" _SOURCES_TARGET_BASE = "testprojects/src/python/sources" diff --git a/tests/python/pants_test/integration/list_integration_test.py b/tests/python/pants_test/integration/list_integration_test.py index 6443c3cc1ca..858e54fa18c 100644 --- a/tests/python/pants_test/integration/list_integration_test.py +++ b/tests/python/pants_test/integration/list_integration_test.py @@ -5,28 +5,30 @@ class ListIntegrationTest(PantsIntegrationTest): - def get_target_set(self, std_out): - return sorted([l for l in std_out.split("\n") if l]) - - def run_engine_list(self, success, *args): - return self.get_target_set(self.do_command(*args, success=success).stdout) - - def test_list_all(self): - pants_run = self.do_command("list", "::", success=True) + def test_list_all(self) -> None: + pants_run = self.run_pants(["--backend-packages=pants.backend.python", "list", "::"]) + self.assert_success(pants_run) self.assertGreater(len(pants_run.stdout.strip().split()), 1) - def test_list_none(self): - pants_run = self.do_command("list", success=True) + def test_list_none(self) -> None: + pants_run = self.run_pants(["list"]) + self.assert_success(pants_run) self.assertIn("WARNING: No targets were matched in", pants_run.stderr) - def test_list_invalid_dir(self): - pants_run = self.do_command("list", "abcde::", success=False) + def test_list_invalid_dir(self) -> None: + pants_run = self.run_pants(["list", "abcde::"]) + self.assert_failure(pants_run) self.assertIn("ResolveError", pants_run.stderr) - def test_list_nested_function_scopes(self): - pants_run = self.do_command( - "list", "testprojects/tests/python/pants/build_parsing::", success=True + def test_list_testproject(self) -> None: + pants_run = self.run_pants( + [ + "--backend-packages=pants.backend.python", + "list", + "testprojects/tests/python/pants/build_parsing::", + ] ) + self.assert_success(pants_run) self.assertEqual( pants_run.stdout.strip(), "testprojects/tests/python/pants/build_parsing:test-nested-variable-access-in-function-call", diff --git a/tests/python/pants_test/integration/log_output_integration_test.py b/tests/python/pants_test/integration/log_output_integration_test.py index f81f3a74186..fc506e25f96 100644 --- a/tests/python/pants_test/integration/log_output_integration_test.py +++ b/tests/python/pants_test/integration/log_output_integration_test.py @@ -32,7 +32,13 @@ def test_completed_log_output(self) -> None: tmpdir_relative = self._prepare_sources(tmpdir, build_root) test_run_result = self.run_pants( - ["--no-dynamic-ui", "-ldebug", "typecheck", f"{tmpdir_relative}/src/python/project"] + [ + "--no-dynamic-ui", + "--backend-packages=['pants.backend.python', 'pants.backend.python.typecheck.mypy']", + "-ldebug", + "typecheck", + f"{tmpdir_relative}/src/python/project", + ] ) assert "[DEBUG] Starting: Run MyPy on" in test_run_result.stderr diff --git a/tests/python/pants_test/integration/test_prelude_integration.py b/tests/python/pants_test/integration/test_prelude_integration.py index a547a2eca4d..8aa12ad0d17 100644 --- a/tests/python/pants_test/integration/test_prelude_integration.py +++ b/tests/python/pants_test/integration/test_prelude_integration.py @@ -24,6 +24,7 @@ def make_target(): ), self.temporary_file_content("main.py", python): run = self.run_pants( [ + "--backend-packages=pants.backend.python", "--build-file-prelude-globs=prelude", "--source-root-patterns=['/']", "run", diff --git a/tests/python/pants_test/logging/native_engine_logging_integration_test.py b/tests/python/pants_test/logging/native_engine_logging_integration_test.py index 7ab1487800b..34021c5e7ef 100644 --- a/tests/python/pants_test/logging/native_engine_logging_integration_test.py +++ b/tests/python/pants_test/logging/native_engine_logging_integration_test.py @@ -6,15 +6,6 @@ class NativeEngineLoggingTest(PantsIntegrationTest): - @classmethod - def use_pantsd_env_var(cls): - """Some of the tests here expect to read the standard error after an intentional failure. - - However, when pantsd is enabled, these errors are logged to logs/exceptions..log So - stderr appears empty. (see #7320) - """ - return False - def test_native_logging(self) -> None: expected_msg = r"\[DEBUG\] Launching \d+ root" pants_run = self.run_pants(["-linfo", "list", "3rdparty::"]) @@ -27,7 +18,9 @@ def test_native_logging(self) -> None: class PantsdNativeLoggingTest(PantsDaemonIntegrationTestBase): def test_pantsd_file_logging(self) -> None: with self.pantsd_successful_run_context("debug") as ctx: - daemon_run = ctx.runner(["list", "3rdparty::"]) + daemon_run = ctx.runner( + ["--backend-packages=pants.backend.python", "list", "3rdparty::"] + ) ctx.checker.assert_started() assert "[DEBUG] connecting to pantsd on port" in daemon_run.stderr_data diff --git a/tests/python/pants_test/pantsd/pantsd_integration_test.py b/tests/python/pants_test/pantsd/pantsd_integration_test.py index 1d5d49cc557..5ce315aedcd 100644 --- a/tests/python/pants_test/pantsd/pantsd_integration_test.py +++ b/tests/python/pants_test/pantsd/pantsd_integration_test.py @@ -42,6 +42,8 @@ def join(): class TestPantsDaemonIntegration(PantsDaemonIntegrationTestBase): + hermetic = False + def test_pantsd_run(self): with self.pantsd_successful_run_context(log_level="debug") as ctx: ctx.runner(["list", "3rdparty::"]) @@ -52,7 +54,9 @@ def test_pantsd_run(self): def test_pantsd_broken_pipe(self): with self.pantsd_test_context() as (workdir, pantsd_config, checker): - run = self.run_pants_with_workdir("help | head -1", workdir, pantsd_config, shell=True) + run = self.run_pants_with_workdir( + "help | head -1", workdir=workdir, config=pantsd_config, shell=True + ) self.assertNotIn("broken pipe", run.stderr.lower()) checker.assert_started() @@ -62,14 +66,16 @@ def test_pantsd_pantsd_runner_doesnt_die_after_failed_run(self): self.assert_failure( self.run_pants_with_workdir( ["lint", "testprojects/src/python/unicode/compilation_failure"], - workdir, - pantsd_config, + workdir=workdir, + config=pantsd_config, ) ) checker.assert_started() # Assert pantsd is in a good functional state. - self.assert_success(self.run_pants_with_workdir(["help"], workdir, pantsd_config)) + self.assert_success( + self.run_pants_with_workdir(["help"], workdir=workdir, config=pantsd_config) + ) checker.assert_running() def test_pantsd_lifecycle_invalidation(self): @@ -123,13 +129,15 @@ def test_pantsd_lifecycle_non_invalidation_on_config_string(self): def test_pantsd_lifecycle_shutdown_for_broken_scheduler(self): with self.pantsd_test_context() as (workdir, config, checker): # Run with valid options. - self.assert_success(self.run_pants_with_workdir(["help"], workdir, config)) + self.assert_success( + self.run_pants_with_workdir(["help"], workdir=workdir, config=config) + ) checker.assert_started() # And again with invalid scheduler-fingerprinted options that trigger a re-init. self.assert_failure( self.run_pants_with_workdir( - ["--backend-packages=nonsensical", "help"], workdir, config + ["--backend-packages=nonsensical", "help"], workdir=workdir, config=config ) ) checker.assert_stopped() @@ -201,14 +209,16 @@ def test_pantsd_client_env_var_is_inherited_by_pantsd_runner_children(self): def test_pantsd_launch_env_var_is_not_inherited_by_pantsd_runner_children(self): with self.pantsd_test_context() as (workdir, pantsd_config, checker): with environment_as(NO_LEAKS="33"): - self.assert_success(self.run_pants_with_workdir(["help"], workdir, pantsd_config)) + self.assert_success( + self.run_pants_with_workdir(["help"], workdir=workdir, config=pantsd_config) + ) checker.assert_started() self.assert_failure( self.run_pants_with_workdir( ["run", "testprojects/src/python/print_env", "--", "NO_LEAKS"], - workdir, - pantsd_config, + workdir=workdir, + config=pantsd_config, ) ) checker.assert_running() @@ -415,8 +425,8 @@ def test_pantsd_multiple_parallel_runs(self): file_to_make = os.path.join(workdir, "some_magic_file") waiter_handle = self.run_pants_with_workdir_without_waiting( ["run", "testprojects/src/python/coordinated_runs:waiter", "--", file_to_make], - workdir, - config, + workdir=workdir, + config=config, ) checker.assert_started() @@ -424,8 +434,8 @@ def test_pantsd_multiple_parallel_runs(self): creator_handle = self.run_pants_with_workdir_without_waiting( ["run", "testprojects/src/python/coordinated_runs:creator", "--", file_to_make], - workdir, - config, + workdir=workdir, + config=config, ) self.assert_success(creator_handle.join()) @@ -456,7 +466,9 @@ def _assert_pantsd_keyboardinterrupt_signal(self, signum, regexps=[], quit_timeo "--", file_to_make, ] - waiter_handle = self.run_pants_with_workdir_without_waiting(argv, workdir, config) + waiter_handle = self.run_pants_with_workdir_without_waiting( + argv, workdir=workdir, config=config + ) client_pid = waiter_handle.process.pid checker.assert_started() diff --git a/tests/python/pants_test/pantsd/pantsd_integration_test_base.py b/tests/python/pants_test/pantsd/pantsd_integration_test_base.py index f53ab8370d7..ac97223bfeb 100644 --- a/tests/python/pants_test/pantsd/pantsd_integration_test_base.py +++ b/tests/python/pants_test/pantsd/pantsd_integration_test_base.py @@ -95,10 +95,7 @@ class PantsdRunContext: class PantsDaemonIntegrationTestBase(PantsIntegrationTest): - @classmethod - def use_pantsd_env_var(cls): - """We set our own ad-hoc pantsd configuration in most of these tests.""" - return False + use_pantsd = False # We set our own ad-hoc pantsd configuration in most of these tests. @contextmanager def pantsd_test_context( @@ -140,7 +137,6 @@ def pantsd_run_context( extra_config: Optional[Dict[str, Any]] = None, extra_env: Optional[Dict[str, str]] = None, success: bool = True, - no_track_run_counts: bool = False, ) -> Iterator[PantsdRunContext]: with self.pantsd_test_context(log_level=log_level, extra_config=extra_config) as ( workdir, @@ -185,12 +181,7 @@ def assert_runner( run_count = self._run_count(workdir) start_time = time.time() run = self.run_pants_with_workdir( - cmd, - workdir, - combined_config, - extra_env=extra_env, - # TODO: With this uncommented, `test_pantsd_run` fails. - # tee_output=True + cmd, workdir=workdir, config=combined_config, extra_env=extra_env ) elapsed = time.time() - start_time print(bold(cyan(f"\ncompleted in {elapsed} seconds")))