From eb8e3158d088e01bb6b28ad545f7aca5944bba51 Mon Sep 17 00:00:00 2001 From: James Garner Date: Thu, 5 Sep 2024 14:46:06 +1200 Subject: [PATCH 01/33] Add coverage to error branch of Snap._snap_daemons --- tests/unit/test_snap.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 94875a1c..260f494c 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -350,6 +350,15 @@ def test_can_run_snap_daemon_commands(self, mock_subprocess): capture_output=True, ) + @patch( + "charms.operator_libs_linux.v2.snap.subprocess.run", + side_effect=CalledProcessError(returncode=1, cmd=""), + ) + def test_snap_daemon_commands_raise_snap_error(self, mock_subprocess: MagicMock): + foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic") + with self.assertRaises(snap.SnapError): + foo.start(["bad", "arguments"], enable=True) + @patch("charms.operator_libs_linux.v2.snap.subprocess.run") def test_snap_connect(self, mock_subprocess): mock_subprocess.return_value = MagicMock() From c7a0a528f2dfbacad4e777fd28bad3cbf6031290 Mon Sep 17 00:00:00 2001 From: James Garner Date: Thu, 5 Sep 2024 15:53:55 +1200 Subject: [PATCH 02/33] Add coverage for Snap.logs under test_can_run_snap_daemon_commands --- tests/unit/test_snap.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 260f494c..df720cf6 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -350,6 +350,22 @@ def test_can_run_snap_daemon_commands(self, mock_subprocess): capture_output=True, ) + foo.logs() + mock_subprocess.assert_called_with( + ["snap", "logs", "-n=10", "foo"], + universal_newlines=True, + check=True, + capture_output=True, + ) + + foo.logs(services=["bar", "baz"], num_lines=None) + mock_subprocess.assert_called_with( + ["snap", "logs", "foo.bar", "foo.baz"], + universal_newlines=True, + check=True, + capture_output=True, + ) + @patch( "charms.operator_libs_linux.v2.snap.subprocess.run", side_effect=CalledProcessError(returncode=1, cmd=""), From b08c27140db8ad6d3e115de0aa2dcdad8715f200 Mon Sep 17 00:00:00 2001 From: James Garner Date: Thu, 5 Sep 2024 16:05:08 +1200 Subject: [PATCH 03/33] Add coverage to error branch of Snap.connect --- tests/unit/test_snap.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index df720cf6..ef6955c3 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -404,6 +404,15 @@ def test_snap_connect(self, mock_subprocess): capture_output=True, ) + @patch( + "charms.operator_libs_linux.v2.snap.subprocess.run", + side_effect=CalledProcessError(returncode=1, cmd=""), + ) + def test_snap_snap_connect_raises_snap_error(self, mock_subprocess: MagicMock): + foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic") + with self.assertRaises(snap.SnapError): + foo.connect(plug="bad", slot="argument") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_snap_hold_timedelta(self, mock_subprocess): mock_subprocess.return_value = 0 From a070cb4fbd3adbda31ce3af9a5823fc598bf1616 Mon Sep 17 00:00:00 2001 From: James Garner Date: Thu, 5 Sep 2024 16:13:29 +1200 Subject: [PATCH 04/33] Add coverage for Snap.restart under test_can_run_snap_daemon_commands --- tests/unit/test_snap.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index ef6955c3..747b9e2b 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -366,6 +366,22 @@ def test_can_run_snap_daemon_commands(self, mock_subprocess): capture_output=True, ) + foo.restart() + mock_subprocess.assert_called_with( + ["snap", "restart", "foo"], + universal_newlines=True, + check=True, + capture_output=True, + ) + + foo.restart(["bar", "baz"], reload=True) + mock_subprocess.assert_called_with( + ["snap", "restart", "--reload", "foo.bar", "foo.baz"], + universal_newlines=True, + check=True, + capture_output=True, + ) + @patch( "charms.operator_libs_linux.v2.snap.subprocess.run", side_effect=CalledProcessError(returncode=1, cmd=""), From 3d7cb8d7639635baf428686d16e882c605780a7e Mon Sep 17 00:00:00 2001 From: James Garner Date: Thu, 5 Sep 2024 16:47:30 +1200 Subject: [PATCH 05/33] Complete branch coverage in Snap.ensure --- tests/unit/test_snap.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 747b9e2b..dac16a52 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -260,7 +260,7 @@ def test_can_compare_snap_equality(self): self.assertEqual(foo1, foo2) @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") - def test_can_run_snap_commands(self, mock_subprocess): + def test_can_run_snap_commands(self, mock_subprocess: MagicMock): mock_subprocess.return_value = 0 foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic") self.assertEqual(foo.present, True) @@ -291,7 +291,17 @@ def test_can_run_snap_commands(self, mock_subprocess): ) @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") - def test_can_run_snap_commands_devmode(self, mock_check_output): + def test_no_subprocess_when_not_installed(self, mock_subprocess: MagicMock): + foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic") + not_installed_states = (snap.SnapState.Absent, snap.SnapState.Available) + for _state in not_installed_states: + foo._state = _state + for state in not_installed_states: + foo.ensure(state) + mock_subprocess.assert_not_called() + + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") + def test_can_run_snap_commands_devmode(self, mock_check_output: MagicMock): mock_check_output.return_value = 0 foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "devmode") self.assertEqual(foo.present, True) @@ -321,6 +331,9 @@ def test_can_run_snap_commands_devmode(self, mock_check_output): ["snap", "install", "foo", "--devmode", '--revision="123"'], universal_newlines=True ) + with self.assertRaises(ValueError): + foo.ensure(snap.SnapState.Latest, devmode=True, classic=True) + @patch("charms.operator_libs_linux.v2.snap.subprocess.run") def test_can_run_snap_daemon_commands(self, mock_subprocess): mock_subprocess.return_value = MagicMock() From 1d09be3d6aefae546e16f8973acedaeff3c1a410 Mon Sep 17 00:00:00 2001 From: James Garner Date: Thu, 5 Sep 2024 16:48:13 +1200 Subject: [PATCH 06/33] Add test to cover Snap.unset --- tests/unit/test_snap.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index dac16a52..89452941 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -684,6 +684,19 @@ def test_snap_set_untyped(self, mock_subprocess): universal_newlines=True, ) + @patch( + "charms.operator_libs_linux.v2.snap.subprocess.check_output", + side_effect=lambda *args, **kwargs: "", # pyright: ignore[reportUnknownLambdaType] + ) + def test_snap_unset(self, mock_subprocess: MagicMock): + foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic") + key: str = "test_key" + self.assertEqual(foo.unset(key), "") + mock_subprocess.assert_called_with( + ["snap", "unset", "foo", key], + universal_newlines=True, + ) + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_call") def test_system_set(self, mock_subprocess): snap._system_set("refresh.hold", "foobar") From f17bd9c4985be30bc606af4935a1c97ddc24290f Mon Sep 17 00:00:00 2001 From: James Garner Date: Thu, 5 Sep 2024 16:53:01 +1200 Subject: [PATCH 07/33] Add test for Snap.get (tests pass) --- tests/unit/test_snap.py | 42 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 89452941..5e87285c 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -5,6 +5,7 @@ import json import unittest from subprocess import CalledProcessError +from typing import Iterable, Optional from unittest.mock import MagicMock, mock_open, patch import fake_snapd as fake_snapd @@ -660,6 +661,47 @@ def raise_error(cmd, **kwargs): self.assertEqual("", ctx.exception.name) self.assertIn("Failed to install or refresh snap(s): nothere", ctx.exception.message) + def test_snap_get(self): + def fake_snap(command: str, optargs: Optional[Iterable[str]] = None) -> str: + assert command == "get" + assert optargs is not None + optargs = list(optargs) + if optargs == ["-d"]: + return json.dumps(keys_and_values) + if len(optargs) == 1: # [] + key = optargs[0] + if key in keys_and_values: + return str(keys_and_values[key]) + raise snap.SnapError() + if len(optargs) == 2 and optargs[0] == "-d": # ["-d", ] + key = optargs[1] + if key in keys_and_values: + return json.dumps({key: keys_and_values[key]}) + return json.dumps({}) + raise snap.SnapError("Bad arguments:", command, optargs) + + foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic") + foo._snap = MagicMock(side_effect=fake_snap) + keys_and_values = { + "key_w_string_value": "string", + "key_w_float_value": 4.2, + "key_w_int_value": 13, + "key_w_json_value": {"key1": "string", "key2": 4.2, "key3": 13}, + } + for key, value in keys_and_values.items(): + self.assertEqual(foo.get(key, typed=True), value) + self.assertEqual(foo.get(key, typed=False), str(value)) + self.assertEqual(foo.get(key), str(value)) + self.assertEqual(foo.get(None, typed=True), keys_and_values) + self.assertEqual(foo.get("", typed=True), keys_and_values) + self.assertIs(foo.get("missing_key", typed=True), None) + with self.assertRaises(snap.SnapError): + foo.get("missing_key", typed=False) + with self.assertRaises(TypeError): + foo.get(None, typed=False) # pyright: ignore[reportCallIssue, reportArgumentType] + with self.assertRaises(TypeError): + foo.get(None) # pyright: ignore[reportArgumentType] + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_snap_set_typed(self, mock_subprocess): foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic") From 8a2a48dd4588b49e039605cda2e206e48c0e8588 Mon Sep 17 00:00:00 2001 From: James Garner Date: Thu, 5 Sep 2024 17:27:12 +1200 Subject: [PATCH 08/33] Complete branch coverage for Snap._refresh --- tests/unit/test_snap.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 5e87285c..19e6b006 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -291,6 +291,42 @@ def test_can_run_snap_commands(self, mock_subprocess: MagicMock): ["snap", "install", "foo", "--classic", '--revision="123"'], universal_newlines=True ) + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") + def test_refresh_revision_devmode_cohort_args(self, mock_subprocess: MagicMock): + foo = snap.Snap( + name="foo", + state=snap.SnapState.Present, + channel="stable", + revision="1", + confinement="devmode", + apps=None, + cohort="A", + ) + foo.ensure(snap.SnapState.Latest, revision="2", devmode=True) + mock_subprocess.assert_called_with( + [ + "snap", + "refresh", + "foo", + '--revision="2"', + "--devmode", + '--cohort="A"', + ], + universal_newlines=True, + ) + + foo._refresh(leave_cohort=True) + mock_subprocess.assert_called_with( + [ + "snap", + "refresh", + "foo", + "--leave-cohort", + ], + universal_newlines=True, + ) + self.assertEqual(foo._cohort, "") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_no_subprocess_when_not_installed(self, mock_subprocess: MagicMock): foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic") From b839ddd2784924f4fe815710277278948a11c208 Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:04:55 +1200 Subject: [PATCH 09/33] Add file level pyright directive to test_snap.py --- tests/unit/test_snap.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 19e6b006..ed1007bc 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -1,6 +1,8 @@ # Copyright 2021 Canonical Ltd. # See LICENSE file for licensing details. +# pyright: reportPrivateUsage=false + import datetime import json import unittest From ca7cf22aaea2a148f28d136fd12372be5be0a8e3 Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:06:42 +1200 Subject: [PATCH 10/33] Test strip resulting in an empty string when loading snaps --- tests/unit/test_snap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index ed1007bc..1b555936 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -197,7 +197,7 @@ def __init__(self): class TestSnapCache(unittest.TestCase): - @patch("builtins.open", new_callable=mock_open, read_data="foo\nbar\n") + @patch("builtins.open", new_callable=mock_open, read_data="foo\nbar\n \n") @patch("os.path.isfile") def test_can_load_snap_cache(self, mock_exists, m): m.return_value.__iter__ = lambda self: self From 67f13d9014c0fb0cb0b063094012cd7677efd33a Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:07:57 +1200 Subject: [PATCH 11/33] Add test for when snapd not installed --- tests/unit/test_snap.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 1b555936..f5c0f4a2 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -197,6 +197,11 @@ def __init__(self): class TestSnapCache(unittest.TestCase): + def test_error_on_not_snapd_installed(self): + with patch.object(snap.SnapCache, "snapd_installed", new=False): + with self.assertRaises(snap.SnapError): + snap.SnapCache() + @patch("builtins.open", new_callable=mock_open, read_data="foo\nbar\n \n") @patch("os.path.isfile") def test_can_load_snap_cache(self, mock_exists, m): From 8642aa4012c74d88903e22c55ab1722eaf15e09b Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:08:33 +1200 Subject: [PATCH 12/33] Test snap cache initialised automatically On first call of decorated functions --- tests/unit/test_snap.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index f5c0f4a2..6216c3bb 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -202,6 +202,39 @@ def test_error_on_not_snapd_installed(self): with self.assertRaises(snap.SnapError): snap.SnapCache() + @patch( + "charms.operator_libs_linux.v2.snap.subprocess.check_output", + return_value=0, + ) + @patch.object(snap, "SnapCache", new=SnapCacheTester) + def test_new_snap_cache_on_first_decorated(self, _mock_check_output: MagicMock): + """Test that the snap cache is created when a decorated function is called. + + add, remove and ensure are decorated with cache_init, which initialises a new cache + when these functions are called if there isn't one yet + """ + + class FakeCache: + cache = None + + def __getitem__(self, name: str) -> snap.Snap: + return self.cache[name] # pyright: ignore + + with patch.object(snap, "_Cache", new=FakeCache()): + self.assertIsNone(snap._Cache.cache) + snap.add(snap_names="curl") + self.assertIsInstance(snap._Cache.cache, snap.SnapCache) + + with patch.object(snap, "_Cache", new=FakeCache()): + self.assertIsNone(snap._Cache.cache) + snap.remove(snap_names="curl") + self.assertIsInstance(snap._Cache.cache, snap.SnapCache) + + with patch.object(snap, "_Cache", new=FakeCache()): + self.assertIsNone(snap._Cache.cache) + snap.ensure(snap_names="curl", state="latest") + self.assertIsInstance(snap._Cache.cache, snap.SnapCache) + @patch("builtins.open", new_callable=mock_open, read_data="foo\nbar\n \n") @patch("os.path.isfile") def test_can_load_snap_cache(self, mock_exists, m): From 4104e4bc527ab7db5583f40331aee565180f2909 Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:09:46 +1200 Subject: [PATCH 13/33] Test snap map loading deferred if catalog not populated --- tests/unit/test_snap.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 6216c3bb..bdb0af5b 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -246,6 +246,12 @@ def test_can_load_snap_cache(self, mock_exists, m): self.assertIn("foo", s._snap_map) self.assertEqual(len(s._snap_map), 2) + @patch("os.path.isfile", return_value=False) + def test_no_load_if_catalog_not_populated(self, mock_isfile: MagicMock): + s = SnapCacheTester() + s._load_available_snaps() + self.assertFalse(s._snap_map) # pyright: ignore[reportUnknownMemberType] + @patch("builtins.open", new_callable=mock_open, read_data="curl\n") @patch("os.path.isfile") def test_can_lazy_load_snap_info(self, mock_exists, m): From 2f9e9d711ff9e280f686c35ae9c05ceeb0b8ed82 Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:10:58 +1200 Subject: [PATCH 14/33] Add test cases for behaviour of loaded snap cache --- tests/unit/test_snap.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index bdb0af5b..3d8454c5 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -282,6 +282,7 @@ def test_can_load_installed_snap_info(self, mock_exists): self.assertEqual(len(s), 2) self.assertIn("charmcraft", s) + self.assertEqual(list(s), [s["charmcraft"], s["core"]]) # test SnapCache.__iter__ self.assertEqual(s["charmcraft"].name, "charmcraft") self.assertEqual(s["charmcraft"].state, snap.SnapState.Latest) @@ -289,6 +290,12 @@ def test_can_load_installed_snap_info(self, mock_exists): self.assertEqual(s["charmcraft"].confinement, "classic") self.assertEqual(s["charmcraft"].revision, "603") + s._snap_client.get_snap_information.side_effect = snap.SnapAPIError( + body={}, code=123, status="status", message="message" + ) + with self.assertRaises(snap.SnapNotFoundError): + s["unknown-snap"] + @patch("os.path.isfile") def test_raises_error_if_snap_not_running(self, mock_exists): mock_exists.return_value = False From 5cf1c0b4bff5eab3d928e7f106ad34f62362aea2 Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:14:11 +1200 Subject: [PATCH 15/33] Ensure custom __repr__ etc of various objects do not raise errors --- tests/unit/test_snap.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 3d8454c5..01c4ec1f 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -305,6 +305,7 @@ def test_raises_error_if_snap_not_running(self, mock_exists): ) with self.assertRaises(snap.SnapAPIError) as ctx: s._load_installed_snaps() + repr(ctx.exception) # ensure custom __repr__ doesn't error self.assertEqual("", ctx.exception.name) self.assertIn("snapd is not running", ctx.exception.message) @@ -313,6 +314,12 @@ def test_can_compare_snap_equality(self): foo2 = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic") self.assertEqual(foo1, foo2) + def test_snap_magic_methods(self): + foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic") + self.assertEqual(hash(foo), hash((foo._name, foo._revision))) + str(foo) # ensure custom __str__ doesn't error + repr(foo) # ensure custom __repr__ doesn't error + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_can_run_snap_commands(self, mock_subprocess: MagicMock): mock_subprocess.return_value = 0 @@ -747,6 +754,7 @@ def raise_error(cmd, **kwargs): mock_subprocess.side_effect = raise_error with self.assertRaises(snap.SnapError) as ctx: snap.add("nothere") + repr(ctx.exception) # ensure custom __repr__ doesn't error self.assertEqual("", ctx.exception.name) self.assertIn("Failed to install or refresh snap(s): nothere", ctx.exception.message) From af115aa233dafb5411af7d7d092af752d3d7c80e Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:14:59 +1200 Subject: [PATCH 16/33] Assert setting state to current doesn't call subprocess --- tests/unit/test_snap.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 01c4ec1f..8ccf1d58 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -325,6 +325,8 @@ def test_can_run_snap_commands(self, mock_subprocess: MagicMock): mock_subprocess.return_value = 0 foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic") self.assertEqual(foo.present, True) + foo.state = snap.SnapState.Present + mock_subprocess.assert_not_called() foo.ensure(snap.SnapState.Absent) mock_subprocess.assert_called_with(["snap", "remove", "foo"], universal_newlines=True) From b8d95a5c310fd640961ddfce1f03172274da5ac7 Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:15:51 +1200 Subject: [PATCH 17/33] Fix: revision should be a string --- tests/unit/test_snap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 8ccf1d58..5a4d2e92 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -348,7 +348,7 @@ def test_can_run_snap_commands(self, mock_subprocess: MagicMock): foo.state = snap.SnapState.Absent mock_subprocess.assert_called_with(["snap", "remove", "foo"], universal_newlines=True) - foo.ensure(snap.SnapState.Latest, revision=123) + foo.ensure(snap.SnapState.Latest, revision="123") mock_subprocess.assert_called_with( ["snap", "install", "foo", "--classic", '--revision="123"'], universal_newlines=True ) From b2befeb6adc04b6094279a3d3a04e763cf15c110 Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:30:44 +1200 Subject: [PATCH 18/33] Test non-covered internal paths of socket client --- tests/unit/test_snap.py | 55 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 5a4d2e92..1aa3f3e8 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -639,6 +639,61 @@ def test_fake_socket(self): finally: shutdown() + def test_internals(self): + """Test some internal paths that aren't covered by existing public methods.""" + # test NotImplementedError raised when missing socket.AF_UNIX + with patch("builtins.hasattr", return_value=False): + # test timeout not provided + s = snap._UnixSocketConnection("localhost") + with self.assertRaises(NotImplementedError): + s.connect() # hasattr(socket, "AF_UNIX") == False + + shutdown, socket_path = fake_snapd.start_server() + try: + # test path when _UnixSocketConnection.timeout somehow becomes None + s = snap._UnixSocketConnection("localhost", socket_path=socket_path, timeout=1) + s.timeout = None + with patch.object(snap.socket.socket, "connect"): + s.connect() + self.assertEqual(s.sock.gettimeout(), None) + + client = snap.SnapClient( + socket_path, + # cover path in __init__ when opener is provided + opener=snap.SnapClient._get_default_opener( # pyright: ignore[reportUnknownArgumentType, reportUnknownMemberType] + socket_path + ), + ) + # test calling SnapClient._request with a body argument + with patch.object(client, "_request_raw", side_effect=client._request_raw) as mock_raw: + body = {"some": "body"} + with self.assertRaises(snap.SnapAPIError): + client._request( # pyright: ignore[reportUnknownMemberType] + "GET", "snaps", body=body + ) + mock_raw.assert_called_with( + "GET", # method + "snaps", # path + None, # query + {"Accept": "application/json", "Content-Type": "application/json"}, # headers + json.dumps(body).encode("utf-8"), # body + ) + # test calling _request_raw with no headers + with patch.object( + snap.urllib.request, "Request", side_effect=snap.urllib.request.Request + ) as mock_request: + with self.assertRaises(snap.SnapAPIError): + client._request_raw("GET", "snaps") # pyright: ignore[reportUnknownMemberType] + self.assertEqual(mock_request.call_args.kwargs["headers"], {}) + # test error on invalid response + with patch.object(snap.json, "loads", return_value={}): + with self.assertRaises(snap.SnapAPIError) as ctx: + client._request_raw("GET", "snaps") # pyright: ignore[reportUnknownMemberType] + self.assertEqual(ctx.exception.body, {}) + self.assertEqual(ctx.exception.message, "KeyError - 'result'") + finally: + shutdown() + class TestSnapBareMethods(unittest.TestCase): @patch("builtins.open", new_callable=mock_open, read_data="curl\n") From ec806bd1569a33c7a83ecf60f54c3edfe3ccf95e Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:35:40 +1200 Subject: [PATCH 19/33] Cover additional paths when running snap commands --- tests/unit/test_snap.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 1aa3f3e8..500b41c5 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -713,7 +713,7 @@ def setUp(self, mock_exists, m): snap._Cache.cache._load_available_snaps() @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") - def test_can_run_bare_changes(self, mock_subprocess): + def test_can_run_bare_changes(self, mock_subprocess: MagicMock): mock_subprocess.return_value = 0 foo = snap.add("curl", classic=True, channel="latest") mock_subprocess.assert_called_with( @@ -721,10 +721,19 @@ def test_can_run_bare_changes(self, mock_subprocess): universal_newlines=True, ) self.assertTrue(foo.present) + snap.add("curl", state="latest") # cover string conversion path + mock_subprocess.assert_called_with( + ["snap", "refresh", "curl", '--channel="latest"'], + universal_newlines=True, + ) + with self.assertRaises(TypeError): # cover error path + snap.add(snap_names=[]) bar = snap.remove("curl") mock_subprocess.assert_called_with(["snap", "remove", "curl"], universal_newlines=True) self.assertFalse(bar.present) + with self.assertRaises(TypeError): # cover error path + snap.remove(snap_names=[]) baz = snap.add("curl", classic=True, revision=123) mock_subprocess.assert_called_with( From cd21ab0f1762bc722f7cdb5cb99b0f89486e52fa Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:37:28 +1200 Subject: [PATCH 20/33] Cover both failure paths when running snap commands --- tests/unit/test_snap.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 500b41c5..6bea76b8 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -810,7 +810,7 @@ def test_can_ensure_states(self, mock_subprocess): self.assertTrue(baz.present) @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") - def test_raises_snap_not_found_error(self, mock_subprocess): + def test_raises_snap_error_on_failed_subprocess(self, mock_subprocess: MagicMock): def raise_error(cmd, **kwargs): # If we can't find the snap, we should raise a CalledProcessError. # @@ -824,6 +824,22 @@ def raise_error(cmd, **kwargs): self.assertEqual("", ctx.exception.name) self.assertIn("Failed to install or refresh snap(s): nothere", ctx.exception.message) + def test_raises_snap_error_on_snap_not_found(self): + """A cache failure will also ultimately result in a SnapError.""" + + class NotFoundCache: + cache = None + + def __getitem__(self, name: str) -> snap.Snap: + raise snap.SnapNotFoundError() + + with patch.object(snap, "_Cache", new=NotFoundCache()): + with self.assertRaises(snap.SnapError) as ctx: + snap.add("nothere") + repr(ctx.exception) # ensure custom __repr__ doesn't error + self.assertEqual("", ctx.exception.name) + self.assertIn("Failed to install or refresh snap(s): nothere", ctx.exception.message) + def test_snap_get(self): def fake_snap(command: str, optargs: Optional[Iterable[str]] = None) -> str: assert command == "get" From 417722cfea438bf6763188bd5f12bfd5a97d20c4 Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:37:55 +1200 Subject: [PATCH 21/33] Cover devmode case in existing tests --- tests/unit/test_snap.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 6bea76b8..87506707 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -998,6 +998,7 @@ def test_install_local_args(self, mock_subprocess): mock_subprocess.return_value = "curl XXX installed" for kwargs, cmd_args in [ ({"classic": True}, ["--classic"]), + ({"devmode": True}, ["--devmode"]), ({"dangerous": True}, ["--dangerous"]), ({"classic": True, "dangerous": True}, ["--classic", "--dangerous"]), ]: From 878d5d8029d458eb76de4e6de0d97d4723b903f2 Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:38:32 +1200 Subject: [PATCH 22/33] Cover error cases when using local install --- tests/unit/test_snap.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 87506707..a8d5e34e 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -1009,6 +1009,30 @@ def test_install_local_args(self, mock_subprocess): ) mock_subprocess.reset_mock() + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") + def test_install_local_snap_api_error(self, mock_subprocess: MagicMock): + """install_local raises a SnapError if cache access raises a SnapAPIError.""" + + class APIErrorCache: + def __getitem__(self, key): + raise snap.SnapAPIError(body={}, code=123, status="status", message="message") + + mock_subprocess.return_value = "curl XXX installed" + with patch.object(snap, "SnapCache", new=APIErrorCache): + with self.assertRaises(snap.SnapError) as ctx: + snap.install_local("./curl.snap") + self.assertEqual(ctx.exception.message, "Failed to find snap curl in Snap cache") + + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") + def test_install_local_called_process_error(self, mock_subprocess: MagicMock): + """install_local raises a SnapError if the subprocess raises a CalledProcessError.""" + mock_subprocess.side_effect = CalledProcessError( + returncode=1, cmd="cmd", output="dummy-output" + ) + with self.assertRaises(snap.SnapError) as ctx: + snap.install_local("./curl.snap") + self.assertEqual(ctx.exception.message, "Could not install snap ./curl.snap: dummy-output") + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_alias(self, mock_subprocess): mock_subprocess.return_value = "" From efa99902d0b8108734351ef25d717ffa943a5aa4 Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 6 Sep 2024 14:39:10 +1200 Subject: [PATCH 23/33] Test reading hold state of snaps --- tests/unit/test_snap.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index a8d5e34e..73256e68 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -1064,3 +1064,11 @@ def test_alias_raises_snap_error(self, mock_subprocess): universal_newlines=True, ) mock_subprocess.reset_mock() + + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") + def test_held(self, mock_subprocess: MagicMock): + foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic") + mock_subprocess.return_value = {} + self.assertEqual(foo.held, False) + mock_subprocess.return_value = {"hold:": "key isn't checked"} + self.assertEqual(foo.held, True) From 331816a5a4ecb9ee9115d8b2583ce0bf49794058 Mon Sep 17 00:00:00 2001 From: James Garner Date: Wed, 18 Sep 2024 09:34:24 +1200 Subject: [PATCH 24/33] Use a decorator instead of with statement --- tests/unit/test_snap.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 73256e68..d8c8ba8e 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -197,10 +197,10 @@ def __init__(self): class TestSnapCache(unittest.TestCase): + @patch.object(snap.SnapCache, "snapd_installed", new=False) def test_error_on_not_snapd_installed(self): - with patch.object(snap.SnapCache, "snapd_installed", new=False): - with self.assertRaises(snap.SnapError): - snap.SnapCache() + with self.assertRaises(snap.SnapError): + snap.SnapCache() @patch( "charms.operator_libs_linux.v2.snap.subprocess.check_output", From 9308b719407d3430adf6bb6b455e09e831ad420b Mon Sep 17 00:00:00 2001 From: James Garner Date: Wed, 18 Sep 2024 10:01:41 +1200 Subject: [PATCH 25/33] Factor useful tests from test_internals into individual tests Remove non-useful tests --- tests/unit/test_snap.py | 64 ++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index d8c8ba8e..d0f80787 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -639,34 +639,24 @@ def test_fake_socket(self): finally: shutdown() - def test_internals(self): - """Test some internal paths that aren't covered by existing public methods.""" - # test NotImplementedError raised when missing socket.AF_UNIX - with patch("builtins.hasattr", return_value=False): - # test timeout not provided - s = snap._UnixSocketConnection("localhost") - with self.assertRaises(NotImplementedError): - s.connect() # hasattr(socket, "AF_UNIX") == False - + @patch("builtins.hasattr", return_value=False) + def test_not_implemented_raised_when_missing_socket_af_unix(self, _: MagicMock): + """Assert NotImplementedError raised when missing socket.AF_UNIX""" + s = snap._UnixSocketConnection("localhost") + with self.assertRaises(NotImplementedError): + s.connect() # hasattr(socket, "AF_UNIX") == False + + def test_request_bad_body_raises_snapapierror(self): + """Assert SnapAPIError raised on SnapClient._request with bad body.""" shutdown, socket_path = fake_snapd.start_server() try: - # test path when _UnixSocketConnection.timeout somehow becomes None - s = snap._UnixSocketConnection("localhost", socket_path=socket_path, timeout=1) - s.timeout = None - with patch.object(snap.socket.socket, "connect"): - s.connect() - self.assertEqual(s.sock.gettimeout(), None) - - client = snap.SnapClient( - socket_path, - # cover path in __init__ when opener is provided - opener=snap.SnapClient._get_default_opener( # pyright: ignore[reportUnknownArgumentType, reportUnknownMemberType] - socket_path - ), - ) - # test calling SnapClient._request with a body argument - with patch.object(client, "_request_raw", side_effect=client._request_raw) as mock_raw: - body = {"some": "body"} + client = snap.SnapClient(socket_path) + body = {"bad": "body"} + with patch.object( + client, + "_request_raw", + side_effect=client._request_raw, # pyright: ignore[reportUnknownMemberType] + ) as mock_raw: with self.assertRaises(snap.SnapAPIError): client._request( # pyright: ignore[reportUnknownMemberType] "GET", "snaps", body=body @@ -678,18 +668,34 @@ def test_internals(self): {"Accept": "application/json", "Content-Type": "application/json"}, # headers json.dumps(body).encode("utf-8"), # body ) - # test calling _request_raw with no headers + finally: + shutdown() + + def test_request_raw_missing_headers_raises_snapapierror(self): + """Assert SnapAPIError raised on SnapClient._request_raw when missing headers.""" + shutdown, socket_path = fake_snapd.start_server() + try: + client = snap.SnapClient(socket_path) with patch.object( snap.urllib.request, "Request", side_effect=snap.urllib.request.Request ) as mock_request: with self.assertRaises(snap.SnapAPIError): client._request_raw("GET", "snaps") # pyright: ignore[reportUnknownMemberType] self.assertEqual(mock_request.call_args.kwargs["headers"], {}) - # test error on invalid response + finally: + shutdown() + + def test_request_raw_bad_response_raises_snapapierror(self): + """Assert SnapAPIError raised on SnapClient._request_raw when receiving a bad response.""" + shutdown, socket_path = fake_snapd.start_server() + try: + client = snap.SnapClient(socket_path) with patch.object(snap.json, "loads", return_value={}): with self.assertRaises(snap.SnapAPIError) as ctx: client._request_raw("GET", "snaps") # pyright: ignore[reportUnknownMemberType] - self.assertEqual(ctx.exception.body, {}) + # the return_value was correctly patched in + self.assertEqual(ctx.exception.body, {}) # pyright: ignore[reportUnknownMemberType] + # response is bad because it's missing expected keys self.assertEqual(ctx.exception.message, "KeyError - 'result'") finally: shutdown() From fe4b9997c40b06d3bf8758c00e0b0253c281332f Mon Sep 17 00:00:00 2001 From: James Garner Date: Wed, 18 Sep 2024 10:12:57 +1200 Subject: [PATCH 26/33] Remove spurious test --- tests/unit/test_snap.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index d0f80787..80b1c616 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -290,12 +290,6 @@ def test_can_load_installed_snap_info(self, mock_exists): self.assertEqual(s["charmcraft"].confinement, "classic") self.assertEqual(s["charmcraft"].revision, "603") - s._snap_client.get_snap_information.side_effect = snap.SnapAPIError( - body={}, code=123, status="status", message="message" - ) - with self.assertRaises(snap.SnapNotFoundError): - s["unknown-snap"] - @patch("os.path.isfile") def test_raises_error_if_snap_not_running(self, mock_exists): mock_exists.return_value = False From 41d550fd1aa93140af6cdbb0026de94b9d4dd46b Mon Sep 17 00:00:00 2001 From: James Garner Date: Wed, 18 Sep 2024 10:13:38 +1200 Subject: [PATCH 27/33] Rename cache placeholder to reflect it's job --- tests/unit/test_snap.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 80b1c616..020a186a 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -214,23 +214,23 @@ def test_new_snap_cache_on_first_decorated(self, _mock_check_output: MagicMock): when these functions are called if there isn't one yet """ - class FakeCache: + class CachePlaceholder: cache = None def __getitem__(self, name: str) -> snap.Snap: return self.cache[name] # pyright: ignore - with patch.object(snap, "_Cache", new=FakeCache()): + with patch.object(snap, "_Cache", new=CachePlaceholder()): self.assertIsNone(snap._Cache.cache) snap.add(snap_names="curl") self.assertIsInstance(snap._Cache.cache, snap.SnapCache) - with patch.object(snap, "_Cache", new=FakeCache()): + with patch.object(snap, "_Cache", new=CachePlaceholder()): self.assertIsNone(snap._Cache.cache) snap.remove(snap_names="curl") self.assertIsInstance(snap._Cache.cache, snap.SnapCache) - with patch.object(snap, "_Cache", new=FakeCache()): + with patch.object(snap, "_Cache", new=CachePlaceholder()): self.assertIsNone(snap._Cache.cache) snap.ensure(snap_names="curl", state="latest") self.assertIsInstance(snap._Cache.cache, snap.SnapCache) From 0c35f3201b233d72f15f605fbbba79c5d186f936 Mon Sep 17 00:00:00 2001 From: James Garner Date: Wed, 18 Sep 2024 10:18:43 +1200 Subject: [PATCH 28/33] Document reason for raising error --- tests/unit/test_snap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 020a186a..277b3d06 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -424,7 +424,7 @@ def test_can_run_snap_commands_devmode(self, mock_check_output: MagicMock): ["snap", "install", "foo", "--devmode", '--revision="123"'], universal_newlines=True ) - with self.assertRaises(ValueError): + with self.assertRaises(ValueError): # devmode and classic are mutually exclusive foo.ensure(snap.SnapState.Latest, devmode=True, classic=True) @patch("charms.operator_libs_linux.v2.snap.subprocess.run") From 4ca072dd1d0e06c1715e38aed3d7647795c0517a Mon Sep 17 00:00:00 2001 From: James Garner Date: Wed, 18 Sep 2024 10:27:28 +1200 Subject: [PATCH 29/33] Document some tests added in this branch --- tests/unit/test_snap.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 277b3d06..fe9c4025 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -349,6 +349,7 @@ def test_can_run_snap_commands(self, mock_subprocess: MagicMock): @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_refresh_revision_devmode_cohort_args(self, mock_subprocess: MagicMock): + """Test that ensure and _refresh succeed and call the correct snap commands.""" foo = snap.Snap( name="foo", state=snap.SnapState.Present, @@ -385,6 +386,7 @@ def test_refresh_revision_devmode_cohort_args(self, mock_subprocess: MagicMock): @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") def test_no_subprocess_when_not_installed(self, mock_subprocess: MagicMock): + """Don't call out to snap when ensuring an uninstalled state when not installed.""" foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic") not_installed_states = (snap.SnapState.Absent, snap.SnapState.Available) for _state in not_installed_states: @@ -530,7 +532,8 @@ def test_snap_connect(self, mock_subprocess): "charms.operator_libs_linux.v2.snap.subprocess.run", side_effect=CalledProcessError(returncode=1, cmd=""), ) - def test_snap_snap_connect_raises_snap_error(self, mock_subprocess: MagicMock): + def test_snap_connect_raises_snap_error(self, mock_subprocess: MagicMock): + """Ensure that a SnapError is raised when Snap.connect is called with bad arguments.""" foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic") with self.assertRaises(snap.SnapError): foo.connect(plug="bad", slot="argument") From 65ebb75b11dacbb97dae3b8309867d16f21b2b95 Mon Sep 17 00:00:00 2001 From: James Garner Date: Wed, 18 Sep 2024 10:28:16 +1200 Subject: [PATCH 30/33] Remove test of overly specific implementation details --- tests/unit/test_snap.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index fe9c4025..dc3d94b6 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -824,8 +824,6 @@ def raise_error(cmd, **kwargs): with self.assertRaises(snap.SnapError) as ctx: snap.add("nothere") repr(ctx.exception) # ensure custom __repr__ doesn't error - self.assertEqual("", ctx.exception.name) - self.assertIn("Failed to install or refresh snap(s): nothere", ctx.exception.message) def test_raises_snap_error_on_snap_not_found(self): """A cache failure will also ultimately result in a SnapError.""" From 2a41ea38423279a43442f6838dcc78f99f79bebc Mon Sep 17 00:00:00 2001 From: James Garner Date: Wed, 18 Sep 2024 10:41:21 +1200 Subject: [PATCH 31/33] Document the test for the Snap.get method --- tests/unit/test_snap.py | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index dc3d94b6..a6cebc2d 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -7,7 +7,7 @@ import json import unittest from subprocess import CalledProcessError -from typing import Iterable, Optional +from typing import Any, Dict, Iterable, Optional from unittest.mock import MagicMock, mock_open, patch import fake_snapd as fake_snapd @@ -842,7 +842,26 @@ def __getitem__(self, name: str) -> snap.Snap: self.assertIn("Failed to install or refresh snap(s): nothere", ctx.exception.message) def test_snap_get(self): + """Test the multiple different ways of calling the Snap.get function. + + Valid ways: + ("key", typed=False) -> returns a string + ("key", typed=True) -> returns value parsed from json + (None, typed=True) -> returns parsed json for all keys + ("", typed=True) -> returns parsed json for all keys + + An invalid key will raise an error if typed=False, but return None if typed=True. + """ def fake_snap(command: str, optargs: Optional[Iterable[str]] = None) -> str: + """Snap._snap would normally call subprocess.check_output(["snap", ...], ...) + + Here we only handle the "get" commands generated by Snap.get: + ["snap", "get", "-d"] -- equivalent to (None, typed=True) + ["snap", "get", "key"] -- equivalent to ("key", typed=False) + ["snap", "get", "-d" "key"] -- equivalent to ("key", typed=True) + + Values are returned from the local keys_and_values dict instead of calling out to snap. + """ assert command == "get" assert optargs is not None optargs = list(optargs) @@ -862,7 +881,7 @@ def fake_snap(command: str, optargs: Optional[Iterable[str]] = None) -> str: foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic") foo._snap = MagicMock(side_effect=fake_snap) - keys_and_values = { + keys_and_values: Dict[str, Any] = { "key_w_string_value": "string", "key_w_float_value": 4.2, "key_w_int_value": 13, From 2e9b3bf7762c74cb211a4cf38e6f5e035aa602a1 Mon Sep 17 00:00:00 2001 From: James Garner Date: Wed, 18 Sep 2024 10:43:54 +1200 Subject: [PATCH 32/33] Add pyright ignore directive --- tests/unit/test_snap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index a6cebc2d..7caaab4f 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -932,7 +932,7 @@ def test_snap_set_untyped(self, mock_subprocess): def test_snap_unset(self, mock_subprocess: MagicMock): foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic") key: str = "test_key" - self.assertEqual(foo.unset(key), "") + self.assertEqual(foo.unset(key), "") # pyright: ignore[reportUnknownMemberType] mock_subprocess.assert_called_with( ["snap", "unset", "foo", key], universal_newlines=True, From 12310eb00b395fd84f4cf3cd03113ed14dd3f6e9 Mon Sep 17 00:00:00 2001 From: James Garner Date: Wed, 18 Sep 2024 10:59:16 +1200 Subject: [PATCH 33/33] Add periods and newlines to make ruff happy --- tests/unit/test_snap.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 7caaab4f..896aadf1 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -638,7 +638,7 @@ def test_fake_socket(self): @patch("builtins.hasattr", return_value=False) def test_not_implemented_raised_when_missing_socket_af_unix(self, _: MagicMock): - """Assert NotImplementedError raised when missing socket.AF_UNIX""" + """Assert NotImplementedError raised when missing socket.AF_UNIX.""" s = snap._UnixSocketConnection("localhost") with self.assertRaises(NotImplementedError): s.connect() # hasattr(socket, "AF_UNIX") == False @@ -852,8 +852,9 @@ def test_snap_get(self): An invalid key will raise an error if typed=False, but return None if typed=True. """ + def fake_snap(command: str, optargs: Optional[Iterable[str]] = None) -> str: - """Snap._snap would normally call subprocess.check_output(["snap", ...], ...) + """Snap._snap would normally call subprocess.check_output(["snap", ...], ...). Here we only handle the "get" commands generated by Snap.get: ["snap", "get", "-d"] -- equivalent to (None, typed=True)