diff --git a/src/ape_plugins/_cli.py b/src/ape_plugins/_cli.py index ba70caead7..fc45854f24 100644 --- a/src/ape_plugins/_cli.py +++ b/src/ape_plugins/_cli.py @@ -170,7 +170,7 @@ def install(cli_ctx, plugins, skip_confirmation, upgrade): else: cli_ctx.logger.warning( - f"'{plugin.name}' is already installed. " f"Did you mean to include '--upgrade'." + f"'{plugin.name}' is already installed. Did you mean to include '--upgrade'?" ) if failures_occurred: diff --git a/src/ape_plugins/utils.py b/src/ape_plugins/utils.py index 46b4306acf..83e68ce12c 100644 --- a/src/ape_plugins/utils.py +++ b/src/ape_plugins/utils.py @@ -38,6 +38,11 @@ class PluginType(Enum): """ +def _check_pip_freeze() -> str: + result = subprocess.check_output([sys.executable, "-m", "pip", "freeze"]) + return result.decode("utf8") + + class _PipFreeze: cache: Optional[Set[str]] = None @@ -45,25 +50,27 @@ def get_plugins(self, use_cache: bool = True) -> Set[str]: if use_cache and self.cache is not None: return self.cache - output = subprocess.check_output([sys.executable, "-m", "pip", "freeze"]) + output = _check_pip_freeze() lines = [ p - for p in output.decode().splitlines() + for p in output.splitlines() if p.startswith("ape-") or (p.startswith("-e") and "ape-" in p) ] - new_lines = [] + # NOTE: Package IDs should look like "name==version" + # if version is available. + package_ids = [] for package in lines: if "-e" in package: - new_lines.append(package.split(".git")[0].split("/")[-1]) + package_ids.append(package.split(".git")[0].split("/")[-1]) elif "@" in package: - new_lines.append(package.split("@")[0].strip()) + package_ids.append(package.split("@")[0].strip()) elif "==" in package: - new_lines.append(package.split("==")[0].strip()) + package_ids.append(package) else: - new_lines.append(package) + package_ids.append(package) - self.cache = {x for x in new_lines} + self.cache = set(x for x in package_ids) return self.cache @@ -296,7 +303,7 @@ class ModifyPluginResultHandler: def __init__(self, plugin: PluginMetadata): self._plugin = plugin - def handle_install_result(self, result) -> bool: + def handle_install_result(self, result: int) -> bool: if not self._plugin.check_installed(use_cache=False): self._log_modify_failed("install") return False @@ -318,21 +325,23 @@ def handle_upgrade_result(self, result, version_before: str) -> bool: self._log_errors_occurred("upgrading") return False - pip_freeze_version = self._plugin.pip_freeze_version - if version_before == pip_freeze_version or not pip_freeze_version: - if self._plugin.version: - logger.info(f"'{self._plugin.name}' already has version '{self._plugin.version}'.") - else: - logger.info(f"'{self._plugin.name}' already up to date.") - + version_now = self._plugin.pip_freeze_version + if version_now is not None and version_before == version_now: + logger.info(f"'{self._plugin.name}' already has version '{version_now}'.") return True - else: + + elif self._plugin.pip_freeze_version: logger.success( f"Plugin '{self._plugin.name}' has been " f"upgraded to version {self._plugin.pip_freeze_version}." ) return True + else: + # The process was successful but there is still no pip freeze version. + # This may happen when installing things from GitHub. + return True + def handle_uninstall_result(self, result) -> bool: if self._plugin.check_installed(use_cache=False): self._log_modify_failed("uninstall") diff --git a/tests/functional/test_plugins.py b/tests/functional/test_plugins.py index 61fba0f209..09319dcf61 100644 --- a/tests/functional/test_plugins.py +++ b/tests/functional/test_plugins.py @@ -5,30 +5,53 @@ from ape_plugins.utils import ( ApePluginsRepr, + ModifyPluginResultHandler, PluginGroup, PluginMetadata, PluginMetadataList, PluginType, + _PipFreeze, ) CORE_PLUGINS = ("run",) AVAILABLE_PLUGINS = ("available", "installed") INSTALLED_PLUGINS = ("installed", "thirdparty") THIRD_PARTY = ("thirdparty",) -VERSION = "0.6.2" +VERSION = "0.7.0" + + +def get_pip_freeze_output(version: str): + return f"FOOFOO==1.1.1\n-e git+ssh://git@github.com/ApeWorX/ape-{INSTALLED_PLUGINS[0]}.git@aaaaaaaabbbbbb3343#egg=ape-{INSTALLED_PLUGINS[0]}\naiohttp==3.8.5\nape-{THIRD_PARTY[0]}=={version}\n" # noqa: E501 + + +@pytest.fixture(autouse=True) +def mock_pip_freeze(mocker): + def fn(version: str): + patch = mocker.patch("ape_plugins.utils._check_pip_freeze") + patch.return_value = get_pip_freeze_output(version) + return patch + + return fn @pytest.fixture(autouse=True) -def plugin_test_env(mocker): +def plugin_test_env(mocker, mock_pip_freeze): root = "ape_plugins.utils" # Prevent calling out to GitHub gh_mock = mocker.patch(f"{root}._get_available_plugins") gh_mock.return_value = {f"ape_{x}" for x in AVAILABLE_PLUGINS} + # Used when testing PipFreeze object itself but also extra avoids + # actually calling out pip ever in tests. + mock_pip_freeze(VERSION) + # Prevent requiring plugins to be installed. installed_mock = mocker.patch(f"{root}._pip_freeze_plugins") - installed_mock.return_value = {f"ape-{x}" for x in INSTALLED_PLUGINS} + installed_mock.return_value = { + f"ape-{INSTALLED_PLUGINS[0]}", + f"ape-{INSTALLED_PLUGINS[1]}=={VERSION}", + } # Prevent version lookups. version_mock = mocker.patch(f"{root}.get_package_version") @@ -72,11 +95,11 @@ def test_names(self, name): def test_model_validator_when_version_included_with_name(self): # This allows parsing requirements files easier - metadata = PluginMetadata(name="ape-foo-bar==0.5.0") + metadata = PluginMetadata(name="ape-foo-bar==0.7.0") assert metadata.name == "foo-bar" - assert metadata.version == "==0.5.0" + assert metadata.version == "==0.7.0" - @pytest.mark.parametrize("version", ("0.5.0", "v0.5.0", "0.5.0a123")) + @pytest.mark.parametrize("version", ("0.7.0", "v0.7.0", "0.7.0a123")) def test_version(self, version): metadata = PluginMetadata(name="foo", version=version) assert metadata.version == version @@ -87,19 +110,19 @@ def test_install_str_without_version(self): assert actual == "ape-foo-bar" def test_install_str_with_version(self): - metadata = PluginMetadata(name="foo-bar", version="0.5.0") + metadata = PluginMetadata(name="foo-bar", version="0.7.0") actual = metadata.install_str - assert actual == "ape-foo-bar==0.5.0" + assert actual == "ape-foo-bar==0.7.0" def test_install_str_with_complex_constraint(self): - metadata = PluginMetadata(name="foo", version=">=0.5.0,<0.6.0") + metadata = PluginMetadata(name="foo", version=">=0.7.0,<0.8.0") actual = metadata.install_str - assert actual == "ape-foo>=0.5.0,<0.6.0" + assert actual == "ape-foo>=0.7.0,<0.8.0" def test_install_str_with_complex_constraint_in_name(self): - metadata = PluginMetadata(name="foo>=0.5.0,<0.6.0") + metadata = PluginMetadata(name="foo>=0.7.0,<0.8.0") actual = metadata.install_str - assert actual == "ape-foo>=0.5.0,<0.6.0" + assert actual == "ape-foo>=0.7.0,<0.8.0" def test_install_str_when_using_git_remote(self): url = "git+https://example.com/ape-foo/branch" @@ -185,3 +208,39 @@ def test_repr_when_exception(self, mocker): group = PluginGroup(plugin_type=PluginType.INSTALLED) assert repr(group) == "" + + +def test_pip_freeze_includes_version_when_available(): + pip_freeze = _PipFreeze() + actual = pip_freeze.get_plugins() + expected = {f"ape-{INSTALLED_PLUGINS[0]}", f"ape-{THIRD_PARTY[0]}==0.7.0"} + assert actual == expected + + +def test_handle_upgrade_result_when_upgrading_to_same_version(caplog, logger): + # NOTE: pip freeze mock also returns version 0.7.0 (so upgrade to same). + logger.set_level("INFO") # Required for test. + plugin = PluginMetadata(name=THIRD_PARTY[0]) + handler = ModifyPluginResultHandler(plugin) + handler.handle_upgrade_result(0, "0.7.0") + if records := caplog.records: + assert f"'{THIRD_PARTY[0]}' already has version '0.7.0'" in records[-1].message + else: + version_at_end = plugin.pip_freeze_version + pytest.fail( + "Missing logs when upgrading to same version 0.7.0. " + f"pip_freeze_version={version_at_end}" + ) + + +def test_handle_upgrade_result_when_no_pip_freeze_version_does_not_log(caplog): + plugin_no_version = INSTALLED_PLUGINS[0] # Version not in pip-freeze + plugin = PluginMetadata(name=plugin_no_version) + handler = ModifyPluginResultHandler(plugin) + handler.handle_upgrade_result(0, "0.7.0") + + log_parts = ("already has version", "already up to date") + messages = [x.message for x in caplog.records] + for message in messages: + for pt in log_parts: + assert pt not in message