From b62e65535c59cccf798ebed5c49b02f5a2543382 Mon Sep 17 00:00:00 2001 From: arturo-seijas <102022572+arturo-seijas@users.noreply.github.com> Date: Wed, 29 Nov 2023 10:51:44 +0100 Subject: [PATCH] Refactor unit tests (#25) --- tests/unit/test_charm.py | 191 +++++++++++++++----------------- tests/unit/test_library_smtp.py | 122 ++++++++++---------- 2 files changed, 153 insertions(+), 160 deletions(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 533440d..1ac9a4a 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -2,7 +2,8 @@ # See LICENSE file for licensing details. """Unit tests.""" -# pylint: disable=protected-access + +import secrets from unittest.mock import MagicMock, patch import ops @@ -10,11 +11,20 @@ from charm import SmtpIntegratorOperatorCharm +MINIMAL_CHARM_CONFIG = { + "host": "smtp.example", + "port": 25, +} +MINIMAL_CHARM_CONFIG_WITH_PASSWORD = { + **MINIMAL_CHARM_CONFIG, + "password": secrets.token_hex(), +} + def test_unconfigured_charm_reaches_blocked_status(): """ - arrange: set up a charm. - act: trigger a configuration change missing required configs. + arrange: set up a charm without configuration. + act: none assert: the charm reaches BlockedStatus. """ harness = Harness(SmtpIntegratorOperatorCharm) @@ -24,8 +34,8 @@ def test_unconfigured_charm_reaches_blocked_status(): def test_misconfigured_port_charm_reaches_blocked_status(): """ - arrange: set up a charm. - act: trigger a configuration change with an invalid port. + arrange: set up a charm with an invalid port. + act: none assert: the charm reaches BlockedStatus. """ harness = Harness(SmtpIntegratorOperatorCharm) @@ -41,15 +51,14 @@ def test_misconfigured_port_charm_reaches_blocked_status(): def test_misconfigured_auth_type_charm_reaches_blocked_status(): """ - arrange: set up a charm. - act: trigger a configuration change with an invalid auth type. + arrange: set up a charm ith an invalid auth type. + act: none assert: the charm reaches BlockedStatus. """ harness = Harness(SmtpIntegratorOperatorCharm) harness.update_config( { - "host": "smtp.example", - "port": 25, + **MINIMAL_CHARM_CONFIG, "auth_type": "nonexisting", } ) @@ -59,15 +68,14 @@ def test_misconfigured_auth_type_charm_reaches_blocked_status(): def test_misconfigured_transport_security_charm_reaches_blocked_status(): """ - arrange: set up a charm. - act: trigger a configuration change with an invalid transport security. + arrange: set up a charm with an invalid transport security. + act: none assert: the charm reaches BlockedStatus. """ harness = Harness(SmtpIntegratorOperatorCharm) harness.update_config( { - "host": "smtp.example", - "port": 25, + **MINIMAL_CHARM_CONFIG, "transport_security": "nonexisting", } ) @@ -77,100 +85,100 @@ def test_misconfigured_transport_security_charm_reaches_blocked_status(): def test_charm_reaches_active_status(): """ - arrange: set up a charm. + arrange: set up a charm with minimal valid configuration. act: trigger a configuration change with the required configs. assert: the charm reaches ActiveStatus. """ harness = Harness(SmtpIntegratorOperatorCharm) - harness.update_config( - { - "host": "smtp.example", - "port": 25, - } - ) + harness.update_config(MINIMAL_CHARM_CONFIG) harness.begin() harness.charm.on.config_changed.emit() assert harness.model.unit.status == ops.ActiveStatus() -def test_legacy_relation_joined_when_leader(): +def test_legacy_relation_joined_populates_data(): """ - arrange: set up a configured charm and set leadership for the unit. - act: add a relation. - assert: the relation gets populated with the SMTP data. + arrange: set up a charm with valid configuration and leadership for the unit. + act: add an smtp-legacy relation. + assert: the smtp-legacy relation gets populated with the SMTP data. """ harness = Harness(SmtpIntegratorOperatorCharm) harness.set_leader(True) - host = "smtp.example" - port = 25 - password = "somepassword" # nosec - harness.update_config( - { - "host": host, - "port": port, - "password": password, - } - ) + harness.update_config(MINIMAL_CHARM_CONFIG_WITH_PASSWORD) + harness.begin() + harness.charm.on.config_changed.emit() + harness.add_relation("smtp-legacy", "example") + data = harness.model.get_relation("smtp-legacy").data[harness.model.app] + assert data["host"] == MINIMAL_CHARM_CONFIG_WITH_PASSWORD["host"] + assert data["port"] == str(MINIMAL_CHARM_CONFIG_WITH_PASSWORD["port"]) + assert data["password"] == MINIMAL_CHARM_CONFIG_WITH_PASSWORD["password"] + + +def test_legacy_relation_joined_doesnt_populate_password_id(): + """ + arrange: set up a charm with valid configuration and leadership for the unit. + act: add an smtp-legacy relation. + assert: the smtp-legacy relation does not get populated with the password_id. + """ + harness = Harness(SmtpIntegratorOperatorCharm) + harness.set_leader(True) + harness.update_config(MINIMAL_CHARM_CONFIG_WITH_PASSWORD) harness.begin() harness.charm.on.config_changed.emit() harness.add_relation("smtp-legacy", "example") data = harness.model.get_relation("smtp-legacy").data[harness.model.app] - assert data["host"] == host - assert data["port"] == str(port) - assert data["password"] == password assert "password_id" not in data @patch.object(ops.JujuVersion, "from_environ") -def test_relation_joined_when_leader_and_secrets(mock_juju_env): +def test_relation_joined_when_secrets_enabled_populates_data(mock_juju_env): """ - arrange: set up a configured charm mimicking Juju 3 and set leadership for the unit. - act: add a relation. - assert: the relation gets populated with the SMTP data. + arrange: set up a charm with valid configuration mimicking Juju 3 and leadership for the unit. + act: add an smtp relation. + assert: the smtp relation gets populated with the SMTP data. """ mock_juju_env.return_value = MagicMock(has_secrets=True) harness = Harness(SmtpIntegratorOperatorCharm) harness.set_leader(True) - host = "smtp.example" - port = 25 - password = "somepassword" # nosec - harness.update_config( - { - "host": host, - "port": port, - "password": password, - } - ) + harness.update_config(MINIMAL_CHARM_CONFIG_WITH_PASSWORD) harness.begin() harness.charm.on.config_changed.emit() harness.add_relation("smtp", "example") data = harness.model.get_relation("smtp").data[harness.model.app] - assert data["host"] == host - assert data["port"] == str(port) - assert "password" not in data + assert data["host"] == MINIMAL_CHARM_CONFIG_WITH_PASSWORD["host"] + assert data["port"] == str(MINIMAL_CHARM_CONFIG_WITH_PASSWORD["port"]) assert data["password_id"] is not None @patch.object(ops.JujuVersion, "from_environ") -def test_relation_joined_when_leader_and_no_secrets(mock_juju_env): +def test_relation_joined_when_secrets_enabled_doesnt_populate_password(mock_juju_env): """ - arrange: set up a configured charm mimicking Juju 2 and set leadership for the unit. - act: add a relation. - assert: the relation does not get populated with the SMTP data. + arrange: set up a charm with valid configuration mimicking Juju 3 and leadership for the unit. + act: add an smtp relation. + assert: the smtp relation does not populate with the password. + """ + mock_juju_env.return_value = MagicMock(has_secrets=True) + harness = Harness(SmtpIntegratorOperatorCharm) + harness.set_leader(True) + harness.update_config(MINIMAL_CHARM_CONFIG_WITH_PASSWORD) + harness.begin() + harness.charm.on.config_changed.emit() + harness.add_relation("smtp", "example") + data = harness.model.get_relation("smtp").data[harness.model.app] + assert "password" not in data + + +@patch.object(ops.JujuVersion, "from_environ") +def test_relation_joined_when_no_secrets_enabled(mock_juju_env): + """ + arrange: set up a charm with valid configuration mimicking Juju 2 and leadership for the unit. + act: add an smtp relation. + assert: the smtp relation does not get populated with the SMTP data. """ mock_juju_env.return_value = MagicMock(has_secrets=False) harness = Harness(SmtpIntegratorOperatorCharm) harness.set_leader(True) - host = "smtp.example" - port = 25 - password = "somepassword" # nosec - harness.update_config( - { - "host": host, - "port": port, - "password": password, - } - ) + harness.update_config(MINIMAL_CHARM_CONFIG_WITH_PASSWORD) harness.begin() harness.charm.on.config_changed.emit() harness.add_relation("smtp", "example") @@ -179,29 +187,22 @@ def test_relation_joined_when_leader_and_no_secrets(mock_juju_env): @patch.object(ops.JujuVersion, "from_environ") -def test_relation_joined_when_leader_and_no_password(mock_juju_env): +def test_relation_joined_when_no_password_configured(mock_juju_env): """ - arrange: set up a configured charm mimicking Juju 3 and set leadership for the unit. - act: add a relation. - assert: the relation gets populated with the SMTP data. + arrange: set up a configured charm mimicking Juju 3 and leadership for the unit. + act: add an smtp relation. + assert: the relation gets populated with the SMTP data and the password_id is not present. """ mock_juju_env.return_value = MagicMock(has_secrets=True) harness = Harness(SmtpIntegratorOperatorCharm) harness.set_leader(True) - host = "smtp.example" - port = 25 - harness.update_config( - { - "host": host, - "port": port, - } - ) + harness.update_config(MINIMAL_CHARM_CONFIG) harness.begin() harness.charm.on.config_changed.emit() harness.add_relation("smtp", "example") data = harness.model.get_relation("smtp").data[harness.model.app] - assert data["host"] == host - assert data["port"] == str(port) + assert data["host"] == MINIMAL_CHARM_CONFIG["host"] + assert data["port"] == str(MINIMAL_CHARM_CONFIG["port"]) assert "password" not in data assert "password_id" not in data @@ -209,19 +210,12 @@ def test_relation_joined_when_leader_and_no_password(mock_juju_env): def test_legacy_relation_joined_when_not_leader(): """ arrange: set up a charm mimicking Juju 3 and unset leadership for the unit. - act: add a relation. - assert: the relation does not get populated with the SMTP data. + act: add an smtp-legacy relation. + assert: the smtp-legacy relation does not get populated with the SMTP data. """ harness = Harness(SmtpIntegratorOperatorCharm) harness.set_leader(False) - host = "smtp.example" - port = 25 - harness.update_config( - { - "host": host, - "port": port, - } - ) + harness.update_config(MINIMAL_CHARM_CONFIG) harness.begin() harness.charm.on.config_changed.emit() harness.add_relation("smtp-legacy", "example") @@ -233,20 +227,13 @@ def test_legacy_relation_joined_when_not_leader(): def test_relation_joined_when_not_leader(mock_juju_env): """ arrange: set up a charm mimicking Juju 3 and unset leadership for the unit. - act: add a relation. - assert: the relation does not get populated with the SMTP data. + act: add an smtp relation. + assert: the smtp relation does not get populated with the SMTP data. """ mock_juju_env.return_value = MagicMock(has_secrets=True) harness = Harness(SmtpIntegratorOperatorCharm) harness.set_leader(False) - host = "smtp.example" - port = 25 - harness.update_config( - { - "host": host, - "port": port, - } - ) + harness.update_config(MINIMAL_CHARM_CONFIG) harness.begin() harness.charm.on.config_changed.emit() harness.add_relation("smtp", "example") diff --git a/tests/unit/test_library_smtp.py b/tests/unit/test_library_smtp.py index 7f3d558..44fbf4b 100644 --- a/tests/unit/test_library_smtp.py +++ b/tests/unit/test_library_smtp.py @@ -2,6 +2,8 @@ # See LICENSE file for licensing details. """SMTP library unit tests""" +import secrets + import ops import pytest from charms.smtp_integrator.v0 import smtp @@ -25,6 +27,24 @@ interface: smtp """ +RELATION_DATA = { + "host": "example.smtp", + "port": "25", + "user": "example_user", + "auth_type": "plain", + "transport_security": "tls", + "domain": "domain", +} + +SAMPLE_LEGACY_RELATION_DATA = { + **RELATION_DATA, + "password": secrets.token_hex(), +} +SAMPLE_RELATION_DATA = { + **RELATION_DATA, + "password_id": secrets.token_hex(), +} + class SmtpRequirerCharm(ops.CharmBase): """Class for requirer charm testing.""" @@ -78,7 +98,7 @@ def _record_event(self, event: ops.EventBase) -> None: def test_smtp_provider_charm_relations(): """ - arrange: instantiate a SmtpProviderCharm and add a relation. + arrange: instantiate a SmtpProviderCharm and add an smtp-legacy relation. act: obtain the relations. assert: the relations retrieved match the existing relations. """ @@ -92,7 +112,7 @@ def test_smtp_provider_charm_relations(): def test_smtp_provider_update_relation_data(): """ - arrange: instantiate a SmtpProviderCharm object and add a relation. + arrange: instantiate a SmtpProviderCharm object and add an smtp-legacy relation. act: update the relation data. assert: the relation data is updated. """ @@ -125,8 +145,8 @@ def test_smtp_relation_data_to_relation_data(): host="example.smtp", port=25, user="example_user", - password="somepassword", # nosec - password_id="someid", + password=secrets.token_hex(), + password_id=secrets.token_hex(), auth_type="plain", transport_security="tls", domain="domain", @@ -136,8 +156,8 @@ def test_smtp_relation_data_to_relation_data(): "host": "example.smtp", "port": "25", "user": "example_user", - "password": "somepassword", # nosec - "password_id": "someid", + "password": smtp_data.password, + "password_id": smtp_data.password_id, "auth_type": "plain", "transport_security": "tls", "domain": "domain", @@ -148,7 +168,7 @@ def test_smtp_relation_data_to_relation_data(): def test_legacy_requirer_charm_does_not_emit_event_id_when_no_data(): """ arrange: set up a charm with no relation data to be populated. - act: trigger a relation changed event. + act: add an smtp-legacy relation. assert: no events are emitted. """ harness = Harness(SmtpRequirerCharm, meta=REQUIRER_METADATA) @@ -163,8 +183,8 @@ def test_legacy_requirer_charm_does_not_emit_event_id_when_no_data(): def test_requirer_charm_does_not_emit_event_id_when_no_data(): """ arrange: set up a charm with no relation data to be populated. - act: trigger a relation changed event. - assert: no events are emitted. + act: add an smtp relation. + assert: no SmtpDataAvailable events are emitted. """ harness = Harness(SmtpRequirerCharm, meta=REQUIRER_METADATA) harness.begin() @@ -179,81 +199,67 @@ def test_requirer_charm_does_not_emit_event_id_when_no_data(): def test_legacy_requirer_charm_with_valid_relation_data_emits_event(is_leader): """ arrange: set up a charm. - act: trigger a relation changed event with valid data. - assert: a event containing the relation data is emitted. + act: add an smtp-legacy relation. + assert: an SmtpDataAvailable event containing the relation data is emitted. """ - relation_data = { - "host": "example.smtp", - "port": "25", - "user": "example_user", - "password": "somepassword", # nosec - "auth_type": "plain", - "transport_security": "tls", - "domain": "domain", - } - harness = Harness(SmtpRequirerCharm, meta=REQUIRER_METADATA) harness.begin() harness.set_leader(is_leader) - harness.add_relation("smtp-legacy", "smtp-provider", app_data=relation_data) + harness.add_relation("smtp-legacy", "smtp-provider", app_data=SAMPLE_LEGACY_RELATION_DATA) assert len(harness.charm.events) == 1 - assert harness.charm.events[0].host == relation_data["host"] - assert harness.charm.events[0].port == int(relation_data["port"]) - assert harness.charm.events[0].user == relation_data["user"] - assert harness.charm.events[0].password == relation_data["password"] - assert harness.charm.events[0].auth_type == relation_data["auth_type"] - assert harness.charm.events[0].transport_security == relation_data["transport_security"] - assert harness.charm.events[0].domain == relation_data["domain"] + assert harness.charm.events[0].host == SAMPLE_LEGACY_RELATION_DATA["host"] + assert harness.charm.events[0].port == int(SAMPLE_LEGACY_RELATION_DATA["port"]) + assert harness.charm.events[0].user == SAMPLE_LEGACY_RELATION_DATA["user"] + assert harness.charm.events[0].password == SAMPLE_LEGACY_RELATION_DATA["password"] + assert harness.charm.events[0].auth_type == SAMPLE_LEGACY_RELATION_DATA["auth_type"] + assert ( + harness.charm.events[0].transport_security + == SAMPLE_LEGACY_RELATION_DATA["transport_security"] + ) + assert harness.charm.events[0].domain == SAMPLE_LEGACY_RELATION_DATA["domain"] retrieved_relation_data = harness.charm.smtp_legacy.get_relation_data() - assert retrieved_relation_data.host == relation_data["host"] - assert retrieved_relation_data.port == int(relation_data["port"]) - assert retrieved_relation_data.user == relation_data["user"] - assert retrieved_relation_data.password == relation_data["password"] - assert retrieved_relation_data.auth_type == relation_data["auth_type"] - assert retrieved_relation_data.transport_security == relation_data["transport_security"] - assert retrieved_relation_data.domain == relation_data["domain"] + assert retrieved_relation_data.host == SAMPLE_LEGACY_RELATION_DATA["host"] + assert retrieved_relation_data.port == int(SAMPLE_LEGACY_RELATION_DATA["port"]) + assert retrieved_relation_data.user == SAMPLE_LEGACY_RELATION_DATA["user"] + assert retrieved_relation_data.password == SAMPLE_LEGACY_RELATION_DATA["password"] + assert retrieved_relation_data.auth_type == SAMPLE_LEGACY_RELATION_DATA["auth_type"] + assert ( + retrieved_relation_data.transport_security + == SAMPLE_LEGACY_RELATION_DATA["transport_security"] + ) + assert retrieved_relation_data.domain == SAMPLE_LEGACY_RELATION_DATA["domain"] @pytest.mark.parametrize("is_leader", [True, False]) def test_requirer_charm_with_valid_relation_data_emits_event(is_leader): """ arrange: set up a charm. - act: trigger a relation changed event with valid data. - assert: a event containing the relation data is emitted. + act: add an smtp relation. + assert: an SmtpDataAvailable event containing the relation data is emitted. """ - relation_data = { - "host": "example.smtp", - "port": "25", - "user": "example_user", - "password_id": "someid", - "auth_type": "plain", - "transport_security": "tls", - "domain": "domain", - } - harness = Harness(SmtpRequirerCharm, meta=REQUIRER_METADATA) harness.begin() harness.set_leader(is_leader) - harness.add_relation("smtp", "smtp-provider", app_data=relation_data) + harness.add_relation("smtp", "smtp-provider", app_data=SAMPLE_RELATION_DATA) assert len(harness.charm.events) == 1 - assert harness.charm.events[0].host == relation_data["host"] - assert harness.charm.events[0].port == int(relation_data["port"]) - assert harness.charm.events[0].user == relation_data["user"] - assert harness.charm.events[0].password_id == relation_data["password_id"] - assert harness.charm.events[0].auth_type == relation_data["auth_type"] - assert harness.charm.events[0].transport_security == relation_data["transport_security"] - assert harness.charm.events[0].domain == relation_data["domain"] + assert harness.charm.events[0].host == SAMPLE_RELATION_DATA["host"] + assert harness.charm.events[0].port == int(SAMPLE_RELATION_DATA["port"]) + assert harness.charm.events[0].user == SAMPLE_RELATION_DATA["user"] + assert harness.charm.events[0].password_id == SAMPLE_RELATION_DATA["password_id"] + assert harness.charm.events[0].auth_type == SAMPLE_RELATION_DATA["auth_type"] + assert harness.charm.events[0].transport_security == SAMPLE_RELATION_DATA["transport_security"] + assert harness.charm.events[0].domain == SAMPLE_RELATION_DATA["domain"] @pytest.mark.parametrize("is_leader", [True, False]) def test_requirer_charm_with_invalid_relation_data_doesnt_emit_event(is_leader): """ arrange: set up a charm. - act: trigger a relation changed event with invalid data. - assert: an event containing the relation data is not emitted. + act: add an smtp-legacy relation changed event with invalid data. + assert: an SmtpDataAvailable event is not emitted. """ relation_data = { "port": "25",