From 1a09b1602f60a73d3515e93143ea7c37e41abb09 Mon Sep 17 00:00:00 2001 From: Guillaume Boutry Date: Wed, 17 May 2023 06:48:15 +0200 Subject: [PATCH] fix(snap): handle revision as str (#92) Snap revisions can be strings and should be handled as strings --- .../operator_libs_linux/{v1 => v2}/snap.py | 24 ++++----- tests/integration/test_snap.py | 6 +-- tests/unit/test_snap.py | 54 +++++++++---------- 3 files changed, 42 insertions(+), 42 deletions(-) rename lib/charms/operator_libs_linux/{v1 => v2}/snap.py (98%) diff --git a/lib/charms/operator_libs_linux/v1/snap.py b/lib/charms/operator_libs_linux/v2/snap.py similarity index 98% rename from lib/charms/operator_libs_linux/v1/snap.py rename to lib/charms/operator_libs_linux/v2/snap.py index eacda12..b82024c 100644 --- a/lib/charms/operator_libs_linux/v1/snap.py +++ b/lib/charms/operator_libs_linux/v2/snap.py @@ -79,11 +79,11 @@ LIBID = "05394e5893f94f2d90feb7cbe6b633cd" # Increment this major API version when introducing breaking changes -LIBAPI = 1 +LIBAPI = 2 # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 12 +LIBPATCH = 0 # Regex to locate 7-bit C1 ANSI sequences @@ -222,7 +222,7 @@ def __init__( name, state: SnapState, channel: str, - revision: int, + revision: str, confinement: str, apps: Optional[List[Dict[str, str]]] = None, cohort: Optional[str] = "", @@ -427,7 +427,7 @@ def _install( self, channel: Optional[str] = "", cohort: Optional[str] = "", - revision: Optional[int] = None, + revision: Optional[str] = None, ) -> None: """Add a snap to the system. @@ -454,7 +454,7 @@ def _refresh( self, channel: Optional[str] = "", cohort: Optional[str] = "", - revision: Optional[int] = None, + revision: Optional[str] = None, leave_cohort: Optional[bool] = False, ) -> None: """Refresh a snap. @@ -498,7 +498,7 @@ def ensure( classic: Optional[bool] = False, channel: Optional[str] = "", cohort: Optional[str] = "", - revision: Optional[int] = None, + revision: Optional[str] = None, ): """Ensure that a snap is in a given state. @@ -575,7 +575,7 @@ def state(self, state: SnapState) -> None: self._state = state @property - def revision(self) -> int: + def revision(self) -> str: """Returns the revision for a snap.""" return self._revision @@ -828,7 +828,7 @@ def _load_installed_snaps(self) -> None: name=i["name"], state=SnapState.Latest, channel=i["channel"], - revision=int(i["revision"]), + revision=i["revision"], confinement=i["confinement"], apps=i.get("apps", None), ) @@ -846,7 +846,7 @@ def _load_info(self, name) -> Snap: name=info["name"], state=SnapState.Available, channel=info["channel"], - revision=int(info["revision"]), + revision=info["revision"], confinement=info["confinement"], apps=None, ) @@ -859,7 +859,7 @@ def add( channel: Optional[str] = "", classic: Optional[bool] = False, cohort: Optional[str] = "", - revision: Optional[int] = None, + revision: Optional[str] = None, ) -> Union[Snap, List[Snap]]: """Add a snap to the system. @@ -871,7 +871,7 @@ def add( classic: an (Optional) boolean specifying whether it should be added with classic confinement. Default `False` cohort: an (Optional) string specifying the snap cohort to use - revision: an (Optional) integer specifying the snap revision to use + revision: an (Optional) string specifying the snap revision to use Raises: SnapError if some snaps failed to install or were not found. @@ -947,7 +947,7 @@ def _wrap_snap_operations( channel: str, classic: bool, cohort: Optional[str] = "", - revision: Optional[int] = None, + revision: Optional[str] = None, ) -> Union[Snap, List[Snap]]: """Wrap common operations for bare commands.""" snaps = {"success": [], "failed": []} diff --git a/tests/integration/test_snap.py b/tests/integration/test_snap.py index e295c3c..472582e 100644 --- a/tests/integration/test_snap.py +++ b/tests/integration/test_snap.py @@ -9,7 +9,7 @@ from subprocess import CalledProcessError, check_output, run import pytest -from charms.operator_libs_linux.v1 import snap +from charms.operator_libs_linux.v2 import snap from helpers import get_command_path logger = logging.getLogger(__name__) @@ -119,7 +119,7 @@ def test_snap_ensure_revision(): match = re.search(r"latest/edge.*\((\d+)\)", line) if match: - edge_revision = int(match.group(1)) + edge_revision = match.group(1) break assert edge_revision is not None @@ -139,7 +139,7 @@ def test_snap_ensure_revision(): match = re.search(r"installed.*\((\d+)\)", line) assert match is not None - assert int(match.group(1)) == edge_revision + assert match.group(1) == edge_revision def test_snap_start(): diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 9028f9c..73a0649 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -8,9 +8,9 @@ from unittest.mock import MagicMock, mock_open, patch import fake_snapd as fake_snapd -from charms.operator_libs_linux.v1 import snap +from charms.operator_libs_linux.v2 import snap -patch("charms.operator_libs_linux.v1.snap._cache_init", lambda x: x).start() +patch("charms.operator_libs_linux.v2.snap._cache_init", lambda x: x).start() lazy_load_result = r""" { @@ -223,7 +223,7 @@ def test_can_lazy_load_snap_info(self, mock_exists, m): self.assertEqual(result.state, snap.SnapState.Available) self.assertEqual(result.channel, "stable") self.assertEqual(result.confinement, "strict") - self.assertEqual(result.revision, 233) + self.assertEqual(result.revision, "233") @patch("os.path.isfile") def test_can_load_installed_snap_info(self, mock_exists): @@ -240,7 +240,7 @@ def test_can_load_installed_snap_info(self, mock_exists): self.assertEqual(s["charmcraft"].state, snap.SnapState.Latest) self.assertEqual(s["charmcraft"].channel, "latest/stable") self.assertEqual(s["charmcraft"].confinement, "classic") - self.assertEqual(s["charmcraft"].revision, 603) + self.assertEqual(s["charmcraft"].revision, "603") @patch("os.path.isfile") def test_raises_error_if_snap_not_running(self, mock_exists): @@ -251,7 +251,7 @@ def test_raises_error_if_snap_not_running(self, mock_exists): ) with self.assertRaises(snap.SnapAPIError) as ctx: s._load_installed_snaps() - self.assertEqual("", ctx.exception.name) + self.assertEqual("", ctx.exception.name) self.assertIn("snapd is not running", ctx.exception.message) def test_can_compare_snap_equality(self): @@ -259,7 +259,7 @@ def test_can_compare_snap_equality(self): foo2 = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic") self.assertEqual(foo1, foo2) - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_can_run_snap_commands(self, mock_subprocess): mock_subprocess.return_value = 0 foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic") @@ -290,7 +290,7 @@ def test_can_run_snap_commands(self, mock_subprocess): ["snap", "install", "foo", "--classic", '--revision="123"'], universal_newlines=True ) - @patch("charms.operator_libs_linux.v1.snap.subprocess.run") + @patch("charms.operator_libs_linux.v2.snap.subprocess.run") def test_can_run_snap_daemon_commands(self, mock_subprocess): mock_subprocess.return_value = MagicMock() foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic") @@ -319,7 +319,7 @@ def test_can_run_snap_daemon_commands(self, mock_subprocess): capture_output=True, ) - @patch("charms.operator_libs_linux.v1.snap.subprocess.run") + @patch("charms.operator_libs_linux.v2.snap.subprocess.run") def test_snap_connect(self, mock_subprocess): mock_subprocess.return_value = MagicMock() foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic") @@ -348,7 +348,7 @@ def test_snap_connect(self, mock_subprocess): capture_output=True, ) - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_snap_hold_timedelta(self, mock_subprocess): mock_subprocess.return_value = 0 foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic") @@ -364,7 +364,7 @@ def test_snap_hold_timedelta(self, mock_subprocess): universal_newlines=True, ) - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_snap_hold_forever(self, mock_subprocess): mock_subprocess.return_value = 0 foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic") @@ -380,7 +380,7 @@ def test_snap_hold_forever(self, mock_subprocess): universal_newlines=True, ) - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_snap_unhold(self, mock_subprocess): mock_subprocess.return_value = 0 foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic") @@ -396,7 +396,7 @@ def test_snap_unhold(self, mock_subprocess): universal_newlines=True, ) - @patch("charms.operator_libs_linux.v1.snap.SnapClient.get_installed_snap_apps") + @patch("charms.operator_libs_linux.v2.snap.SnapClient.get_installed_snap_apps") def test_apps_property(self, patched): s = SnapCacheTester() s._snap_client.get_installed_snaps.return_value = json.loads(installed_result)["result"] @@ -406,7 +406,7 @@ def test_apps_property(self, patched): self.assertEqual(len(s["charmcraft"].apps), 2) self.assertIn({"snap": "charmcraft", "name": "charmcraft"}, s["charmcraft"].apps) - @patch("charms.operator_libs_linux.v1.snap.SnapClient.get_installed_snap_apps") + @patch("charms.operator_libs_linux.v2.snap.SnapClient.get_installed_snap_apps") def test_services_property(self, patched): s = SnapCacheTester() s._snap_client.get_installed_snaps.return_value = json.loads(installed_result)["result"] @@ -464,7 +464,7 @@ def setUp(self, mock_exists, m): snap._Cache.cache._load_installed_snaps() snap._Cache.cache._load_available_snaps() - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_can_run_bare_changes(self, mock_subprocess): mock_subprocess.return_value = 0 foo = snap.add("curl", classic=True, channel="latest") @@ -484,7 +484,7 @@ def test_can_run_bare_changes(self, mock_subprocess): ) self.assertTrue(baz.present) - @patch("charms.operator_libs_linux.v1.snap.subprocess") + @patch("charms.operator_libs_linux.v2.snap.subprocess") def test_cohort(self, mock_subprocess): mock_subprocess.check_output = MagicMock() @@ -512,7 +512,7 @@ def test_cohort(self, mock_subprocess): universal_newlines=True, ) - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_can_ensure_states(self, mock_subprocess): mock_subprocess.return_value = 0 foo = snap.ensure("curl", "latest", classic=True, channel="latest/test") @@ -533,7 +533,7 @@ def test_can_ensure_states(self, mock_subprocess): ) self.assertTrue(baz.present) - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_raises_snap_not_found_error(self, mock_subprocess): def raise_error(cmd, **kwargs): # If we can't find the snap, we should raise a CalledProcessError. @@ -544,10 +544,10 @@ def raise_error(cmd, **kwargs): mock_subprocess.side_effect = raise_error with self.assertRaises(snap.SnapError) as ctx: snap.add("nothere") - self.assertEqual("", ctx.exception.name) + self.assertEqual("", ctx.exception.name) self.assertIn("Failed to install or refresh snap(s): nothere", ctx.exception.message) - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_snap_set(self, mock_subprocess): foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic") @@ -572,7 +572,7 @@ def test_snap_set(self, mock_subprocess): universal_newlines=True, ) - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_call") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_call") def test_system_set(self, mock_subprocess): snap._system_set("refresh.hold", "foobar") mock_subprocess.assert_called_with( @@ -580,7 +580,7 @@ def test_system_set(self, mock_subprocess): universal_newlines=True, ) - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_call") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_call") def test_system_set_fail(self, mock_subprocess): mock_subprocess.side_effect = CalledProcessError(1, "foobar") with self.assertRaises(snap.SnapError): @@ -598,7 +598,7 @@ def test_hold_refresh_invalid_non_bool(self): with self.assertRaises(TypeError): snap.hold_refresh(forever="foobar") - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_call") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_call") def test_hold_refresh_reset(self, mock_subprocess): snap.hold_refresh(days=0) mock_subprocess.assert_called_with( @@ -606,7 +606,7 @@ def test_hold_refresh_reset(self, mock_subprocess): universal_newlines=True, ) - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_call") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_call") def test_hold_refresh_forever(self, mock_subprocess): snap.hold_refresh(forever=True) @@ -615,8 +615,8 @@ def test_hold_refresh_forever(self, mock_subprocess): universal_newlines=True, ) - @patch("charms.operator_libs_linux.v1.snap.datetime") - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_call") + @patch("charms.operator_libs_linux.v2.snap.datetime") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_call") def test_hold_refresh_valid_days(self, mock_subprocess, mock_datetime): # A little too closely-tied to the internals of hold_refresh(), but at least # the test runs whatever your local time zone is. @@ -638,7 +638,7 @@ def test_ansi_filter(self): assert snap.ansi_filter.sub("", "\x1b[0m\x1b[?25h\x1b[Kpypi-server") == "pypi-server" assert snap.ansi_filter.sub("", "\x1b[0m\x1b[?25h\x1b[Kparca") == "parca" - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_install_local(self, mock_subprocess): mock_subprocess.return_value = "curl XXX installed" snap.install_local("./curl.snap") @@ -647,7 +647,7 @@ def test_install_local(self, mock_subprocess): universal_newlines=True, ) - @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_install_local_args(self, mock_subprocess): mock_subprocess.return_value = "curl XXX installed" for kwargs, cmd_args in [