From 7fe0937190c8636d12103442d7c1981f7678807c Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Thu, 16 Jul 2020 13:16:38 +0200 Subject: [PATCH] Using real objects is considered polite Previously, ToddlerBase and derived classes were used directly, but none of the methods is marked as a class or static method. Amazingly enough, it worked regardless. It's useful to have a constructor, though, so do things conventionally. As we can't just monkey-patch the classes then, use the shared `toddler` fixture which creates the object for the test methods that need it. Signed-off-by: Nils Philippsen --- tests/conftest.py | 17 +++ tests/plugins/test_debug.py | 15 +-- tests/plugins/test_flag_ci_pr.py | 57 ++++------ tests/plugins/test_flag_commit_build.py | 108 ++++++------------- tests/plugins/test_packager_bugzilla_sync.py | 50 ++++----- tests/plugins/test_pdc_retired_packages.py | 88 ++++++--------- tests/test_base.py | 21 ++-- toddlers/base.py | 8 +- toddlers/plugins/debug.py | 4 +- toddlers/plugins/flag_ci_pr.py | 4 +- toddlers/plugins/flag_commit_build.py | 4 +- toddlers/plugins/packager_bugzilla_sync.py | 6 +- toddlers/plugins/pdc_retired_packages.py | 8 +- toddlers/runner.py | 2 +- 14 files changed, 167 insertions(+), 225 deletions(-) create mode 100644 tests/conftest.py diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..037630e --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,17 @@ +import pytest + + +@pytest.fixture +def toddler(request, monkeypatch): + """Fixture creating a toddler for a class testing it + + The test class must set the toddler class in its `toddler_cls` attribute. + """ + test_cls = request.cls + if not hasattr(test_cls, "toddler_cls"): + raise RuntimeError(f"{test_cls} needs to set `toddler_cls`") + + toddler_cls = test_cls.toddler_cls + + toddler_obj = toddler_cls() + return toddler_obj diff --git a/tests/plugins/test_debug.py b/tests/plugins/test_debug.py index 9825e91..97f35a5 100644 --- a/tests/plugins/test_debug.py +++ b/tests/plugins/test_debug.py @@ -1,16 +1,17 @@ import fedora_messaging.api -import toddlers.plugins.debug +from toddlers.plugins.debug import DebugToddler class TestDebugToddler: - def test_accepts_topic(self): - assert toddlers.plugins.debug.DebugToddler.accepts_topic("foo.bar") - def test_process(self): + toddler_cls = DebugToddler + + def test_accepts_topic(self, toddler): + assert toddler.accepts_topic("foo.bar") + + def test_process(self, toddler): msg = fedora_messaging.api.Message() msg.id = 123 msg.topic = "toddlers.test.topic" - assert ( - toddlers.plugins.debug.DebugToddler.process(config={}, message=msg) is None - ) + assert toddler.process(config={}, message=msg) is None diff --git a/tests/plugins/test_flag_ci_pr.py b/tests/plugins/test_flag_ci_pr.py index 09fa8ed..c5c90ab 100644 --- a/tests/plugins/test_flag_ci_pr.py +++ b/tests/plugins/test_flag_ci_pr.py @@ -4,12 +4,15 @@ from unittest.mock import MagicMock, patch import fedora_messaging.api import pytest -import toddlers.plugins.flag_ci_pr +from toddlers.plugins.flag_ci_pr import FlagCIPR class TestFlagCIPRToddler: - def test_accepts_topic_invalid(self): - assert toddlers.plugins.flag_ci_pr.FlagCIPR.accepts_topic("foo.bar") is False + + toddler_cls = FlagCIPR + + def test_accepts_topic_invalid(self, toddler): + assert toddler.accepts_topic("foo.bar") is False @pytest.mark.parametrize( "topic", @@ -25,55 +28,47 @@ class TestFlagCIPRToddler: "org.centos.stg.ci.dist-git-pr.test.running", ], ) - def test_accepts_topic_valid(self, topic): - assert toddlers.plugins.flag_ci_pr.FlagCIPR.accepts_topic(topic) + def test_accepts_topic_valid(self, toddler, topic): + assert toddler.accepts_topic(topic) - def test_process_invalid(self): + def test_process_invalid(self, toddler): msg = fedora_messaging.api.Message() msg.id = 123 msg.topic = "toddlers.test.topic" - assert ( - toddlers.plugins.flag_ci_pr.FlagCIPR.process(config={}, message=msg) is None - ) + assert toddler.process(config={}, message=msg) is None - def test_process_invalid_status(self, caplog): + def test_process_invalid_status(self, toddler, caplog): caplog.set_level(logging.INFO) msg = fedora_messaging.api.Message() msg.id = 123 msg.topic = "org.centos.stg.ci.dist-git-pr.test.invalid" - assert ( - toddlers.plugins.flag_ci_pr.FlagCIPR.process(config={}, message=msg) is None - ) + assert toddler.process(config={}, message=msg) is None assert ( caplog.records[-1].message == "Pipeline state is not 'complete' or 'running' or 'error'." ) - def test_process_no_version(self, caplog): + def test_process_no_version(self, toddler, caplog): caplog.set_level(logging.INFO) msg = fedora_messaging.api.Message() msg.id = 123 msg.topic = "org.centos.stg.ci.dist-git-pr.test.complete" - assert ( - toddlers.plugins.flag_ci_pr.FlagCIPR.process(config={}, message=msg) is None - ) + assert toddler.process(config={}, message=msg) is None assert caplog.records[-1].message == "Unsupported msg version, ignoring" - def test_process_invalid_complete_status(self, caplog): + def test_process_invalid_complete_status(self, toddler, caplog): caplog.set_level(logging.INFO) msg = fedora_messaging.api.Message() msg.id = 123 msg.topic = "org.centos.stg.ci.dist-git-pr.test.complete" msg.body = {"version": "0.2.1", "test": {"result": "invalid"}} - assert ( - toddlers.plugins.flag_ci_pr.FlagCIPR.process(config={}, message=msg) is None - ) + assert toddler.process(config={}, message=msg) is None assert ( caplog.records[-1].message == "Build is not in one of the expected status, ignoring" ) - def test_process_no_artifact(self, caplog): + def test_process_no_artifact(self, toddler, caplog): caplog.set_level(logging.INFO) msg = fedora_messaging.api.Message() msg.id = 123 @@ -83,9 +78,7 @@ class TestFlagCIPRToddler: "test": {"result": "passed"}, "artifact": {"commit_hash": "abcdefghijklmnopqrst"}, } - assert ( - toddlers.plugins.flag_ci_pr.FlagCIPR.process(config={}, message=msg) is None - ) + assert toddler.process(config={}, message=msg) is None assert ( caplog.records[-1].message == "Invalid message: {'commit_hash': 'abcdefghijklmnopqrst'}, could not extract " @@ -93,7 +86,7 @@ class TestFlagCIPRToddler: ) @patch("toddlers.plugins.flag_ci_pr.requests_session") - def test_process_request_failed(self, mock_requests, caplog): + def test_process_request_failed(self, mock_requests, toddler, caplog): mock_requests.request.return_value = MagicMock( ok=False, status_code=401, text="invalid" ) @@ -117,16 +110,13 @@ class TestFlagCIPRToddler: "pagure_token": "ahah", } - assert ( - toddlers.plugins.flag_ci_pr.FlagCIPR.process(config=config, message=msg) - == 1 - ) + assert toddler.process(config=config, message=msg) == 1 assert ( caplog.records[-1].message == "Request to https://pagure.io returned: 401" ) @patch("toddlers.plugins.flag_ci_pr.requests_session") - def test_process_valid(self, mock_requests, caplog): + def test_process_valid(self, mock_requests, toddler, caplog): mock_requests.request.return_value = MagicMock( ok=True, status_code=200, text="invalid" ) @@ -150,10 +140,7 @@ class TestFlagCIPRToddler: "pagure_token": "ahah", } - assert ( - toddlers.plugins.flag_ci_pr.FlagCIPR.process(config=config, message=msg) - is None - ) + assert toddler.process(config=config, message=msg) is None assert ( caplog.records[-3].message == "Request to https://pagure.io returned: 200" ) diff --git a/tests/plugins/test_flag_commit_build.py b/tests/plugins/test_flag_commit_build.py index 735e293..cfd3fcf 100644 --- a/tests/plugins/test_flag_commit_build.py +++ b/tests/plugins/test_flag_commit_build.py @@ -4,15 +4,15 @@ from unittest.mock import MagicMock, Mock, patch import fedora_messaging.api import pytest -import toddlers.plugins.flag_commit_build +from toddlers.plugins.flag_commit_build import FlagCommitBuild class TestFlagCommitBuildToddler: - def test_accepts_topic_invalid(self): - assert ( - toddlers.plugins.flag_commit_build.FlagCommitBuild.accepts_topic("foo.bar") - is False - ) + + toddler_cls = FlagCommitBuild + + def test_accepts_topic_invalid(self, toddler): + assert toddler.accepts_topic("foo.bar") is False @pytest.mark.parametrize( "topic", @@ -22,38 +22,28 @@ class TestFlagCommitBuildToddler: "org.fedoraproject.prod.buildsys.build.state.change", ], ) - def test_accepts_topic_valid(self, topic): - assert toddlers.plugins.flag_commit_build.FlagCommitBuild.accepts_topic(topic) + def test_accepts_topic_valid(self, toddler, topic): + assert toddler.accepts_topic(topic) - def test_process_containerbuild(self, caplog): + def test_process_containerbuild(self, toddler, caplog): caplog.set_level(logging.INFO) msg = fedora_messaging.api.Message() msg.id = 123 msg.topic = "org.fedoraproject.prod.buildsys.build.state.change" msg.body = {"owner": "containerbuild"} - assert ( - toddlers.plugins.flag_commit_build.FlagCommitBuild.process( - config={}, message=msg - ) - is None - ) + assert toddler.process(config={}, message=msg) is None assert caplog.records[-1].message == "Skipping container build" - def test_process_mbs_build(self, caplog): + def test_process_mbs_build(self, toddler, caplog): caplog.set_level(logging.INFO) msg = fedora_messaging.api.Message() msg.id = 123 msg.topic = "org.fedoraproject.prod.buildsys.build.state.change" msg.body = {"owner": "mbs/mbs.fedoraproject.org"} - assert ( - toddlers.plugins.flag_commit_build.FlagCommitBuild.process( - config={}, message=msg - ) - is None - ) + assert toddler.process(config={}, message=msg) is None assert caplog.records[-1].message == "Skipping MBS builds" - def test_process_secondary_instance(self, caplog): + def test_process_secondary_instance(self, toddler, caplog): caplog.set_level(logging.INFO) msg = fedora_messaging.api.Message() msg.id = 123 @@ -62,15 +52,10 @@ class TestFlagCommitBuildToddler: "owner": "username", "instance": "secondary", } - assert ( - toddlers.plugins.flag_commit_build.FlagCommitBuild.process( - config={}, message=msg - ) - is None - ) + assert toddler.process(config={}, message=msg) is None assert caplog.records[-1].message == "Ignoring secondary arch task..." - def test_process_uninteresting_status(self, caplog): + def test_process_uninteresting_status(self, toddler, caplog): caplog.set_level(logging.INFO) msg = fedora_messaging.api.Message() msg.id = 123 @@ -80,19 +65,14 @@ class TestFlagCommitBuildToddler: "instance": "primary", "new": 2, } - assert ( - toddlers.plugins.flag_commit_build.FlagCommitBuild.process( - config={}, message=msg - ) - is None - ) + assert toddler.process(config={}, message=msg) is None assert ( caplog.records[-1].message == "Build is not in a state we care about, skipping" ) @patch("toddlers.plugins.flag_commit_build.koji") - def test_process_no_git_url(self, mock_koji, caplog): + def test_process_no_git_url(self, mock_koji, toddler, caplog): client = Mock() client.getBuild = Mock(return_value={}) mock_koji.ClientSession = Mock(return_value=client) @@ -107,19 +87,14 @@ class TestFlagCommitBuildToddler: "build_id": 42, } config = {"koji_url": "https://koji.fedoraproject.org"} - assert ( - toddlers.plugins.flag_commit_build.FlagCommitBuild.process( - config=config, message=msg - ) - is None - ) + assert toddler.process(config=config, message=msg) is None assert ( caplog.records[-1].message == "No git url found in the extra information: None" ) @patch("toddlers.plugins.flag_commit_build.koji") - def test_process_invalid_git_url(self, mock_koji, caplog): + def test_process_invalid_git_url(self, mock_koji, toddler, caplog): client = Mock() client.getBuild = Mock( return_value={"extra": {"source": {"original_url": "foobar"}}} @@ -136,17 +111,12 @@ class TestFlagCommitBuildToddler: "build_id": 42, } config = {"koji_url": "https://koji.fedoraproject.org"} - assert ( - toddlers.plugins.flag_commit_build.FlagCommitBuild.process( - config=config, message=msg - ) - is None - ) + assert toddler.process(config=config, message=msg) is None assert caplog.records[-1].message == "No # in the git_url: foobar" @patch("toddlers.plugins.flag_commit_build.requests_session") @patch("toddlers.plugins.flag_commit_build.koji") - def test_process_flag_no_task_id(self, mock_koji, mock_requests, caplog): + def test_process_flag_no_task_id(self, mock_koji, mock_requests, toddler, caplog): mock_requests.request.return_value = MagicMock( ok=False, status_code=401, text="invalid" ) @@ -182,12 +152,7 @@ class TestFlagCommitBuildToddler: "pagure_url": "https://src.fedoraproject.org", "pagure_token": "ahah", } - assert ( - toddlers.plugins.flag_commit_build.FlagCommitBuild.process( - config=config, message=msg - ) - is None - ) + assert toddler.process(config=config, message=msg) is None assert ( caplog.records[-1].message == "Request to https://src.fedoraproject.org returned: 401" @@ -195,7 +160,7 @@ class TestFlagCommitBuildToddler: @patch("toddlers.plugins.flag_commit_build.requests_session") @patch("toddlers.plugins.flag_commit_build.koji") - def test_process_flag_failed(self, mock_koji, mock_requests, caplog): + def test_process_flag_failed(self, mock_koji, mock_requests, toddler, caplog): mock_requests.request.return_value = MagicMock( ok=False, status_code=401, text="invalid" ) @@ -231,12 +196,7 @@ class TestFlagCommitBuildToddler: "pagure_url": "https://src.fedoraproject.org", "pagure_token": "ahah", } - assert ( - toddlers.plugins.flag_commit_build.FlagCommitBuild.process( - config=config, message=msg - ) - is None - ) + assert toddler.process(config=config, message=msg) is None assert ( caplog.records[-1].message == "Request to https://src.fedoraproject.org returned: 401" @@ -244,7 +204,9 @@ class TestFlagCommitBuildToddler: @patch("toddlers.plugins.flag_commit_build.requests_session") @patch("toddlers.plugins.flag_commit_build.koji") - def test_process_flag_failed_cancel(self, mock_koji, mock_requests, caplog): + def test_process_flag_failed_cancel( + self, mock_koji, mock_requests, toddler, caplog + ): mock_requests.request.return_value = MagicMock( ok=False, status_code=401, text="invalid" ) @@ -280,12 +242,7 @@ class TestFlagCommitBuildToddler: "pagure_url": "https://src.fedoraproject.org", "pagure_token": "ahah", } - assert ( - toddlers.plugins.flag_commit_build.FlagCommitBuild.process( - config=config, message=msg - ) - is None - ) + assert toddler.process(config=config, message=msg) is None assert ( caplog.records[-1].message == "Request to https://src.fedoraproject.org returned: 401" @@ -293,7 +250,7 @@ class TestFlagCommitBuildToddler: @patch("toddlers.plugins.flag_commit_build.requests_session") @patch("toddlers.plugins.flag_commit_build.koji") - def test_process_flag_pass(self, mock_koji, mock_requests, caplog): + def test_process_flag_pass(self, mock_koji, mock_requests, toddler, caplog): mock_requests.request.return_value = MagicMock( ok=True, status_code=200, text="woohoo!" ) @@ -328,12 +285,7 @@ class TestFlagCommitBuildToddler: "pagure_url": "https://src.fedoraproject.org", "pagure_token": "ahah", } - assert ( - toddlers.plugins.flag_commit_build.FlagCommitBuild.process( - config=config, message=msg - ) - is None - ) + assert toddler.process(config=config, message=msg) is None assert ( caplog.records[-3].message == "Request to https://src.fedoraproject.org returned: 200" diff --git a/tests/plugins/test_packager_bugzilla_sync.py b/tests/plugins/test_packager_bugzilla_sync.py index 84ac134..5b042d8 100644 --- a/tests/plugins/test_packager_bugzilla_sync.py +++ b/tests/plugins/test_packager_bugzilla_sync.py @@ -2,17 +2,15 @@ from unittest.mock import Mock, patch import pytest -import toddlers.plugins.packager_bugzilla_sync +from toddlers.plugins.packager_bugzilla_sync import main, PackagerBugzillaSync class TestPackagerBugzillaSyncToddler: - def test_accepts_topic_invalid(self): - assert ( - toddlers.plugins.packager_bugzilla_sync.PackagerBugzillaSync.accepts_topic( - "foo.bar" - ) - is False - ) + + toddler_cls = PackagerBugzillaSync + + def test_accepts_topic_invalid(self, toddler): + assert toddler.accepts_topic("foo.bar") is False @pytest.mark.parametrize( "topic", @@ -22,26 +20,22 @@ class TestPackagerBugzillaSyncToddler: "org.fedoraproject.stg.toddlers.trigger.packager_bugzilla_sync", ], ) - def test_accepts_topic_valid(self, topic): - assert toddlers.plugins.packager_bugzilla_sync.PackagerBugzillaSync.accepts_topic( - topic - ) + def test_accepts_topic_valid(self, toddler, topic): + assert toddler.accepts_topic(topic) - def test_process_no_email_override(self, capsys): + def test_process_no_email_override(self, toddler, capsys): with pytest.raises(KeyError, match=r"'email_overrides_file'"): - toddlers.plugins.packager_bugzilla_sync.PackagerBugzillaSync.process( - config={}, message=None, username=False, dry_run=True - ) + toddler.process(config={}, message=None, username=False, dry_run=True) out, err = capsys.readouterr() assert out == "Failed to load the file containing the email-overrides\n" assert err == "" - def test_process_no_email_override_file(self, capsys): + def test_process_no_email_override_file(self, toddler, capsys): with pytest.raises( FileNotFoundError, match=r"No such file or directory: 'test'" ): - toddlers.plugins.packager_bugzilla_sync.PackagerBugzillaSync.process( + toddler.process( config={"email_overrides_file": "test"}, message=None, username=False, @@ -60,14 +54,20 @@ class TestPackagerBugzillaSyncToddler: @patch("toddlers.utils.bugzilla_system.add_user_to_group") @patch("toml.load") def test_process( - self, toml_load, bz_user_grp, get_bz_grp_mbr, get_bz_email, get_fas_grp_mbr + self, + toml_load, + bz_user_grp, + get_bz_grp_mbr, + get_bz_email, + get_fas_grp_mbr, + toddler, ): toml_load.return_value = {} get_fas_grp_mbr.return_value = ["pingou", "nils"] get_bz_email.side_effect = ["pingou@fp.o", "nils@fp.o"] get_bz_grp_mbr.return_value = ["pingou@fp.o", "nphilipp@fp.o"] - toddlers.plugins.packager_bugzilla_sync.PackagerBugzillaSync.process( + toddler.process( config={"email_overrides_file": "test", "bugzilla_group": "fedora_contrib"}, message=None, username=False, @@ -92,13 +92,13 @@ class TestPackagerBugzillaSyncToddler: @patch("toddlers.utils.bugzilla_system.add_user_to_group") @patch("toml.load") def test_process_username( - self, toml_load, bz_user_grp, get_bz_grp_mbr, get_bz_email + self, toml_load, bz_user_grp, get_bz_grp_mbr, get_bz_email, toddler ): toml_load.return_value = {} get_bz_email.side_effect = ["nils@fp.o"] get_bz_grp_mbr.return_value = ["pingou@fp.o"] - toddlers.plugins.packager_bugzilla_sync.PackagerBugzillaSync.process( + toddler.process( config={"email_overrides_file": "test", "bugzilla_group": "fedora_contrib"}, message=None, username="nils", @@ -117,7 +117,7 @@ class TestPackagerBugzillaSyncToddler: def test_main_no_args(self, capsys): with pytest.raises(SystemExit): - toddlers.plugins.packager_bugzilla_sync.main([]) + main([]) out, err = capsys.readouterr() assert out == "" @@ -131,7 +131,7 @@ class TestPackagerBugzillaSyncToddler: @patch("toml.load", new=Mock(return_value={})) def test_main_debug(self, capsys): with pytest.raises(KeyError, match=r"'email_overrides_file'"): - toddlers.plugins.packager_bugzilla_sync.main(["test.cfg", "--debug"]) + main(["test.cfg", "--debug"]) out, err = capsys.readouterr() assert out == "Failed to load the file containing the email-overrides\n" assert err == "" @@ -139,7 +139,7 @@ class TestPackagerBugzillaSyncToddler: @patch("toml.load", new=Mock(return_value={})) def test_main(self, capsys): with pytest.raises(KeyError, match=r"'email_overrides_file'"): - toddlers.plugins.packager_bugzilla_sync.main(["test.cfg"]) + main(["test.cfg"]) out, err = capsys.readouterr() assert out == "Failed to load the file containing the email-overrides\n" assert err == "" diff --git a/tests/plugins/test_pdc_retired_packages.py b/tests/plugins/test_pdc_retired_packages.py index 69863dc..aaf69ed 100644 --- a/tests/plugins/test_pdc_retired_packages.py +++ b/tests/plugins/test_pdc_retired_packages.py @@ -9,13 +9,11 @@ import toddlers.plugins.pdc_retired_packages class TestPDCRetiredPackagesToddler: - def test_accepts_topic_invalid(self): - assert ( - toddlers.plugins.pdc_retired_packages.PDCRetiredPackages.accepts_topic( - "foo.bar" - ) - is False - ) + + toddler_cls = toddlers.plugins.pdc_retired_packages.PDCRetiredPackages + + def test_accepts_topic_invalid(self, toddler): + assert toddler.accepts_topic("foo.bar") is False @pytest.mark.parametrize( "topic", @@ -28,14 +26,12 @@ class TestPDCRetiredPackagesToddler: "org.fedoraproject.stg.git.receive", ], ) - def test_accepts_topic_valid(self, topic): - assert toddlers.plugins.pdc_retired_packages.PDCRetiredPackages.accepts_topic( - topic - ) + def test_accepts_topic_valid(self, toddler, topic): + assert toddler.accepts_topic(topic) @patch("toddlers.plugins.pdc_retired_packages.PDCRetiredPackages._process_dist_git") @patch("pdc_client.PDCClient") - def test_process_full_dist_git(self, pdc, process_dg): + def test_process_full_dist_git(self, pdc, process_dg, toddler): client = Mock() pdc.return_value = client @@ -44,9 +40,7 @@ class TestPDCRetiredPackagesToddler: msg.topic = "org.fedoraproject.prod.toddlers.trigger.pdc_retired_packages" msg.body = {} - toddlers.plugins.pdc_retired_packages.PDCRetiredPackages.process( - {"config": "foobar"}, msg - ) + toddler.process({"config": "foobar"}, msg) process_dg.assert_called_once_with({"config": "foobar"}, client) @@ -54,7 +48,7 @@ class TestPDCRetiredPackagesToddler: "toddlers.plugins.pdc_retired_packages.PDCRetiredPackages._process_single_package" ) @patch("pdc_client.PDCClient") - def test_process_single_package(self, pdc, process_dg): + def test_process_single_package(self, pdc, process_dg, toddler): client = Mock() pdc.return_value = client @@ -63,14 +57,12 @@ class TestPDCRetiredPackagesToddler: msg.topic = "org.fedoraproject.prod.git.receive" msg.body = {} - toddlers.plugins.pdc_retired_packages.PDCRetiredPackages.process( - {"config": "foobar"}, msg - ) + toddler.process({"config": "foobar"}, msg) process_dg.assert_called_once_with({"config": "foobar"}, client, msg) @patch("toddlers.plugins.pdc_retired_packages._is_retired_in_dist_git") - def test_process_dist_git(self, retired_in_dg): + def test_process_dist_git(self, retired_in_dg, toddler): page_component_branches = [ { "id": 44, @@ -116,9 +108,7 @@ class TestPDCRetiredPackagesToddler: msg.topic = "org.fedoraproject.prod.toddlers.trigger.pdc_retired_packages" msg.body = {} - toddlers.plugins.pdc_retired_packages.PDCRetiredPackages._process_dist_git( - {}, client - ) + toddler._process_dist_git({}, client) client.get_paged.assert_has_calls(calls=[call(page_component_branches)]) retired_in_dg.assert_has_calls( @@ -133,7 +123,9 @@ class TestPDCRetiredPackagesToddler: ) @patch("toddlers.plugins.pdc_retired_packages._is_retired_in_dist_git") - def test_process_dist_git_invalid_pdc_namespace(self, retired_in_dg, caplog): + def test_process_dist_git_invalid_pdc_namespace( + self, retired_in_dg, caplog, toddler + ): caplog.set_level(logging.DEBUG) page_component_branches = [ { @@ -180,9 +172,7 @@ class TestPDCRetiredPackagesToddler: msg.topic = "org.fedoraproject.prod.toddlers.trigger.pdc_retired_packages" msg.body = {} - toddlers.plugins.pdc_retired_packages.PDCRetiredPackages._process_dist_git( - {}, client - ) + toddler._process_dist_git({}, client) client.get_paged.assert_has_calls(calls=[call(page_component_branches)]) retired_in_dg.assert_has_calls( @@ -201,7 +191,7 @@ class TestPDCRetiredPackagesToddler: @patch("toddlers.plugins.pdc_retired_packages._retire_branch") @patch("toddlers.plugins.pdc_retired_packages._is_retired_in_dist_git") - def test_process_dist_git_full_distgit(self, retired_in_dg, retire_branch): + def test_process_dist_git_full_distgit(self, retired_in_dg, retire_branch, toddler): page_component_branches = [ { "id": 44, @@ -247,9 +237,7 @@ class TestPDCRetiredPackagesToddler: msg.topic = "org.fedoraproject.prod.toddlers.trigger.pdc_retired_packages" msg.body = {} - toddlers.plugins.pdc_retired_packages.PDCRetiredPackages._process_dist_git( - {}, client - ) + toddler._process_dist_git({}, client) client.get_paged.assert_has_calls(calls=[call(page_component_branches)]) retired_in_dg.assert_has_calls( @@ -310,7 +298,7 @@ class TestPDCRetiredPackagesToddler: ] ) - def test__process_single_package_regular_commit(self, caplog): + def test__process_single_package_regular_commit(self, toddler, caplog): caplog.set_level(logging.INFO) client = MagicMock() @@ -328,13 +316,11 @@ class TestPDCRetiredPackagesToddler: } } - toddlers.plugins.pdc_retired_packages.PDCRetiredPackages._process_single_package( - {}, client, msg - ) + toddler._process_single_package({}, client, msg) assert caplog.records[-1].message == "No dead.package in the commit, bailing" - def test__process_single_package_un_retirement(self, caplog): + def test__process_single_package_un_retirement(self, toddler, caplog): caplog.set_level(logging.INFO) client = MagicMock() @@ -353,13 +339,11 @@ class TestPDCRetiredPackagesToddler: } } - toddlers.plugins.pdc_retired_packages.PDCRetiredPackages._process_single_package( - {}, client, msg - ) + toddler._process_single_package({}, client, msg) assert caplog.records[-1].message == "dead.package file was not added, bailing" - def test__process_single_package_incomplete_config(self, caplog): + def test__process_single_package_incomplete_config(self, toddler, caplog): caplog.set_level(logging.INFO) client = MagicMock() @@ -381,13 +365,11 @@ class TestPDCRetiredPackagesToddler: } } - toddlers.plugins.pdc_retired_packages.PDCRetiredPackages._process_single_package( - {}, client, msg - ) + toddler._process_single_package({}, client, msg) assert caplog.records[-1].message == "No check URL configured, ignoring" - def test__process_single_package_package_not_in_pdc(self, caplog): + def test__process_single_package_package_not_in_pdc(self, toddler, caplog): caplog.set_level(logging.INFO) page_component_branches = { @@ -414,7 +396,7 @@ class TestPDCRetiredPackagesToddler: } } - toddlers.plugins.pdc_retired_packages.PDCRetiredPackages._process_single_package( + toddler._process_single_package( { "file_check_url": "https://src.fedoraproject.org/" "%(namespace)s/%(repo)s/blob/%(branch)s/f/%(file)s", @@ -425,7 +407,7 @@ class TestPDCRetiredPackagesToddler: assert caplog.records[-1].message == '"rpms/0ad" was not found in PDC' - def test__process_single_package_namespace_not_in_pdc(self, caplog): + def test__process_single_package_namespace_not_in_pdc(self, toddler, caplog): caplog.set_level(logging.INFO) page_component_branches = { @@ -452,7 +434,7 @@ class TestPDCRetiredPackagesToddler: } } - toddlers.plugins.pdc_retired_packages.PDCRetiredPackages._process_single_package( + toddler._process_single_package( { "file_check_url": "https://src.fedoraproject.org/" "%(namespace)s/%(repo)s/blob/%(branch)s/f/%(file)s", @@ -466,7 +448,7 @@ class TestPDCRetiredPackagesToddler: == "Ignoring namespace 'flatpack', error: The namespace \"flatpack\" is not supported" ) - def test__process_single_package_package_inactive(self, caplog): + def test__process_single_package_package_inactive(self, toddler, caplog): caplog.set_level(logging.INFO) page_component_branches = { @@ -504,7 +486,7 @@ class TestPDCRetiredPackagesToddler: } } - toddlers.plugins.pdc_retired_packages.PDCRetiredPackages._process_single_package( + toddler._process_single_package( { "file_check_url": "https://src.fedoraproject.org/" "%(namespace)s/%(repo)s/blob/%(branch)s/f/%(file)s", @@ -516,7 +498,7 @@ class TestPDCRetiredPackagesToddler: assert caplog.records[-1].message == '"rpms/0ad" not active in PDC in master' @patch("requests.head") - def test__process_single_package_package_not_retired(self, req, caplog): + def test__process_single_package_package_not_retired(self, req, toddler, caplog): caplog.set_level(logging.INFO) resp = Mock() resp.status_code = 404 @@ -557,7 +539,7 @@ class TestPDCRetiredPackagesToddler: } } - toddlers.plugins.pdc_retired_packages.PDCRetiredPackages._process_single_package( + toddler._process_single_package( { "file_check_url": "https://src.fedoraproject.org/" "%(namespace)s/%(repo)s/blob/%(branch)s/f/%(file)s", @@ -573,7 +555,7 @@ class TestPDCRetiredPackagesToddler: @patch("toddlers.plugins.pdc_retired_packages._retire_branch") @patch("requests.head") - def test__process_single_package(self, req, retire, caplog): + def test__process_single_package(self, req, retire, toddler, caplog): caplog.set_level(logging.INFO) resp = Mock() resp.status_code = 200 @@ -614,7 +596,7 @@ class TestPDCRetiredPackagesToddler: } } - toddlers.plugins.pdc_retired_packages.PDCRetiredPackages._process_single_package( + toddler._process_single_package( { "file_check_url": "https://src.fedoraproject.org/" "%(namespace)s/%(repo)s/blob/%(branch)s/f/%(file)s", diff --git a/tests/test_base.py b/tests/test_base.py index 5788076..6e1d3e0 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -1,15 +1,18 @@ -import toddlers.base +from toddlers.base import ToddlerBase class TestToddlerBase: - def test_name(self): - assert toddlers.base.ToddlerBase.name.fget() == "base" - def test_amqp_topics(self): - assert toddlers.base.ToddlerBase.amqp_topics.fget() == [] + toddler_cls = ToddlerBase - def test_accepts_topic(self): - assert toddlers.base.ToddlerBase.accepts_topic("foo.bar") is None + def test_name(self, toddler): + assert toddler.name == "base" - def test_process(self): - assert toddlers.base.ToddlerBase.process(config={}, message={}) is None + def test_amqp_topics(self, toddler): + assert toddler.amqp_topics == [] + + def test_accepts_topic(self, toddler): + assert toddler.accepts_topic("foo.bar") is None + + def test_process(self, toddler): + assert toddler.process(config={}, message={}) is None diff --git a/toddlers/base.py b/toddlers/base.py index 34881cd..2adb56f 100644 --- a/toddlers/base.py +++ b/toddlers/base.py @@ -6,13 +6,13 @@ class ToddlerBase(object): @property @abc.abstractmethod - def name(): + def name(self): """Returns name of the plugin.""" return "base" @property @abc.abstractmethod - def amqp_topics(): + def amqp_topics(self): """Returns the list of topics of interest for this toddler in a format that can be used directly when connecting to amqp. For example, it supports items like: @@ -24,13 +24,13 @@ class ToddlerBase(object): return [] @abc.abstractmethod - def accepts_topic(topic): + def accepts_topic(self, topic): """Returns a boolean whether this toddler is interested in messages from this specific topic. """ return @abc.abstractmethod - def process(config, message): + def process(self, config, message): """Process a given message.""" return diff --git a/toddlers/plugins/debug.py b/toddlers/plugins/debug.py index 04e088b..fdf3956 100644 --- a/toddlers/plugins/debug.py +++ b/toddlers/plugins/debug.py @@ -24,13 +24,13 @@ class DebugToddler(ToddlerBase): amqp_topics = ["#"] - def accepts_topic(topic): + def accepts_topic(self, topic): """Returns a boolean whether this toddler is interested in messages from this specific topic. """ return True - def process(config, message): + def process(self, config, message): """Process a given message.""" print(f"Debug Toddler received message: id:{message.id}, topic:{message.topic}") print("Debug Toddler - Processing done") diff --git a/toddlers/plugins/flag_ci_pr.py b/toddlers/plugins/flag_ci_pr.py index 00275c0..3a0a91f 100644 --- a/toddlers/plugins/flag_ci_pr.py +++ b/toddlers/plugins/flag_ci_pr.py @@ -49,7 +49,7 @@ class FlagCIPR(ToddlerBase): "org.centos.*.ci.dist-git-pr.test.running", ] - def accepts_topic(topic): + def accepts_topic(self, topic): """Returns a boolean whether this toddler is interested in messages from this specific topic. """ @@ -59,7 +59,7 @@ class FlagCIPR(ToddlerBase): and topic.endswith(("complete", "error", "running")) ) - def process(config, message): + def process(self, config, message): """Process a given message.""" pipeline_state = None diff --git a/toddlers/plugins/flag_commit_build.py b/toddlers/plugins/flag_commit_build.py index 7e642dc..1f75a09 100644 --- a/toddlers/plugins/flag_commit_build.py +++ b/toddlers/plugins/flag_commit_build.py @@ -56,7 +56,7 @@ class FlagCommitBuild(ToddlerBase): "org.fedoraproject.*.buildsys.build.state.change", ] - def accepts_topic(topic): + def accepts_topic(self, topic): """Returns a boolean whether this toddler is interested in messages from this specific topic. """ @@ -64,7 +64,7 @@ class FlagCommitBuild(ToddlerBase): "buildsys.build.state.change" ) - def process(config, message): + def process(self, config, message): """Process a given message.""" msg = message.body diff --git a/toddlers/plugins/packager_bugzilla_sync.py b/toddlers/plugins/packager_bugzilla_sync.py index d0a191d..629737f 100644 --- a/toddlers/plugins/packager_bugzilla_sync.py +++ b/toddlers/plugins/packager_bugzilla_sync.py @@ -50,7 +50,7 @@ class PackagerBugzillaSync(ToddlerBase): "org.fedoraproject.*.toddlers.trigger.packager_bugzilla_sync", ] - def accepts_topic(topic): + def accepts_topic(self, topic): """Returns a boolean whether this toddler is interested in messages from this specific topic. """ @@ -58,7 +58,7 @@ class PackagerBugzillaSync(ToddlerBase): "toddlers.trigger.packager_bugzilla_sync" ) - def process(config, message, username=False, dry_run=False): + def process(self, config, message, username=False, dry_run=False): """Process a given message.""" try: @@ -206,7 +206,7 @@ def main(args): setup_logging(log_level=args.log_level) config = toml.load(args.conf) - PackagerBugzillaSync.process( + PackagerBugzillaSync().process( config=config.get("consumer_config", {}).get("packager_bugzilla_sync", {}), message={}, username=args.username, diff --git a/toddlers/plugins/pdc_retired_packages.py b/toddlers/plugins/pdc_retired_packages.py index 9fac448..8fb023d 100644 --- a/toddlers/plugins/pdc_retired_packages.py +++ b/toddlers/plugins/pdc_retired_packages.py @@ -42,7 +42,7 @@ class PDCRetiredPackages(ToddlerBase): "org.fedoraproject.*.git.receive", ] - def accepts_topic(topic): + def accepts_topic(self, topic): """Returns a boolean whether this toddler is interested in messages from this specific topic. """ @@ -50,7 +50,7 @@ class PDCRetiredPackages(ToddlerBase): ("toddlers.trigger.pdc_retired_packages", "git.receive") ) - def process(config, message): + def process(self, config, message): """Process a given message.""" pdc = pdc_client.PDCClient(**config.get("pdc_config", {})) @@ -59,7 +59,7 @@ class PDCRetiredPackages(ToddlerBase): else: PDCRetiredPackages._process_dist_git(config, pdc) - def _process_dist_git(config, pdc): + def _process_dist_git(self, config, pdc): """ Updates PDC retirement status from analyzing dist-git. This steps over all the branches in dist-git and retires any branches @@ -83,7 +83,7 @@ class PDCRetiredPackages(ToddlerBase): if retired_in_dist_git: _retire_branch(pdc, branch) - def _process_single_package(config, pdc, message): + def _process_single_package(self, config, pdc, message): """Handle an incoming bus message. The message should be a dist-git retirement message (where someone adds diff --git a/toddlers/runner.py b/toddlers/runner.py index da89cf5..f5ab0c3 100644 --- a/toddlers/runner.py +++ b/toddlers/runner.py @@ -32,7 +32,7 @@ class RunningToddler(object): if toddler.name in blocked_toddlers: _log.info("Toddler '%s' is blocked, skipping it", toddler.name) continue - self.toddlers.append(toddler) + self.toddlers.append(toddler()) _log.info("Loaded: %s", self.toddlers) if not self.toddlers: raise fedora_messaging.exceptions.ConfigurationException(