From 4845c123cdb2783138b630a484c52f9003cf213e Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Tue, 8 Oct 2024 20:45:34 -0400 Subject: [PATCH 01/16] feat(linter): add 'pip check' linter This linter runs 'pip check' on the venv of the resulting charm and warns if the virtual environment is inconsistent. --- charmcraft/linters.py | 47 +++++++++++++++++++++++- pyproject.toml | 1 + tests/integration/test_linters.py | 60 +++++++++++++++++++++++++++++++ tests/unit/test_linters.py | 54 ++++++++++++++++++++++++++++ 4 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 tests/integration/test_linters.py create mode 100644 tests/unit/test_linters.py diff --git a/charmcraft/linters.py b/charmcraft/linters.py index edcccd2e6..5c4a35a59 100644 --- a/charmcraft/linters.py +++ b/charmcraft/linters.py @@ -1,4 +1,4 @@ -# Copyright 2021-2022 Canonical Ltd. +# Copyright 2021-2024 Canonical Ltd. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -21,6 +21,8 @@ import pathlib import re import shlex +import subprocess +import sys import typing from collections.abc import Generator from typing import final @@ -668,6 +670,49 @@ def run(self, basedir: pathlib.Path) -> str: return self._check_additional_files(stage_dir, basedir) +class PipCheck(Linter): + """Check that the pip virtual environment is valid.""" + + name = "pip-check" + text = "Virtual environment is valid." + url = "https://pip.pypa.io/en/stable/cli/pip_check/" + + def run(self, basedir: pathlib.Path) -> str: + """Run pip check.""" + venv_dir = basedir / "venv" + if not venv_dir.is_dir(): + self.text = "Charm does not contain a Python venv." + return self.Result.NONAPPLICABLE + python_exe = venv_dir / "bin" / "python" + delete_parent = False + if not python_exe.parent.exists(): + delete_parent = True + python_exe.parent.mkdir() + if not python_exe.exists(): + delete_python_exe = True + python_exe.symlink_to(sys.executable) + else: + delete_python_exe = False + + pip_cmd = [sys.executable, "-m", "pip", "--python", str(python_exe), "check"] + check = subprocess.run( + pip_cmd, + text=True, + capture_output=True, + ) + if check.returncode == 0: + result = self.Result.OK + else: + self.text = check.stdout + result = self.Result.WARNING + if delete_python_exe: + python_exe.unlink() + if delete_parent: + python_exe.parent.rmdir() + + return result + + # all checkers to run; the order here is important, as some checkers depend on the # results from others CHECKERS: list[type[BaseChecker]] = [ diff --git a/pyproject.toml b/pyproject.toml index 179746404..8cc132284 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,6 +29,7 @@ dependencies = [ # https://github.com/msabramo/requests-unixsocket/pull/69 # When updating, remove the urllib3 constraint from renovate config. "urllib3<2.0", + "pip>=24.2", ] classifiers = [ "Development Status :: 5 - Production/Stable", diff --git a/tests/integration/test_linters.py b/tests/integration/test_linters.py new file mode 100644 index 000000000..376e7b25e --- /dev/null +++ b/tests/integration/test_linters.py @@ -0,0 +1,60 @@ +# Copyright 2024 Canonical Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# For further info, check https://github.com/canonical/charmcraft +"""Unit tests for linters.""" + +import pathlib +import subprocess +import sys + +import pytest + +from charmcraft import linters +from charmcraft.models.lint import LintResult + +pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="Windows not supported") + + +@pytest.mark.parametrize( + "pip_cmd", + [ + ["--version"], + ["install", "pytest", "hypothesis"], + ] +) +def test_pip_check_success(tmp_path: pathlib.Path, pip_cmd: list[str]): + venv_path = tmp_path / "venv" + subprocess.run([sys.executable, "-m", "venv", venv_path], check=True) + subprocess.run([venv_path / "bin" / "python", "-m", "pip", *pip_cmd], check=True) + + lint = linters.PipCheck() + assert lint.run(tmp_path) == LintResult.OK + assert lint.text == linters.PipCheck.text + + +@pytest.mark.parametrize( + "pip_cmd", + [ + ["install", "--no-deps", "pydantic==2.9.2"], + ] +) +def test_pip_check_failure(tmp_path: pathlib.Path, pip_cmd: list[str]): + venv_path = tmp_path / "venv" + subprocess.run([sys.executable, "-m", "venv", venv_path], check=True) + subprocess.run([venv_path / "bin" / "python", "-m", "pip", *pip_cmd], check=True) + + lint = linters.PipCheck() + assert lint.run(tmp_path) == LintResult.WARNING + assert "pydantic 2.9.2 requires pydantic-core" in lint.text diff --git a/tests/unit/test_linters.py b/tests/unit/test_linters.py new file mode 100644 index 000000000..1977d6c5d --- /dev/null +++ b/tests/unit/test_linters.py @@ -0,0 +1,54 @@ +# Copyright 2024 Canonical Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# For further info, check https://github.com/canonical/charmcraft +"""Unit tests for linters.""" + +import pathlib +import subprocess +import sys +from charmcraft import linters +from charmcraft.models.lint import LintResult + + +def test_pip_check_not_venv(fake_path: pathlib.Path): + lint = linters.PipCheck() + assert lint.run(fake_path) == LintResult.NONAPPLICABLE + assert lint.text == "Charm does not contain a Python venv." + + +def test_pip_check_success(fake_path: pathlib.Path, fp): + (fake_path / "venv").mkdir() + fp.register( + [sys.executable, "-m", "pip", "--python", fp.any(), "check"], + returncode=0, + stdout="Loo loo loo, doing pip stuff. Pip stuff is my favourite stuff." + ) + + lint = linters.PipCheck() + assert lint.run(fake_path) == LintResult.OK + assert lint.text == linters.PipCheck.text + + +def test_pip_check_warning(fake_path: pathlib.Path, fp): + (fake_path / "venv").mkdir() + fp.register( + [sys.executable, "-m", "pip", "--python", fp.any(), "check"], + returncode=1, + stdout="This error was sponsored by Raytheon Knife Missiles™" + ) + + lint = linters.PipCheck() + assert lint.run(fake_path) == LintResult.WARNING + assert lint.text == "This error was sponsored by Raytheon Knife Missiles™" From 05f2585f4898295fd8bdbc08f1c8f24889ce88c1 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Thu, 10 Oct 2024 15:44:55 -0400 Subject: [PATCH 02/16] chore: fix ruff warning on subprocess call --- charmcraft/linters.py | 1 + 1 file changed, 1 insertion(+) diff --git a/charmcraft/linters.py b/charmcraft/linters.py index 5c4a35a59..bf9172622 100644 --- a/charmcraft/linters.py +++ b/charmcraft/linters.py @@ -699,6 +699,7 @@ def run(self, basedir: pathlib.Path) -> str: pip_cmd, text=True, capture_output=True, + check=False, ) if check.returncode == 0: result = self.Result.OK From 24d08778cc939c441a7ae715df598746f34c2112 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Thu, 10 Oct 2024 15:45:41 -0400 Subject: [PATCH 03/16] feat: add pip-check to the checkers to be ran --- charmcraft/linters.py | 1 + 1 file changed, 1 insertion(+) diff --git a/charmcraft/linters.py b/charmcraft/linters.py index bf9172622..0ca7e1216 100644 --- a/charmcraft/linters.py +++ b/charmcraft/linters.py @@ -726,4 +726,5 @@ def run(self, basedir: pathlib.Path) -> str: Entrypoint, OpsMainCall, AdditionalFiles, + PipCheck, ] From b48d0e6cc7c8e0dae7ea397076b02e4d058df6ef Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Fri, 11 Oct 2024 09:33:28 -0400 Subject: [PATCH 04/16] chore: autofix remaining ruff warnings --- tests/unit/test_linters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_linters.py b/tests/unit/test_linters.py index 1977d6c5d..719ae6117 100644 --- a/tests/unit/test_linters.py +++ b/tests/unit/test_linters.py @@ -16,8 +16,8 @@ """Unit tests for linters.""" import pathlib -import subprocess import sys + from charmcraft import linters from charmcraft.models.lint import LintResult From 169224651da42ad97dd503ac47a2c082947c9cdd Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Fri, 11 Oct 2024 09:33:51 -0400 Subject: [PATCH 05/16] style: format with black --- tests/integration/test_linters.py | 4 ++-- tests/unit/test_linters.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/test_linters.py b/tests/integration/test_linters.py index 376e7b25e..10a86ca8e 100644 --- a/tests/integration/test_linters.py +++ b/tests/integration/test_linters.py @@ -32,7 +32,7 @@ [ ["--version"], ["install", "pytest", "hypothesis"], - ] + ], ) def test_pip_check_success(tmp_path: pathlib.Path, pip_cmd: list[str]): venv_path = tmp_path / "venv" @@ -48,7 +48,7 @@ def test_pip_check_success(tmp_path: pathlib.Path, pip_cmd: list[str]): "pip_cmd", [ ["install", "--no-deps", "pydantic==2.9.2"], - ] + ], ) def test_pip_check_failure(tmp_path: pathlib.Path, pip_cmd: list[str]): venv_path = tmp_path / "venv" diff --git a/tests/unit/test_linters.py b/tests/unit/test_linters.py index 719ae6117..3bb263e5c 100644 --- a/tests/unit/test_linters.py +++ b/tests/unit/test_linters.py @@ -33,7 +33,7 @@ def test_pip_check_success(fake_path: pathlib.Path, fp): fp.register( [sys.executable, "-m", "pip", "--python", fp.any(), "check"], returncode=0, - stdout="Loo loo loo, doing pip stuff. Pip stuff is my favourite stuff." + stdout="Loo loo loo, doing pip stuff. Pip stuff is my favourite stuff.", ) lint = linters.PipCheck() @@ -46,7 +46,7 @@ def test_pip_check_warning(fake_path: pathlib.Path, fp): fp.register( [sys.executable, "-m", "pip", "--python", fp.any(), "check"], returncode=1, - stdout="This error was sponsored by Raytheon Knife Missiles™" + stdout="This error was sponsored by Raytheon Knife Missiles™", ) lint = linters.PipCheck() From b7fc57b8cec00d4ab5d2ab731e809a558a7681a4 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Fri, 11 Oct 2024 10:37:41 -0400 Subject: [PATCH 06/16] fix: skip pip check linter on windows --- charmcraft/linters.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/charmcraft/linters.py b/charmcraft/linters.py index 0ca7e1216..9f47c5452 100644 --- a/charmcraft/linters.py +++ b/charmcraft/linters.py @@ -679,6 +679,9 @@ class PipCheck(Linter): def run(self, basedir: pathlib.Path) -> str: """Run pip check.""" + if sys.platform == "win32": + self.text = "Linter does not work on Windows." + return self.Result.NONAPPLICABLE venv_dir = basedir / "venv" if not venv_dir.is_dir(): self.text = "Charm does not contain a Python venv." From 7bbaa57258c8244cda88642d54d2931129a35986 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Fri, 11 Oct 2024 10:55:34 -0400 Subject: [PATCH 07/16] fix: rearrange internal checks for pip check --- charmcraft/linters.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/charmcraft/linters.py b/charmcraft/linters.py index 9f47c5452..df5f2f8ac 100644 --- a/charmcraft/linters.py +++ b/charmcraft/linters.py @@ -679,13 +679,13 @@ class PipCheck(Linter): def run(self, basedir: pathlib.Path) -> str: """Run pip check.""" - if sys.platform == "win32": - self.text = "Linter does not work on Windows." - return self.Result.NONAPPLICABLE venv_dir = basedir / "venv" if not venv_dir.is_dir(): self.text = "Charm does not contain a Python venv." return self.Result.NONAPPLICABLE + if sys.platform == "win32": + self.text = "Linter does not work on Windows." + return self.Result.NONAPPLICABLE python_exe = venv_dir / "bin" / "python" delete_parent = False if not python_exe.parent.exists(): From 3420e24637cfe810fddf977a0229083a4466ea39 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Fri, 11 Oct 2024 11:10:23 -0400 Subject: [PATCH 08/16] fix(linter): skip pip check linter on windows platforms --- charmcraft/linters.py | 3 --- tests/integration/test_linters.py | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/charmcraft/linters.py b/charmcraft/linters.py index df5f2f8ac..0ca7e1216 100644 --- a/charmcraft/linters.py +++ b/charmcraft/linters.py @@ -683,9 +683,6 @@ def run(self, basedir: pathlib.Path) -> str: if not venv_dir.is_dir(): self.text = "Charm does not contain a Python venv." return self.Result.NONAPPLICABLE - if sys.platform == "win32": - self.text = "Linter does not work on Windows." - return self.Result.NONAPPLICABLE python_exe = venv_dir / "bin" / "python" delete_parent = False if not python_exe.parent.exists(): diff --git a/tests/integration/test_linters.py b/tests/integration/test_linters.py index 10a86ca8e..c4d6de492 100644 --- a/tests/integration/test_linters.py +++ b/tests/integration/test_linters.py @@ -34,6 +34,7 @@ ["install", "pytest", "hypothesis"], ], ) +@pytest.mark.skipif(sys.platform == "win32", reason = "Windows not [yet] supported") def test_pip_check_success(tmp_path: pathlib.Path, pip_cmd: list[str]): venv_path = tmp_path / "venv" subprocess.run([sys.executable, "-m", "venv", venv_path], check=True) From 1c21ba065b455c4db087886fd160e6d795409e5b Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Fri, 11 Oct 2024 11:14:54 -0400 Subject: [PATCH 09/16] fix(linter): apply nonapplicable to pip check on Windows --- charmcraft/linters.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/charmcraft/linters.py b/charmcraft/linters.py index 0ca7e1216..df5f2f8ac 100644 --- a/charmcraft/linters.py +++ b/charmcraft/linters.py @@ -683,6 +683,9 @@ def run(self, basedir: pathlib.Path) -> str: if not venv_dir.is_dir(): self.text = "Charm does not contain a Python venv." return self.Result.NONAPPLICABLE + if sys.platform == "win32": + self.text = "Linter does not work on Windows." + return self.Result.NONAPPLICABLE python_exe = venv_dir / "bin" / "python" delete_parent = False if not python_exe.parent.exists(): From fbb9b4f2c6d5f5a1c39be58850100413f2ef094b Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Fri, 11 Oct 2024 11:21:25 -0400 Subject: [PATCH 10/16] fix(linter): skip more pip check tests --- tests/integration/test_linters.py | 2 +- tests/unit/test_linters.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_linters.py b/tests/integration/test_linters.py index c4d6de492..97645f3fc 100644 --- a/tests/integration/test_linters.py +++ b/tests/integration/test_linters.py @@ -34,7 +34,7 @@ ["install", "pytest", "hypothesis"], ], ) -@pytest.mark.skipif(sys.platform == "win32", reason = "Windows not [yet] supported") +@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported") def test_pip_check_success(tmp_path: pathlib.Path, pip_cmd: list[str]): venv_path = tmp_path / "venv" subprocess.run([sys.executable, "-m", "venv", venv_path], check=True) diff --git a/tests/unit/test_linters.py b/tests/unit/test_linters.py index 3bb263e5c..0651bb3a2 100644 --- a/tests/unit/test_linters.py +++ b/tests/unit/test_linters.py @@ -18,6 +18,8 @@ import pathlib import sys +import pytest + from charmcraft import linters from charmcraft.models.lint import LintResult @@ -28,6 +30,7 @@ def test_pip_check_not_venv(fake_path: pathlib.Path): assert lint.text == "Charm does not contain a Python venv." +@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.") def test_pip_check_success(fake_path: pathlib.Path, fp): (fake_path / "venv").mkdir() fp.register( @@ -41,6 +44,7 @@ def test_pip_check_success(fake_path: pathlib.Path, fp): assert lint.text == linters.PipCheck.text +@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.") def test_pip_check_warning(fake_path: pathlib.Path, fp): (fake_path / "venv").mkdir() fp.register( From ff002a359cacf350e9941c23ca765d855425e9e0 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Tue, 15 Oct 2024 14:14:29 -0400 Subject: [PATCH 11/16] fix(linter): skip pip check linter if charm does not have a valid venv to test --- charmcraft/linters.py | 3 +++ requirements-dev.txt | 1 + requirements.txt | 1 + tests/unit/test_linters.py | 11 +++++++++-- 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/charmcraft/linters.py b/charmcraft/linters.py index df5f2f8ac..20b5ae6f0 100644 --- a/charmcraft/linters.py +++ b/charmcraft/linters.py @@ -683,6 +683,9 @@ def run(self, basedir: pathlib.Path) -> str: if not venv_dir.is_dir(): self.text = "Charm does not contain a Python venv." return self.Result.NONAPPLICABLE + if not (venv_dir / "lib").is_dir(): + self.text = "Python venv is not valid." + return self.Result.NONAPPLICABLE if sys.platform == "win32": self.text = "Linter does not work on Windows." return self.Result.NONAPPLICABLE diff --git a/requirements-dev.txt b/requirements-dev.txt index d06c6ec8f..c6c8985ca 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -44,6 +44,7 @@ more-itertools==10.5.0 oauthlib==3.2.2 overrides==7.7.0 packaging==24.1 +pip==24.2 platformdirs==4.3.6 pluggy==1.5.0 protobuf==5.28.2 diff --git a/requirements.txt b/requirements.txt index 61b3b162b..d8d17b2a7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -35,6 +35,7 @@ more-itertools==10.5.0 oauthlib==3.2.2 overrides==7.7.0 packaging==24.1 +pip==24.2 platformdirs==4.3.6 protobuf==5.28.2 pycparser==2.22 diff --git a/tests/unit/test_linters.py b/tests/unit/test_linters.py index 0651bb3a2..c87c819fe 100644 --- a/tests/unit/test_linters.py +++ b/tests/unit/test_linters.py @@ -30,9 +30,16 @@ def test_pip_check_not_venv(fake_path: pathlib.Path): assert lint.text == "Charm does not contain a Python venv." +def test_pip_invalid_venv(fake_path: pathlib.Path): + (fake_path / "venv").mkdir() + lint = linters.PipCheck() + assert lint.run(fake_path) == LintResult.NONAPPLICABLE + assert lint.text == "Python venv is not valid." + + @pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.") def test_pip_check_success(fake_path: pathlib.Path, fp): - (fake_path / "venv").mkdir() + (fake_path / "venv" / "lib").mkdir(parents=True) fp.register( [sys.executable, "-m", "pip", "--python", fp.any(), "check"], returncode=0, @@ -46,7 +53,7 @@ def test_pip_check_success(fake_path: pathlib.Path, fp): @pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.") def test_pip_check_warning(fake_path: pathlib.Path, fp): - (fake_path / "venv").mkdir() + (fake_path / "venv" / "lib").mkdir(parents=True) fp.register( [sys.executable, "-m", "pip", "--python", fp.any(), "check"], returncode=1, From d01a57a9bf4b2c0d859f19b812af180fbf3b7117 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Wed, 16 Oct 2024 10:42:34 -0400 Subject: [PATCH 12/16] chore: PR suggestion --- pyproject.toml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 0a9e88760..c4c0ab467 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,10 +24,6 @@ dependencies = [ "requests-toolbelt", "snap-helpers", "tabulate", - # Needed until requests-unixsocket supports urllib3 v2 - # https://github.com/msabramo/requests-unixsocket/pull/69 - # When updating, remove the urllib3 constraint from renovate config. - "urllib3<2.0", "pip>=24.2", ] classifiers = [ From 4423d2df4709a2e6af5f8d45337bc009de6c804d Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Thu, 17 Oct 2024 14:37:22 -0400 Subject: [PATCH 13/16] chore: PR feedback --- charmcraft/linters.py | 35 ++++++++++-------- tests/unit/test_linters.py | 73 ++++++++++++++++++++++++++++++++++---- 2 files changed, 87 insertions(+), 21 deletions(-) diff --git a/charmcraft/linters.py b/charmcraft/linters.py index 575e86763..e1a09cad3 100644 --- a/charmcraft/linters.py +++ b/charmcraft/linters.py @@ -722,21 +722,26 @@ def run(self, basedir: pathlib.Path) -> str: delete_python_exe = False pip_cmd = [sys.executable, "-m", "pip", "--python", str(python_exe), "check"] - check = subprocess.run( - pip_cmd, - text=True, - capture_output=True, - check=False, - ) - if check.returncode == 0: - result = self.Result.OK - else: - self.text = check.stdout - result = self.Result.WARNING - if delete_python_exe: - python_exe.unlink() - if delete_parent: - python_exe.parent.rmdir() + try: + check = subprocess.run( + pip_cmd, + text=True, + capture_output=True, + check=False, + ) + if check.returncode == 0: + result = self.Result.OK + else: + self.text = check.stdout + result = self.Result.WARNING + except (FileNotFoundError, PermissionError) as e: + self.text = f"{e.strerror}: Could not run Python executable at {sys.executable}." + result = self.Result.NONAPPLICABLE + finally: + if delete_python_exe: + python_exe.unlink() + if delete_parent: + python_exe.parent.rmdir() return result diff --git a/tests/unit/test_linters.py b/tests/unit/test_linters.py index c87c819fe..26f53ca84 100644 --- a/tests/unit/test_linters.py +++ b/tests/unit/test_linters.py @@ -17,12 +17,19 @@ import pathlib import sys +import subprocess import pytest from charmcraft import linters from charmcraft.models.lint import LintResult +@pytest.fixture +def valid_venv_path(fake_path) -> pathlib.Path: + """Create and return a fakefs path that contains a valid venv structure""" + (fake_path / "venv" / "lib").mkdir(parents=True) + return fake_path + def test_pip_check_not_venv(fake_path: pathlib.Path): lint = linters.PipCheck() @@ -38,8 +45,7 @@ def test_pip_invalid_venv(fake_path: pathlib.Path): @pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.") -def test_pip_check_success(fake_path: pathlib.Path, fp): - (fake_path / "venv" / "lib").mkdir(parents=True) +def test_pip_check_success(valid_venv_path: pathlib.Path, fp): fp.register( [sys.executable, "-m", "pip", "--python", fp.any(), "check"], returncode=0, @@ -47,13 +53,12 @@ def test_pip_check_success(fake_path: pathlib.Path, fp): ) lint = linters.PipCheck() - assert lint.run(fake_path) == LintResult.OK + assert lint.run(valid_venv_path) == LintResult.OK assert lint.text == linters.PipCheck.text @pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.") -def test_pip_check_warning(fake_path: pathlib.Path, fp): - (fake_path / "venv" / "lib").mkdir(parents=True) +def test_pip_check_warning(valid_venv_path: pathlib.Path, fp): fp.register( [sys.executable, "-m", "pip", "--python", fp.any(), "check"], returncode=1, @@ -61,5 +66,61 @@ def test_pip_check_warning(fake_path: pathlib.Path, fp): ) lint = linters.PipCheck() - assert lint.run(fake_path) == LintResult.WARNING + assert lint.run(valid_venv_path) == LintResult.WARNING assert lint.text == "This error was sponsored by Raytheon Knife Missiles™" + +def test_pip_check_exception(valid_venv_path: pathlib.Path, monkeypatch): + def _raises_eperm(*args, **kwargs) -> None: + raise PermissionError(13, "Permission denied") + + monkeypatch.setattr(subprocess, "run", _raises_eperm) + + lint = linters.PipCheck() + assert lint.run(valid_venv_path) == LintResult.NONAPPLICABLE + assert lint.text == f"Permission denied: Could not run Python executable at {sys.executable}." + +def test_pip_check_repair_no_bin(valid_venv_path: pathlib.Path, fp): + """Check that the bin directory is deleted if it was missing before""" + fp.register( + [sys.executable, "-m", "pip", "--python", fp.any(), "check"], + returncode = 0, + stdout = "Gosh, I sure hope I remember where everything went." + ) + lint = linters.PipCheck() + + # Make sure it doesn't leave behind "bin" if it didn't exist + assert lint.run(valid_venv_path) == LintResult.OK + assert lint.text == "Virtual environment is valid." + assert not (valid_venv_path / "venv" / "bin").exists() + +def test_pip_check_repair_no_py(valid_venv_path: pathlib.Path, fp): + """Check that the python symlink is deleted if it was missing before""" + fp.register( + [sys.executable, "-m", "pip", "--python", fp.any(), "check"], + returncode = 0, + stdout = "Gosh, I sure hope I remember where everything went." + ) + lint = linters.PipCheck() + + # Make sure it keeps "bin" if only the Python binary didn't exist + (valid_venv_path / "venv" / "bin").mkdir() + assert lint.run(valid_venv_path) == LintResult.OK + assert lint.text == "Virtual environment is valid." + assert (valid_venv_path / "venv" / "bin").exists() + assert not (valid_venv_path / "venv" / "bin" / "python").exists() + +def test_pip_check_repair_all(valid_venv_path: pathlib.Path, fp): + """Check that nothing is changed if all components are present""" + fp.register( + [sys.executable, "-m", "pip", "--python", fp.any(), "check"], + returncode = 0, + stdout = "Gosh, I sure hope I remember where everything went." + ) + lint = linters.PipCheck() + + (valid_venv_path / "venv" / "bin").mkdir() + (valid_venv_path / "venv" / "bin" / "python").touch() + + assert lint.run(valid_venv_path) == LintResult.OK + assert lint.text == "Virtual environment is valid." + assert (valid_venv_path / "venv" / "bin" / "python").is_file() \ No newline at end of file From 145428b00f26c9348b794f41f1a2f1ff5b8bf354 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Thu, 17 Oct 2024 15:03:25 -0400 Subject: [PATCH 14/16] fix(linter): skip pip check tests on windows --- charmcraft/linters.py | 4 +++- tests/unit/test_linters.py | 34 +++++++++++++++++++++++----------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/charmcraft/linters.py b/charmcraft/linters.py index e1a09cad3..261e1a614 100644 --- a/charmcraft/linters.py +++ b/charmcraft/linters.py @@ -735,7 +735,9 @@ def run(self, basedir: pathlib.Path) -> str: self.text = check.stdout result = self.Result.WARNING except (FileNotFoundError, PermissionError) as e: - self.text = f"{e.strerror}: Could not run Python executable at {sys.executable}." + self.text = ( + f"{e.strerror}: Could not run Python executable at {sys.executable}." + ) result = self.Result.NONAPPLICABLE finally: if delete_python_exe: diff --git a/tests/unit/test_linters.py b/tests/unit/test_linters.py index 26f53ca84..ca1e72073 100644 --- a/tests/unit/test_linters.py +++ b/tests/unit/test_linters.py @@ -16,14 +16,15 @@ """Unit tests for linters.""" import pathlib -import sys import subprocess +import sys import pytest from charmcraft import linters from charmcraft.models.lint import LintResult + @pytest.fixture def valid_venv_path(fake_path) -> pathlib.Path: """Create and return a fakefs path that contains a valid venv structure""" @@ -69,6 +70,8 @@ def test_pip_check_warning(valid_venv_path: pathlib.Path, fp): assert lint.run(valid_venv_path) == LintResult.WARNING assert lint.text == "This error was sponsored by Raytheon Knife Missiles™" + +@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.") def test_pip_check_exception(valid_venv_path: pathlib.Path, monkeypatch): def _raises_eperm(*args, **kwargs) -> None: raise PermissionError(13, "Permission denied") @@ -77,14 +80,19 @@ def _raises_eperm(*args, **kwargs) -> None: lint = linters.PipCheck() assert lint.run(valid_venv_path) == LintResult.NONAPPLICABLE - assert lint.text == f"Permission denied: Could not run Python executable at {sys.executable}." + assert ( + lint.text + == f"Permission denied: Could not run Python executable at {sys.executable}." + ) + +@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.") def test_pip_check_repair_no_bin(valid_venv_path: pathlib.Path, fp): """Check that the bin directory is deleted if it was missing before""" fp.register( [sys.executable, "-m", "pip", "--python", fp.any(), "check"], - returncode = 0, - stdout = "Gosh, I sure hope I remember where everything went." + returncode=0, + stdout="Gosh, I sure hope I remember where everything went.", ) lint = linters.PipCheck() @@ -93,13 +101,15 @@ def test_pip_check_repair_no_bin(valid_venv_path: pathlib.Path, fp): assert lint.text == "Virtual environment is valid." assert not (valid_venv_path / "venv" / "bin").exists() + +@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.") def test_pip_check_repair_no_py(valid_venv_path: pathlib.Path, fp): """Check that the python symlink is deleted if it was missing before""" fp.register( - [sys.executable, "-m", "pip", "--python", fp.any(), "check"], - returncode = 0, - stdout = "Gosh, I sure hope I remember where everything went." - ) + [sys.executable, "-m", "pip", "--python", fp.any(), "check"], + returncode=0, + stdout="Gosh, I sure hope I remember where everything went.", + ) lint = linters.PipCheck() # Make sure it keeps "bin" if only the Python binary didn't exist @@ -109,12 +119,14 @@ def test_pip_check_repair_no_py(valid_venv_path: pathlib.Path, fp): assert (valid_venv_path / "venv" / "bin").exists() assert not (valid_venv_path / "venv" / "bin" / "python").exists() + +@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.") def test_pip_check_repair_all(valid_venv_path: pathlib.Path, fp): """Check that nothing is changed if all components are present""" fp.register( [sys.executable, "-m", "pip", "--python", fp.any(), "check"], - returncode = 0, - stdout = "Gosh, I sure hope I remember where everything went." + returncode=0, + stdout="Gosh, I sure hope I remember where everything went.", ) lint = linters.PipCheck() @@ -123,4 +135,4 @@ def test_pip_check_repair_all(valid_venv_path: pathlib.Path, fp): assert lint.run(valid_venv_path) == LintResult.OK assert lint.text == "Virtual environment is valid." - assert (valid_venv_path / "venv" / "bin" / "python").is_file() \ No newline at end of file + assert (valid_venv_path / "venv" / "bin" / "python").is_file() From 5ac9d2c8cfc53ebb42583e4c8ceff40e17827d8c Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Thu, 17 Oct 2024 16:38:45 -0400 Subject: [PATCH 15/16] Update charmcraft/linters.py Co-authored-by: Callahan --- charmcraft/linters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charmcraft/linters.py b/charmcraft/linters.py index 261e1a614..40e639ca6 100644 --- a/charmcraft/linters.py +++ b/charmcraft/linters.py @@ -729,7 +729,7 @@ def run(self, basedir: pathlib.Path) -> str: capture_output=True, check=False, ) - if check.returncode == 0: + if check.returncode == os.EX_OK: result = self.Result.OK else: self.text = check.stdout From 0e7abd53d560140519fb3e1ce02b3be0316eb5e8 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Fri, 18 Oct 2024 08:57:35 -0400 Subject: [PATCH 16/16] chore: empty commit to pass CLA CI check