From 2479c8b2df47b7ff2224a7cc90c8ec47b461cb4a Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Tue, 27 May 2025 11:19:15 +0800 Subject: [PATCH 1/7] fix: refresh with classic --- lib/charms/operator_libs_linux/v2/snap.py | 3 +++ tests/unit/test_snap.py | 28 ++++++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/lib/charms/operator_libs_linux/v2/snap.py b/lib/charms/operator_libs_linux/v2/snap.py index 5cd0ffd4..61f9bb85 100644 --- a/lib/charms/operator_libs_linux/v2/snap.py +++ b/lib/charms/operator_libs_linux/v2/snap.py @@ -577,6 +577,9 @@ def _refresh( if revision: args.append(f'--revision="{revision}"') + if self.confinement == 'classic': + args.append('--classic') + if devmode: args.append("--devmode") diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 5570466e..184d35b6 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -387,6 +387,31 @@ def test_refresh_revision_devmode_cohort_args(self, mock_subprocess: MagicMock): ) self.assertEqual(foo._cohort, "") + @patch('charms.operator_libs_linux.v2.snap.subprocess.check_output') + def test_refresh_classic(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, + channel='stable', + revision='1', + confinement='classic', + apps=None, + cohort='A', + ) + foo.ensure(snap.SnapState.Latest, revision='2', classic=True) + mock_subprocess.assert_called_with( + [ + 'snap', + 'refresh', + 'foo', + '--revision="2"', + '--classic', + '--cohort="A"', + ], + text=True, + ) + @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.""" @@ -884,7 +909,7 @@ def test_can_run_bare_changes(self, mock_subprocess: MagicMock): self.assertTrue(foo.present) snap.add("curl", state="latest") # cover string conversion path mock_subprocess.assert_called_with( - ["snap", "refresh", "curl", '--channel="latest"'], + ["snap", "refresh", "curl", '--channel="latest"', '--classic'], text=True, ) with self.assertRaises(TypeError): # cover error path @@ -925,6 +950,7 @@ def test_cohort(self, mock_subprocess): "refresh", "curl", '--channel="latest/beta"', + '--classic', '--cohort="+"', ], text=True, From 8d00f3f15b52db8787b458e28a48f6058b31a6ce Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Tue, 27 May 2025 11:22:49 +0800 Subject: [PATCH 2/7] chore: update test docstring --- 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 184d35b6..2f7b10e5 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -389,7 +389,7 @@ def test_refresh_revision_devmode_cohort_args(self, mock_subprocess: MagicMock): @patch('charms.operator_libs_linux.v2.snap.subprocess.check_output') def test_refresh_classic(self, mock_subprocess: MagicMock): - """Test that ensure and _refresh succeed and call the correct snap commands.""" + """Test that ensure and _refresh add the --classic flag with confinement set to classic.""" foo = snap.Snap( name='foo', state=snap.SnapState.Present, From 93772f86dfe1f8c89c3c3cb786be11000991633e Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Tue, 27 May 2025 13:28:32 +0800 Subject: [PATCH 3/7] test: add more test --- tests/unit/test_snap.py | 63 +++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 25 deletions(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 2f7b10e5..f84f12a1 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -14,6 +14,7 @@ from unittest.mock import MagicMock, mock_open, patch import fake_snapd as fake_snapd +import pytest from charms.operator_libs_linux.v2 import snap patch("charms.operator_libs_linux.v2.snap._cache_init", lambda x: x).start() @@ -387,31 +388,6 @@ def test_refresh_revision_devmode_cohort_args(self, mock_subprocess: MagicMock): ) self.assertEqual(foo._cohort, "") - @patch('charms.operator_libs_linux.v2.snap.subprocess.check_output') - def test_refresh_classic(self, mock_subprocess: MagicMock): - """Test that ensure and _refresh add the --classic flag with confinement set to classic.""" - foo = snap.Snap( - name='foo', - state=snap.SnapState.Present, - channel='stable', - revision='1', - confinement='classic', - apps=None, - cohort='A', - ) - foo.ensure(snap.SnapState.Latest, revision='2', classic=True) - mock_subprocess.assert_called_with( - [ - 'snap', - 'refresh', - 'foo', - '--revision="2"', - '--classic', - '--cohort="A"', - ], - text=True, - ) - @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.""" @@ -646,6 +622,43 @@ def test_services_property(self, patched): ) +@patch('charms.operator_libs_linux.v2.snap.subprocess.check_output') +@pytest.mark.parametrize( + 'confinement,classic,expected_flag', + [ + ('classic', False, ['--classic']), + ('classic', True, ['--classic']), + ('strict', False, []), + ('strict', True, ['--classic']), + ], +) +def test_refresh_classic( + mock_subprocess: MagicMock, confinement: str, classic: bool, expected_flag: list[str] +): + """Test that ensure and _refresh add the --classic flag with confinement set to classic.""" + foo = snap.Snap( + name='foo', + state=snap.SnapState.Present, + channel='stable', + revision='1', + confinement=confinement, + apps=None, + cohort='A', + ) + foo.ensure(snap.SnapState.Latest, revision='2', classic=classic) + mock_subprocess.assert_called_with( + [ + 'snap', + 'refresh', + 'foo', + '--revision="2"', + ] + + expected_flag + + ['--cohort="A"'], + text=True, + ) + + class TestSocketClient(unittest.TestCase): def test_socket_not_found(self): client = snap.SnapClient(socket_path="/does/not/exist") From 4090ae0d1757695125bba451761b1a7e69538cd9 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Tue, 27 May 2025 13:32:41 +0800 Subject: [PATCH 4/7] chore: update --- .report/coverage.xml | 563 ++++++++++++++++++++++++++++++++++++++++ tests/unit/test_snap.py | 4 +- 2 files changed, 564 insertions(+), 3 deletions(-) create mode 100644 .report/coverage.xml diff --git a/.report/coverage.xml b/.report/coverage.xml new file mode 100644 index 00000000..1d38850c --- /dev/null +++ b/.report/coverage.xml @@ -0,0 +1,563 @@ + + + + + + /home/ubuntu/operator-libs-linux + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index f84f12a1..2941e075 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -632,9 +632,7 @@ def test_services_property(self, patched): ('strict', True, ['--classic']), ], ) -def test_refresh_classic( - mock_subprocess: MagicMock, confinement: str, classic: bool, expected_flag: list[str] -): +def test_refresh_classic(mock_subprocess: MagicMock, confinement, classic, expected_flag): """Test that ensure and _refresh add the --classic flag with confinement set to classic.""" foo = snap.Snap( name='foo', From 9344f96522e790b3778aa117e2e4df8dc48e4822 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Wed, 4 Jun 2025 09:30:35 +0800 Subject: [PATCH 5/7] chore: add .report to gitignore --- .gitignore | 1 + .report/coverage.xml | 563 ------------------------------------------- 2 files changed, 1 insertion(+), 563 deletions(-) delete mode 100644 .report/coverage.xml diff --git a/.gitignore b/.gitignore index 3f4db9cc..867032fd 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ build/ *.charm *.orig .coverage +.report **/__pycache__/ *.py[cod] .idea/ diff --git a/.report/coverage.xml b/.report/coverage.xml deleted file mode 100644 index 1d38850c..00000000 --- a/.report/coverage.xml +++ /dev/null @@ -1,563 +0,0 @@ - - - - - - /home/ubuntu/operator-libs-linux - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - From 0d1a984b85a2f56de0f5aedfdfc5d8c29dc8401c Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Wed, 4 Jun 2025 09:30:57 +0800 Subject: [PATCH 6/7] chore: add annotation to test and fix linting --- tests/unit/test_snap.py | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index 2941e075..3c9fa98d 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -3,6 +3,8 @@ # pyright: reportPrivateUsage=false +from __future__ import annotations + import datetime import io import json @@ -10,7 +12,7 @@ import typing import unittest from subprocess import CalledProcessError -from typing import Any, Dict, Iterable, Optional +from typing import Any, Iterable from unittest.mock import MagicMock, mock_open, patch import fake_snapd as fake_snapd @@ -632,7 +634,9 @@ def test_services_property(self, patched): ('strict', True, ['--classic']), ], ) -def test_refresh_classic(mock_subprocess: MagicMock, confinement, classic, expected_flag): +def test_refresh_classic( + mock_subprocess: MagicMock, confinement: str, classic: bool, expected_flag: list[str] +): """Test that ensure and _refresh add the --classic flag with confinement set to classic.""" foo = snap.Snap( name='foo', @@ -650,9 +654,9 @@ def test_refresh_classic(mock_subprocess: MagicMock, confinement, classic, expec 'refresh', 'foo', '--revision="2"', - ] - + expected_flag - + ['--cohort="A"'], + *expected_flag, + '--cohort="A"', + ], text=True, ) @@ -743,8 +747,8 @@ def test_wait_changes(self): def _request_raw( method: str, path: str, - query: Dict = None, - headers: Dict = None, + query: dict = None, + headers: dict = None, data: bytes = None, ) -> typing.IO[bytes]: nonlocal change_finished @@ -854,8 +858,8 @@ def test_wait_failed(self): def _request_raw( method: str, path: str, - query: Dict = None, - headers: Dict = None, + query: dict = None, + headers: dict = None, data: bytes = None, ) -> typing.IO[bytes]: if method == "PUT" and path == "snaps/test/conf": @@ -1048,7 +1052,7 @@ 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: + def fake_snap(command: str, optargs: Iterable[str] | None) -> str: """Snap._snap would normally call subprocess.check_output(["snap", ...], ...). Here we only handle the "get" commands generated by Snap.get: @@ -1077,7 +1081,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: Dict[str, Any] = { + keys_and_values: dict[str, Any] = { "key_w_string_value": "string", "key_w_float_value": 4.2, "key_w_int_value": 13, From e14de58165f41239d4ddf8b2391d4e4c4257b734 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Wed, 4 Jun 2025 09:31:16 +0800 Subject: [PATCH 7/7] chore: bump patch version for snap --- lib/charms/operator_libs_linux/v2/snap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/charms/operator_libs_linux/v2/snap.py b/lib/charms/operator_libs_linux/v2/snap.py index 61f9bb85..3623d1db 100644 --- a/lib/charms/operator_libs_linux/v2/snap.py +++ b/lib/charms/operator_libs_linux/v2/snap.py @@ -102,7 +102,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 10 +LIBPATCH = 11 # Regex to locate 7-bit C1 ANSI sequences