From 1bd9618d76bd760f89e279841ebe7b8bc6ca834c Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Tue, 30 Apr 2024 11:15:25 +0000 Subject: [PATCH] review suggestions --- tests/unit/conftest.py | 17 +++++++++++++++++ tests/unit/test_charm.py | 20 +++----------------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 057868fc71..3a4bb9d718 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -12,3 +12,20 @@ def _has_secrets(request, monkeypatch): monkeypatch.setattr("charm.JujuVersion.has_secrets", PropertyMock(return_value=request.param)) return request.param + + +@pytest.fixture +def only_with_juju_secrets(_has_secrets): + """Pretty way to skip Juju 3 tests.""" + if not _has_secrets: + pytest.skip("Secrets test only applies on Juju 3.x") + + +@pytest.fixture +def only_without_juju_secrets(_has_secrets): + """Pretty way to skip Juju 2-specific tests. + + Typically: to save CI time, when the same check were executed in a Juju 3-specific way already + """ + if _has_secrets: + pytest.skip("Skipping legacy secrets tests") diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 3a81274ba0..87ab7d4293 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1785,11 +1785,7 @@ def test_get_secret_secrets(harness, scope, field): @patch_network_get(private_address="1.1.1.1") -def test_set_secret_in_databag(harness, _has_secrets): - # As this test set secrets using set_secret and checks if they end up - # inside the relation databag, this is meant to run on juju2 only - if _has_secrets: - return +def test_set_secret_in_databag(harness, only_without_juju_secrets): with patch("charm.PostgresqlOperatorCharm._on_leader_elected"): rel_id = harness.model.get_relation(PEER).id harness.set_leader() @@ -1929,16 +1925,11 @@ def test_delete_password(harness, _has_secrets, caplog): @pytest.mark.parametrize("scope,is_leader", [("app", True), ("unit", True), ("unit", False)]) @patch_network_get(private_address="1.1.1.1") -def test_migration_from_databag(harness, _has_secrets, scope, is_leader): +def test_migration_from_databag(harness, only_with_juju_secrets, scope, is_leader): """Check if we're moving on to use secrets when live upgrade from databag to Secrets usage.""" with ( patch("charm.PostgresqlOperatorCharm._on_leader_elected"), ): - # as this test checks for a migration from databag to secrets, - # there's no need for this test when secrets are not enabled. - if not _has_secrets: - return - rel_id = harness.model.get_relation(PEER).id # App has to be leader, unit can be either harness.set_leader(is_leader) @@ -1959,16 +1950,11 @@ def test_migration_from_databag(harness, _has_secrets, scope, is_leader): @pytest.mark.parametrize("scope,is_leader", [("app", True), ("unit", True), ("unit", False)]) @patch_network_get(private_address="1.1.1.1") -def test_migration_from_single_secret(harness, _has_secrets, scope, is_leader): +def test_migration_from_single_secret(harness, only_with_juju_secrets, scope, is_leader): """Check if we're moving on to use secrets when live upgrade from databag to Secrets usage.""" with ( patch("charm.PostgresqlOperatorCharm._on_leader_elected"), ): - # as this test checks for a migration from databag to secrets, - # there's no need for this test when secrets are not enabled. - if not _has_secrets: - return - rel_id = harness.model.get_relation(PEER).id # App has to be leader, unit can be either