Merge ~woutervb/various/+git/snapstore-proxy-charm:status_action_test_refactor into ~woutervb/various/+git/snapstore-proxy-charm:main

[Wouter van Bommel]

Added status action and refactored test

Refactored test of the charm in logical subsets, to ease expansion and
keep grouped tests together.

Added an status action, that will return the result of the proxy status
command.

[Przemysław Suliga (suligap)]

One comment

[Wouter van Bommel (woutervb)]

Added a reply.
Again, did a rebase etc to things make sense

[Przemysław Suliga (suligap)]

Thanks, +1 with a reply to the comment.

review: Approve

diff --git a/Makefile b/Makefile
index 24a72d4..bcd2827 100644
--- a/Makefile
+++ b/Makefile
@@ -8,7 +8,7 @@ CHARMCRAFT = $(shell which charmcraft)
 # Releases should match the run-on section in charmcraft.yaml in the same order
 RELEASES = 20.04 18.04
 CHARM_FILENAME = $(CHARM_NAME)$(subst $(noop) $(noop),,$(addsuffix -amd64, $(addprefix _ubuntu-, $(RELEASES)))).charm
-DEPS = $(shell find src -name '*.py') requirements.txt
+DEPS = $(shell find src -name '*.py') requirements.txt actions.yaml config.yaml
 
 .PHONY: charm
 charm: $(CHARM_FILENAME)
diff --git a/README.md b/README.md
index d3157fb..7881848 100644
--- a/README.md
+++ b/README.md
@@ -6,11 +6,15 @@ This charm will allow the installation and management of the snapstore proxy cha
 
 ## Usage
 
-Minimal items that have to be set are an username / password, on ubuntu onethat correspond to an account that does not have 2fa enabled, and has accepted the developer agreement on snapstore.io.
+Minimal items that have to be set are the domain this proxy runs on, and a username / password, on ubuntu one that correspond to an account that does not have 2fa enabled, and has accepted the developer agreement on snapstore.io.
 
 
 ## Relations
 
 This charm depends on a postgresql database, so this charm should relate to a porgresql database, before anything else can be done.
 
+## Actions
 
+The charm supports the following actions:
+
+  * status - Get the status result of the proxy, use like; `juju run-action snapstore-proxy/leader status --wait`
diff --git a/actions.yaml b/actions.yaml
index 1e74ed1..d97efbd 100644
--- a/actions.yaml
+++ b/actions.yaml
@@ -2,5 +2,8 @@
 # See LICENSE file for licensing details.
 #
 # Learn more about actions at: https://juju.is/docs/sdk/actions
-
+status:
+  description: |
+    Get the status of the proxy services, ie 
+    juju run-action snapstore-proxy/leader status --wait
 
diff --git a/src/charm.py b/src/charm.py
index b82eac9..419643a 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -20,6 +20,7 @@ from helpers import all_config_options, configure_proxy, default_values
 from optionvalidation import OptionValidation
 from resource_helpers import (
     create_database,
+    get_status,
     install_from_the_charmstore,
     register_store,
     snap_details,
@@ -58,6 +59,9 @@ class SnapstoreProxyCharm(CharmBase):
             self.db.on.database_relation_broken, self._on_database_relation_broken
         )
 
+        # Actions
+        self.framework.observe(self.on.status_action, self._on_status_action)
+
     def _on_config_changed(self, event):
         """Handle update on the configuration of the proxy.
 
@@ -303,6 +307,13 @@ class SnapstoreProxyCharm(CharmBase):
 
         self.evaluate_status()
 
+    def _on_status_action(self, event):
+        output, exitcode = get_status()
+        if exitcode == 0:
+            event.set_results({"result": output})
+        else:
+            event.fail(f"Failed to get status, errorcode {exitcode}\n, result {output}")
+
 
 if __name__ == "__main__":
     main(SnapstoreProxyCharm)
diff --git a/src/resource_helpers.py b/src/resource_helpers.py
index 09cddc9..e5e9e76 100644
--- a/src/resource_helpers.py
+++ b/src/resource_helpers.py
@@ -45,3 +45,10 @@ def register_store(username, password):
 def create_database(uri):
     """Let the proxy configure the database."""
     run(["/snap/bin/snap-store-proxy", "create-database", uri])
+
+
+def get_status():
+    """Return the status result and exitcode"""
+    result = run(["/snap/bin/snap-store-proxy", "status"], capture_output=True)
+
+    return result.stdout.decode("utf-8"), result.returncode
diff --git a/tests/test_charm.py b/tests/test_charm.py
index 6f58424..f89e6a0 100644
--- a/tests/test_charm.py
+++ b/tests/test_charm.py
@@ -18,7 +18,7 @@ import exceptions
 from charm import DATABASE_NAME, SnapstoreProxyCharm
 
 
-class TestCharm:
+class CharmTest:
     def setup_method(self):
         self.harness = Harness(SnapstoreProxyCharm)
         self.harness.begin()
@@ -32,6 +32,8 @@ class TestCharm:
     def teardown_method(self):
         self.harness.cleanup()
 
+
+class TestConfig(CharmTest):
     def test_default_config_values(self):
         # Our implicit requirement is that the database uri has been set.
         self.harness.charm._stored.db_uri = "postgresql://fake/database"
@@ -156,6 +158,8 @@ class TestCharm:
         assert patched_proxy.called
         patched_proxy.assert_called_once_with(None)
 
+
+class TestStatus(CharmTest):
     def test_status_missing_db_uri(self):
         self.harness.charm._stored.db_uri = None
 
@@ -211,6 +215,8 @@ class TestCharm:
 
         assert self.harness.charm.unit.status.message == ""
 
+
+class TestEvents(CharmTest):
     def test_on_update_status(self):
         self.harness.disable_hooks()
         self.harness.set_leader(True)
@@ -304,6 +310,28 @@ class TestCharm:
         assert self.harness.charm._stored.db_uri is None
         patched_evaluate_status.assert_called_once_with()
 
+    def test_on_status_action_succeeds(self):
+        event = MagicMock()
+        with patch("charm.get_status") as patched_get_status:
+            patched_get_status.return_value = ("String", 0)
+            self.harness.charm._on_status_action(event)
+
+        patched_get_status.assert_called_once_with()
+        event.set_results.assert_called_once_with({"result": "String"})
+
+    def test_on_status_action_fails(self):
+        event = MagicMock()
+        with patch("charm.get_status") as patched_get_status:
+            patched_get_status.return_value = ("String", 1)
+            self.harness.charm._on_status_action(event)
+
+        patched_get_status.assert_called_once_with()
+        event.fail.assert_called_once_with(
+            "Failed to get status, result String, errorcode 1"
+        )
+
+
+class TestRegistration(CharmTest):
     def test_handle_registration_no_database(self, caplog):
         caplog.clear()
         caplog.set_level(logging.INFO)
@@ -397,6 +425,8 @@ class TestCharm:
             == "Failed to register, check logging to find the reason"
         )
 
+
+class TestSanity(CharmTest):
     def test_empty_username_after_registration_and_active_status(self):
         self.harness.charm._stored.registered = True
         self.harness.charm._stored.username = "Some value"
@@ -409,7 +439,7 @@ class TestCharm:
         assert self.harness.charm._stored.username == "Some value"
         assert self.harness.charm.unit.status == ActiveStatus("")
 
-    def test_empty_password(self):
+    def test_empty_password_after_registration_and_active_status(self):
         self.harness.charm._stored.registered = True
         self.harness.charm._stored.password = "Some value"
         self.harness.charm._stored.db_uri = "postgresql://user@host/database"
diff --git a/tests/test_resource_helpers.py b/tests/test_resource_helpers.py
index eaf5b22..1db2005 100644
--- a/tests/test_resource_helpers.py
+++ b/tests/test_resource_helpers.py
@@ -1,6 +1,6 @@
 import logging
 from subprocess import PIPE, CalledProcessError
-from unittest.mock import patch
+from unittest.mock import MagicMock, patch
 
 import resource_helpers
 
@@ -82,3 +82,20 @@ def test_create_database():
     patched_run.assert_called_once_with(
         ["/snap/bin/snap-store-proxy", "create-database", test_dsn]
     )
+
+
+def test_get_status():
+    result = MagicMock()
+    result.returncode = 10
+    result.stdout = "My magic result".encode("utf-8")
+
+    with patch("resource_helpers.run") as patched_run:
+        patched_run.return_value = result
+        output, exitcode = resource_helpers.get_status()
+
+    assert patched_run.called
+    patched_run.assert_called_once_with(
+        ["/snap/bin/snap-store-proxy", "status"], capture_output=True
+    )
+    assert exitcode == 10
+    assert output == "My magic result"