From 737d76b144572bdd494fb50891aab800108f143b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Bompard?= Date: Thu, 19 Dec 2024 09:54:03 +0100 Subject: [PATCH] Use tomllib from the std lib instead of toml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Aurélien Bompard --- .zuul.yaml | 2 - tests/conftest.py | 8 ++ tests/plugins/test_check_commit_rights.py | 5 +- tests/plugins/test_check_email_overrides.py | 10 +- tests/plugins/test_distgit_bugzilla_sync.py | 119 ++---------------- tests/plugins/test_packager_bugzilla_sync.py | 45 +++---- .../test_packagers_without_bugzilla.py | 53 +++----- tests/plugins/test_scm_request_processor.py | 7 +- toddlers/plugins/check_commit_rights.py | 5 +- toddlers/plugins/check_email_overrides.py | 7 +- toddlers/plugins/distgit_bugzilla_sync.py | 8 +- toddlers/plugins/koji_block_retired.py | 5 +- toddlers/plugins/packager_bugzilla_sync.py | 8 +- .../plugins/packagers_without_bugzilla.py | 8 +- toddlers/plugins/scm_commits.py_ | 89 +++++++++++++ toddlers/plugins/scm_request_processor.py | 5 +- 16 files changed, 186 insertions(+), 198 deletions(-) create mode 100644 toddlers/plugins/scm_commits.py_ diff --git a/.zuul.yaml b/.zuul.yaml index 22944ed..2b3497a 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -49,7 +49,6 @@ - python3.11-devel - python3-pluggy - python3-py - - python3-toml - poetry - libmemcached-devel - fi-tox-python312: @@ -64,7 +63,6 @@ - python3.12-devel - python3-pluggy - python3-py - - python3-toml - poetry - libmemcached-devel ... diff --git a/tests/conftest.py b/tests/conftest.py index 19e779e..60908cc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -78,3 +78,11 @@ def msg_file(request, monkeypatch): with open(msg_file, "w") as stream: json.dump(data, stream) return msg_file + + +@pytest.fixture +def empty_config_file(tmp_path): + config_path = tmp_path.joinpath("config.toml").as_posix() + with open(config_path, "w") as fh: + fh.write("") + return config_path diff --git a/tests/plugins/test_check_commit_rights.py b/tests/plugins/test_check_commit_rights.py index d917a54..9b33cb1 100644 --- a/tests/plugins/test_check_commit_rights.py +++ b/tests/plugins/test_check_commit_rights.py @@ -300,13 +300,12 @@ class TestCheckCommitRightsToddler: assert err.startswith("usage:") assert "error: the following arguments are required:" in err - @patch("toml.load", new=Mock(return_value={})) - def test_main_no_exclude_users(self, capsys): + def test_main_no_exclude_users(self, capsys, empty_config_file): with pytest.raises( Exception, match=r"Invalid toddler configuration, no `exclude_users` defined", ): - toddlers.plugins.check_commit_rights.main(["test.cfg"]) + toddlers.plugins.check_commit_rights.main([empty_config_file]) out, err = capsys.readouterr() assert out == "" assert err == "" diff --git a/tests/plugins/test_check_email_overrides.py b/tests/plugins/test_check_email_overrides.py index 150cfb7..bb2312c 100644 --- a/tests/plugins/test_check_email_overrides.py +++ b/tests/plugins/test_check_email_overrides.py @@ -177,24 +177,22 @@ class TestCheckEmailOverridesToddler: assert err.startswith("usage:") assert "error: the following arguments are required:" in err - @patch("toml.load", new=Mock(return_value={})) - def test_main_debug(self, capsys): + def test_main_debug(self, capsys, empty_config_file): with pytest.raises( Exception, match=r"Invalid toddler configuration, no `email_overrides_url` defined", ): - toddlers.plugins.check_email_overrides.main(["test.cfg", "--debug"]) + toddlers.plugins.check_email_overrides.main([empty_config_file, "--debug"]) out, err = capsys.readouterr() assert out == "" assert err == "" - @patch("toml.load", new=Mock(return_value={})) - def test_main(self, capsys): + def test_main(self, capsys, empty_config_file): with pytest.raises( Exception, match=r"Invalid toddler configuration, no `email_overrides_url` defined", ): - toddlers.plugins.check_email_overrides.main(["test.cfg"]) + toddlers.plugins.check_email_overrides.main([empty_config_file]) out, err = capsys.readouterr() assert out == "" assert err == "" diff --git a/tests/plugins/test_distgit_bugzilla_sync.py b/tests/plugins/test_distgit_bugzilla_sync.py index 1d4e55f..d780b5c 100644 --- a/tests/plugins/test_distgit_bugzilla_sync.py +++ b/tests/plugins/test_distgit_bugzilla_sync.py @@ -14,11 +14,14 @@ from toddlers.utils.pagure import PagureError @pytest.fixture -def config(): +def config(tmp_path): """Test configuration for toddler fixture.""" + email_overrides_file = tmp_path.joinpath("email-overrides.toml").as_posix() + with open(email_overrides_file, "w") as fh: + fh.write("") return { # toddlers level config values - "email_overrides_file": "dummy_file", + "email_overrides_file": email_overrides_file, "dist_git_url": "https://src.fedoraproject.org", "mail_server": "bastion", "admin_email": "root@localhost", @@ -130,10 +133,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.bugzilla_system") @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load") def test_process_dry_run_edit_project( self, - mock_toml, mock_set_bodhi, mock_set_pagure, mock_bugzilla, @@ -143,10 +144,6 @@ class TestDistgitBugzillaSyncToddler: toddler, ): """Assert that dry run with edit is processed correctly.""" - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = {"foo": "Summary"} @@ -188,8 +185,6 @@ class TestDistgitBugzillaSyncToddler: toddler.process(config=config, message={}, dry_run=True) # Asserts - mock_toml.assert_called_with("dummy_file") - assert toddler.namespace_to_product == { "rpms": ["Fedora", "Fedora EPEL"], "container": ["Fedora Container Images"], @@ -291,10 +286,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.bugzilla_system") @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load") def test_process_unknown_namespace( self, - mock_toml, mock_set_bodhi, mock_set_pagure, mock_bugzilla, @@ -304,10 +297,6 @@ class TestDistgitBugzillaSyncToddler: toddler, ): """Assert that namespace that we don't know product for will be ignored.""" - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = {"foo": "Summary"} @@ -329,8 +318,6 @@ class TestDistgitBugzillaSyncToddler: toddler.process(config=config, message={}, dry_run=True) # Asserts - mock_toml.assert_called_with("dummy_file") - assert toddler.namespace_to_product == { "rpms": ["Fedora", "Fedora EPEL"], "container": ["Fedora Container Images"], @@ -356,10 +343,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.bugzilla_system") @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load") def test_process_dry_run_add_project( self, - mock_toml, mock_set_bodhi, mock_set_pagure, mock_bugzilla, @@ -369,10 +354,6 @@ class TestDistgitBugzillaSyncToddler: toddler, ): """Assert that dry run with add is processed correctly.""" - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = {"foo": "Summary"} @@ -414,8 +395,6 @@ class TestDistgitBugzillaSyncToddler: toddler.process(config=config, message={}, dry_run=True) # Asserts - mock_toml.assert_called_with("dummy_file") - assert toddler.namespace_to_product == { "rpms": ["Fedora", "Fedora EPEL"], "container": ["Fedora Container Images"], @@ -508,10 +487,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.bugzilla_system") @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load") def test_process_dry_run_specific_project( self, - mock_toml, mock_set_bodhi, mock_set_pagure, mock_bugzilla, @@ -521,10 +498,6 @@ class TestDistgitBugzillaSyncToddler: toddler, ): """Assert that dry run is processed only for the specified project.""" - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = { "foo": "Summary", @@ -642,10 +615,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.bugzilla_system") @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load") def test_process_epel10( self, - mock_toml, mock_set_bodhi, mock_set_pagure, mock_bugzilla, @@ -655,10 +626,6 @@ class TestDistgitBugzillaSyncToddler: toddler, ): """Assert that epel 10 project is processed correctly.""" - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = {"foo": "Summary"} @@ -700,8 +667,6 @@ class TestDistgitBugzillaSyncToddler: toddler.process(config=config, message={}, dry_run=True) # Asserts - mock_toml.assert_called_with("dummy_file") - assert toddler.namespace_to_product == { "rpms": ["Fedora", "Fedora EPEL"], "container": ["Fedora Container Images"], @@ -794,10 +759,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.bugzilla_system") @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load") def test_process_container( self, - mock_toml, mock_set_bodhi, mock_set_pagure, mock_bugzilla, @@ -807,10 +770,6 @@ class TestDistgitBugzillaSyncToddler: toddler, ): """Assert that container project is processed correctly.""" - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = {"foo": "Summary"} @@ -852,8 +811,6 @@ class TestDistgitBugzillaSyncToddler: toddler.process(config=config, message={}, dry_run=True) # Asserts - mock_toml.assert_called_with("dummy_file") - assert toddler.namespace_to_product == { "rpms": ["Fedora", "Fedora EPEL"], "container": ["Fedora Container Images"], @@ -927,10 +884,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.notify") @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load") def test_process_report_protocol_error( self, - mock_toml, mock_set_bodhi, mock_set_pagure, mock_notify, @@ -945,10 +900,6 @@ class TestDistgitBugzillaSyncToddler: # Adjust config config["temp_folder"] = tmpdir - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = { "foo": "Summary", @@ -1053,10 +1004,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.notify") @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load") def test_process_report_client_error( self, - mock_toml, mock_set_bodhi, mock_set_pagure, mock_notify, @@ -1071,10 +1020,6 @@ class TestDistgitBugzillaSyncToddler: # Adjust config config["temp_folder"] = tmpdir - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = { "foo": "Summary", @@ -1200,10 +1145,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.notify") @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load") def test_process_report_missing_mails( self, - mock_toml, mock_set_bodhi, mock_set_pagure, mock_notify, @@ -1214,10 +1157,6 @@ class TestDistgitBugzillaSyncToddler: toddler, ): """Assert that missing bugzilla e-mails are reported correctly.""" - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = { "foo": "Summary", @@ -1309,10 +1248,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.bugzilla_system") @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load") def test_process_dry_run_verbose( self, - mock_toml, mock_set_bodhi, mock_set_pagure, mock_bugzilla, @@ -1323,10 +1260,6 @@ class TestDistgitBugzillaSyncToddler: caplog, ): """Assert that dry run with verbose is processed correctly.""" - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = {"foo": "Summary"} @@ -1418,10 +1351,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.bugzilla_system") @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load") def test_process_missing_namespace( self, - mock_toml, mock_set_bodhi, mock_set_pagure, mock_bugzilla, @@ -1431,10 +1362,6 @@ class TestDistgitBugzillaSyncToddler: toddler, ): """Assert that namespace missing in config is processed correctly.""" - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = {"foo": "Summary"} @@ -1480,10 +1407,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.bugzilla_system") @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load") def test_process_orphaned_project( self, - mock_toml, mock_set_bodhi, mock_set_pagure, mock_bugzilla, @@ -1493,10 +1418,6 @@ class TestDistgitBugzillaSyncToddler: toddler, ): """Assert that dry run with edit is processed correctly.""" - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = {"foo": "Summary"} @@ -1613,10 +1534,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") @patch("toddlers.plugins.distgit_bugzilla_sync.fedora_account") @patch("toddlers.plugins.distgit_bugzilla_sync.bugzilla_system") - @patch("toml.load") def test_process_ignorrable_account( self, - mock_toml, mock_bugzilla, mock_fas, mock_set_bodhi, @@ -1628,10 +1547,6 @@ class TestDistgitBugzillaSyncToddler: """Assert that dry run with edit is processed correctly.""" # Adjust config config["ignorable_accounts"] = ["Slaanesh"] - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = {"foo": "Summary"} @@ -1733,10 +1648,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.notify") @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load") def test_process_notify_user_cache_exists( self, - mock_toml, mock_set_bodhi, mock_set_pagure, mock_notify, @@ -1762,10 +1675,6 @@ class TestDistgitBugzillaSyncToddler: with open(os.path.join(tmpdir, "user_cache.json"), "w") as stream: json.dump(data, stream) - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = { "foo": "Summary", @@ -1827,10 +1736,8 @@ class TestDistgitBugzillaSyncToddler: @patch("toddlers.plugins.distgit_bugzilla_sync.bugzilla_system") @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load") def test_skip_on_pagure_error( self, - mock_toml, mock_set_bodhi, mock_set_pagure, mock_bugzilla, @@ -1840,10 +1747,6 @@ class TestDistgitBugzillaSyncToddler: toddler, ): """Assert that errors are handled if Pagure fails getting the branches.""" - # Mock toml load - email_overrides = Mock() - mock_toml.return_value = email_overrides - # Mock package summaries response mock_summaries.return_value = {"foo": "Summary"} @@ -1928,18 +1831,18 @@ class TestMain: @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load", new=Mock(return_value={})) - def test_main_debug(self, mock_set_bodhi, mock_set_pagure, caplog): + def test_main_debug( + self, mock_set_bodhi, mock_set_pagure, caplog, empty_config_file + ): """Assert that debug is set correctly.""" with pytest.raises(KeyError, match=r"'email_overrides_file'"): - main(["test.cfg", "--debug"]) + main([empty_config_file, "--debug"]) assert "Failed to load the file containing the email-overrides" in caplog.text @patch("toddlers.plugins.distgit_bugzilla_sync.pagure.set_pagure") @patch("toddlers.plugins.distgit_bugzilla_sync.bodhi.set_bodhi") - @patch("toml.load", new=Mock(return_value={})) - def test_main(self, mock_set_bodhi, mock_set_pagure, caplog): + def test_main(self, mock_set_bodhi, mock_set_pagure, caplog, empty_config_file): """Assert that INFO log level is handled correctly.""" with pytest.raises(KeyError, match=r"'email_overrides_file'"): - main(["test.cfg"]) + main([empty_config_file]) assert "Failed to load the file containing the email-overrides" in caplog.text diff --git a/tests/plugins/test_packager_bugzilla_sync.py b/tests/plugins/test_packager_bugzilla_sync.py index b3ac2c2..80b67ce 100644 --- a/tests/plugins/test_packager_bugzilla_sync.py +++ b/tests/plugins/test_packager_bugzilla_sync.py @@ -51,29 +51,29 @@ class TestPackagerBugzillaSyncToddler: @patch("toddlers.utils.fedora_account.get_bz_email_user") @patch("toddlers.utils.bugzilla_system.get_group_member") @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, toddler, + empty_config_file, ): - 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"] toddler.process( - config={"email_overrides_file": "test", "bugzilla_group": "fedora_contrib"}, + config={ + "email_overrides_file": empty_config_file, + "bugzilla_group": "fedora_contrib", + }, message=None, username=False, dry_run=False, ) - toml_load.assert_called_with("test") get_fas_grp_mbr.assert_called_with("packager") get_bz_email.assert_called_with("pingou", {}) get_bz_grp_mbr.assert_called_with("fedora_contrib") @@ -89,22 +89,27 @@ class TestPackagerBugzillaSyncToddler: @patch("toddlers.utils.fedora_account.get_bz_email_user") @patch("toddlers.utils.bugzilla_system.get_group_member") @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, toddler + self, + bz_user_grp, + get_bz_grp_mbr, + get_bz_email, + toddler, + empty_config_file, ): - toml_load.return_value = {} get_bz_email.side_effect = ["nils@fp.o"] get_bz_grp_mbr.return_value = ["pingou@fp.o"] toddler.process( - config={"email_overrides_file": "test", "bugzilla_group": "fedora_contrib"}, + config={ + "email_overrides_file": empty_config_file, + "bugzilla_group": "fedora_contrib", + }, message=None, username="nils", dry_run=False, ) - toml_load.assert_called_with("test") get_bz_email.assert_called_with("nils", {}) get_bz_grp_mbr.assert_called_with("fedora_contrib") bz_user_grp.assert_called_with( @@ -120,31 +125,31 @@ class TestPackagerBugzillaSyncToddler: @patch("toddlers.utils.fedora_account.get_bz_email_user") @patch("toddlers.utils.bugzilla_system.get_group_member") @patch("toddlers.utils.bugzilla_system.add_user_to_group") - @patch("toml.load") def test_process_fas_only_none( self, - toml_load, bz_user_grp, get_bz_grp_mbr, get_bz_email, get_fa_grp_member, toddler, + empty_config_file, ): # Assert that there is no error when the fas_only contains None # https://pagure.io/fedora-infra/toddlers/issue/174 - toml_load.return_value = {} get_fa_grp_member.return_value = ["nils", "zlopez"] get_bz_email.side_effect = [None, "zlopez@fp.o"] get_bz_grp_mbr.return_value = ["pingou@fp.o"] toddler.process( - config={"email_overrides_file": "test", "bugzilla_group": "fedora_contrib"}, + config={ + "email_overrides_file": empty_config_file, + "bugzilla_group": "fedora_contrib", + }, message=None, username=None, dry_run=False, ) - toml_load.assert_called_with("test") get_bz_email.assert_called_with("zlopez", {}) get_bz_grp_mbr.assert_called_with("fedora_contrib") bz_user_grp.assert_called_with( @@ -167,18 +172,16 @@ class TestPackagerBugzillaSyncToddler: assert err.startswith("usage:") assert "error: the following arguments are required:" in err - @patch("toml.load", new=Mock(return_value={})) - def test_main_debug(self, capsys): + def test_main_debug(self, capsys, empty_config_file): with pytest.raises(KeyError, match=r"'email_overrides_file'"): - main(["test.cfg", "--debug"]) + main([empty_config_file, "--debug"]) out, err = capsys.readouterr() assert out == "Failed to load the file containing the email-overrides\n" assert err == "" - @patch("toml.load", new=Mock(return_value={})) - def test_main(self, capsys): + def test_main(self, capsys, empty_config_file): with pytest.raises(KeyError, match=r"'email_overrides_file'"): - main(["test.cfg"]) + main([empty_config_file]) out, err = capsys.readouterr() assert out == "Failed to load the file containing the email-overrides\n" assert err == "" diff --git a/tests/plugins/test_packagers_without_bugzilla.py b/tests/plugins/test_packagers_without_bugzilla.py index b3e83ad..5975d93 100644 --- a/tests/plugins/test_packagers_without_bugzilla.py +++ b/tests/plugins/test_packagers_without_bugzilla.py @@ -30,18 +30,18 @@ class TestPackagersWithoutBugzillaToddler: assert out == "Failed to load the file containing the email-overrides\n" assert err == "" - def test_process_no_email_override_file(self, toddler, capsys): + def test_process_no_email_override_file(self, toddler, capsys, empty_config_file): with pytest.raises( - FileNotFoundError, match=r"No such file or directory: 'test'" + ValueError, match=r"No fas_url found in the configuration file" ): toddler.process( - config={"email_overrides_file": "test"}, + config={"email_overrides_file": empty_config_file}, message=None, username=False, ) out, err = capsys.readouterr() - assert out == "Failed to load the file containing the email-overrides\n" + assert out == "" assert err == "" @patch("toddlers.utils.fedora_account.set_fasjson", new=Mock(return_value=True)) @@ -51,18 +51,16 @@ class TestPackagersWithoutBugzillaToddler: @patch("toddlers.utils.bugzilla_system.get_group_member") @patch("toddlers.utils.fedora_account.get_bz_email_group") @patch("toddlers.utils.fedora_account.get_bz_email_user") - @patch("toml.load") def test_process_ignorable_namespaces( self, - toml_load, get_bz_email_user, get_bz_email_group, bz_get_group_member, bz_get_user, send_email, toddler, + empty_config_file, ): - toml_load.return_value = {} get_bz_email_user.side_effect = [ "besser82@fp.o", "churchyard@fp.o", @@ -85,7 +83,7 @@ class TestPackagersWithoutBugzillaToddler: toddler.process( config={ - "email_overrides_file": "test", + "email_overrides_file": empty_config_file, "bugzilla_group": "fedora_contrib", "dist_git_url": "https://src.fp.o", "admin_email": "admin@fp.o", @@ -96,7 +94,6 @@ class TestPackagersWithoutBugzillaToddler: username=False, ) - toml_load.assert_called_with("test") get_bz_email_user.assert_not_called() get_bz_email_group.assert_not_called() bz_get_group_member.assert_called_with("fedora_contrib") @@ -110,18 +107,16 @@ class TestPackagersWithoutBugzillaToddler: @patch("toddlers.utils.bugzilla_system.get_group_member") @patch("toddlers.utils.fedora_account.get_bz_email_group") @patch("toddlers.utils.fedora_account.get_bz_email_user") - @patch("toml.load") def test_process( self, - toml_load, get_bz_email_user, get_bz_email_group, bz_get_group_member, bz_get_user, send_email, toddler, + empty_config_file, ): - toml_load.return_value = {} get_bz_email_user.side_effect = [ "besser82@fp.o", "churchyard@fp.o", @@ -144,7 +139,7 @@ class TestPackagersWithoutBugzillaToddler: toddler.process( config={ - "email_overrides_file": "test", + "email_overrides_file": empty_config_file, "bugzilla_group": "fedora_contrib", "dist_git_url": "https://src.fp.o", "admin_email": "admin@fp.o", @@ -154,7 +149,6 @@ class TestPackagersWithoutBugzillaToddler: username=False, ) - toml_load.assert_called_with("test") get_bz_email_user.assert_called() get_bz_email_user.assert_has_calls( calls=[call("besser82", {}), call("churchyard", {}), call("dwmw2", {})] @@ -267,18 +261,16 @@ class TestPackagersWithoutBugzillaToddler: @patch("toddlers.utils.bugzilla_system.get_group_member") @patch("toddlers.utils.fedora_account.get_bz_email_group") @patch("toddlers.utils.fedora_account.get_bz_email_user") - @patch("toml.load") def test_process_username_no_bz_email( self, - toml_load, get_bz_email_user, get_bz_email_group, bz_get_group_member, bz_get_user, send_email, toddler, + empty_config_file, ): - toml_load.return_value = {} get_bz_email_user.side_effect = [None] get_bz_email_group.side_effect = ["python-sig@lists.fp.o"] bz_get_group_member.return_value = ["dwmw2@fp.o", "besser82@fp.o"] @@ -297,7 +289,7 @@ class TestPackagersWithoutBugzillaToddler: toddler.process( config={ - "email_overrides_file": "test", + "email_overrides_file": empty_config_file, "bugzilla_group": "fedora_contrib", "dist_git_url": "https://src.fp.o", "admin_email": "admin@fp.o", @@ -307,7 +299,6 @@ class TestPackagersWithoutBugzillaToddler: username="nils", ) - toml_load.assert_called_with("test") bz_get_group_member.assert_called_with("fedora_contrib") get_bz_email_user.assert_called() get_bz_email_user.assert_has_calls(calls=[call("nils", {})]) @@ -324,18 +315,16 @@ class TestPackagersWithoutBugzillaToddler: @patch("toddlers.utils.bugzilla_system.get_group_member") @patch("toddlers.utils.fedora_account.get_bz_email_group") @patch("toddlers.utils.fedora_account.get_bz_email_user") - @patch("toml.load") def test_process_username_ignored( self, - toml_load, get_bz_email_user, get_bz_email_group, bz_get_group_member, bz_get_user, send_email, toddler, + empty_config_file, ): - toml_load.return_value = {} get_bz_email_user.side_effect = [ "dwmw2@fp.o", ] @@ -350,7 +339,7 @@ class TestPackagersWithoutBugzillaToddler: toddler.process( config={ - "email_overrides_file": "test", + "email_overrides_file": empty_config_file, "bugzilla_group": "fedora_contrib", "dist_git_url": "https://src.fp.o", "admin_email": "admin@fp.o", @@ -360,7 +349,6 @@ class TestPackagersWithoutBugzillaToddler: message=None, ) - toml_load.assert_called_with("test") bz_get_group_member.assert_called_with("fedora_contrib") get_bz_email_user.assert_called() get_bz_email_user.assert_has_calls(calls=[call("dwmw2", {})]) @@ -376,18 +364,16 @@ class TestPackagersWithoutBugzillaToddler: @patch("toddlers.utils.bugzilla_system.get_group_member") @patch("toddlers.utils.fedora_account.get_bz_email_group") @patch("toddlers.utils.fedora_account.get_bz_email_user") - @patch("toml.load") def test_process_username_group_no_bz_email( self, - toml_load, get_bz_email_user, get_bz_email_group, bz_get_group_member, bz_get_user, send_email, toddler, + empty_config_file, ): - toml_load.return_value = {} get_bz_email_group.side_effect = [None] bz_get_group_member.return_value = ["dwmw2@fp.o", "besser82@fp.o"] bz_get_user.return_value = False @@ -405,7 +391,7 @@ class TestPackagersWithoutBugzillaToddler: toddler.process( config={ - "email_overrides_file": "test", + "email_overrides_file": empty_config_file, "bugzilla_group": "fedora_contrib", "dist_git_url": "https://src.fp.o", "admin_email": "admin@fp.o", @@ -415,7 +401,6 @@ class TestPackagersWithoutBugzillaToddler: username="@python-sig", ) - toml_load.assert_called_with("test") bz_get_group_member.assert_called_with("fedora_contrib") get_bz_email_user.assert_not_called() get_bz_email_group.assert_called() @@ -437,18 +422,16 @@ class TestPackagersWithoutBugzillaToddler: assert err.startswith("usage:") assert "error: the following arguments are required:" in err - @patch("toml.load", new=Mock(return_value={})) - def test_main_debug(self, capsys): + def test_main_debug(self, capsys, empty_config_file): with pytest.raises(KeyError, match=r"'email_overrides_file'"): - main(["test.cfg", "--debug"]) + main([empty_config_file, "--debug"]) out, err = capsys.readouterr() assert out == "Failed to load the file containing the email-overrides\n" assert err == "" - @patch("toml.load", new=Mock(return_value={})) - def test_main(self, capsys): + def test_main(self, capsys, empty_config_file): with pytest.raises(KeyError, match=r"'email_overrides_file'"): - main(["test.cfg"]) + main([empty_config_file]) out, err = capsys.readouterr() assert out == "Failed to load the file containing the email-overrides\n" assert err == "" diff --git a/tests/plugins/test_scm_request_processor.py b/tests/plugins/test_scm_request_processor.py index ba4c776..ac1281b 100644 --- a/tests/plugins/test_scm_request_processor.py +++ b/tests/plugins/test_scm_request_processor.py @@ -2797,13 +2797,11 @@ class TestMain: assert err.startswith("usage:") assert "error: the following arguments are required:" in err - @patch("toml.load") @patch("toddlers.plugins.scm_request_processor.SCMRequestProcessor.process") @patch("toddlers.plugins.scm_request_processor.pagure") - def test_main(self, mock_pagure, mock_process, mock_toml): + def test_main(self, mock_pagure, mock_process, empty_config_file): """Assert that main is initializing config and message correctly.""" # Preparation - mock_toml.return_value = {} mock_pagure_io = Mock() mock_issue = {"id": 100, "user": {"name": "zlopez"}} mock_pagure_io.get_issue.return_value = mock_issue @@ -2811,10 +2809,9 @@ class TestMain: mock_pagure.set_pagure.return_value = mock_pagure_io # Function to test - scm_request_processor.main(["--config", "test.cfg", "100"]) + scm_request_processor.main(["--config", empty_config_file, "100"]) # Assertions - mock_toml.assert_called_with("test.cfg") mock_pagure.set_pagure.assert_called_with({}) mock_pagure_io.get_issue.assert_called_with( 100, scm_request_processor.PROJECT_NAMESPACE diff --git a/toddlers/plugins/check_commit_rights.py b/toddlers/plugins/check_commit_rights.py index bea7bc5..8bfa64f 100644 --- a/toddlers/plugins/check_commit_rights.py +++ b/toddlers/plugins/check_commit_rights.py @@ -11,7 +11,7 @@ import argparse import logging import sys -import toml +import tomllib from toddlers.base import ToddlerBase from toddlers.utils import fedora_account @@ -183,7 +183,8 @@ def main(args): args = get_arguments(args) setup_logging(log_level=args.log_level) - base_config = toml.load(args.conf) + with open(args.conf, "rb") as conf_fp: + base_config = tomllib.load(conf_fp) default_config = base_config.get("consumer_config", {}).get("default", {}) private_config = base_config.get("consumer_config", {}).get( diff --git a/toddlers/plugins/check_email_overrides.py b/toddlers/plugins/check_email_overrides.py index 422082b..0604ac1 100644 --- a/toddlers/plugins/check_email_overrides.py +++ b/toddlers/plugins/check_email_overrides.py @@ -12,7 +12,7 @@ import argparse import logging import sys -import toml +import tomllib from toddlers.base import ToddlerBase from toddlers.utils import bugzilla_system @@ -66,7 +66,7 @@ class CheckEmailOverrides(ToddlerBase): req.status_code, ) - data = toml.loads(req.text) + data = tomllib.loads(req.text) _log.info("Setting up connection to FAS") fedora_account.set_fasjson(config) @@ -167,7 +167,8 @@ def main(args): args = get_arguments(args) setup_logging(log_level=args.log_level) - base_config = toml.load(args.conf) + with open(args.conf, "rb") as conf_fh: + base_config = tomllib.load(conf_fh) default_config = base_config.get("consumer_config", {}).get("default", {}) private_config = base_config.get("consumer_config", {}).get( diff --git a/toddlers/plugins/distgit_bugzilla_sync.py b/toddlers/plugins/distgit_bugzilla_sync.py index c3e6e71..8366300 100644 --- a/toddlers/plugins/distgit_bugzilla_sync.py +++ b/toddlers/plugins/distgit_bugzilla_sync.py @@ -17,7 +17,7 @@ import time from typing import Any, cast, Dict, Iterator, Mapping, Optional import xmlrpc -import toml +import tomllib try: import tqdm @@ -97,7 +97,8 @@ class DistgitBugzillaSync(ToddlerBase): self.bodhi = bodhi.set_bodhi(config) try: - email_overrides = toml.load(config["email_overrides_file"]) + with open(config["email_overrides_file"], "rb") as overrides_fh: + email_overrides = tomllib.load(overrides_fh) except Exception: _log.error("Failed to load the file containing the email-overrides") raise @@ -571,7 +572,8 @@ def main(args): args = _get_arguments(args) _setup_logging(log_level=args.log_level) - config = toml.load(args.conf) + with open(args.conf, "rb") as conf_fh: + config = tomllib.load(conf_fh) parsed_config = config.get("consumer_config", {}).get("default", {}) parsed_config.update( config.get("consumer_config", {}).get("distgit_bugzilla_sync", ()) diff --git a/toddlers/plugins/koji_block_retired.py b/toddlers/plugins/koji_block_retired.py index d88b39e..00cf0d8 100644 --- a/toddlers/plugins/koji_block_retired.py +++ b/toddlers/plugins/koji_block_retired.py @@ -15,7 +15,7 @@ import time from fedora_messaging_git_hook_messages import CommitV1 import koji import requests -import toml +import tomllib from toddlers.base import ToddlerBase from toddlers.utils import bodhi, pagure @@ -326,7 +326,8 @@ def main(args): """Schedule the first test and run the scheduler.""" args = get_arguments(args) logging.StreamHandler(stream=sys.stdout) - config = toml.load(args.conf) + with open(args.conf, "rb") as conf_fh: + config = tomllib.load(conf_fh) logging.basicConfig(level=args.log_level) msg_file = args.message diff --git a/toddlers/plugins/packager_bugzilla_sync.py b/toddlers/plugins/packager_bugzilla_sync.py index e05b665..310aff9 100644 --- a/toddlers/plugins/packager_bugzilla_sync.py +++ b/toddlers/plugins/packager_bugzilla_sync.py @@ -12,7 +12,7 @@ import argparse import logging import sys -import toml +import tomllib try: import tqdm @@ -50,7 +50,8 @@ class PackagerBugzillaSync(ToddlerBase): """Process a given message.""" try: - email_overrides = toml.load(config["email_overrides_file"]) + with open(config["email_overrides_file"], "rb") as overrides_fh: + email_overrides = tomllib.load(overrides_fh) except Exception: print("Failed to load the file containing the email-overrides") raise @@ -196,7 +197,8 @@ def main(args): args = get_arguments(args) setup_logging(log_level=args.log_level) - config = toml.load(args.conf) + with open(args.conf, "rb") as conf_fh: + config = tomllib.load(conf_fh) PackagerBugzillaSync().process( config=config.get("consumer_config", {}).get("packager_bugzilla_sync", {}), message={}, diff --git a/toddlers/plugins/packagers_without_bugzilla.py b/toddlers/plugins/packagers_without_bugzilla.py index 3ce200d..9b871b9 100644 --- a/toddlers/plugins/packagers_without_bugzilla.py +++ b/toddlers/plugins/packagers_without_bugzilla.py @@ -12,7 +12,7 @@ import logging import sys import time -import toml +import tomllib try: import tqdm @@ -95,7 +95,8 @@ class PackagersWithoutBugzilla(ToddlerBase): self.logs = [] try: - email_overrides = toml.load(config["email_overrides_file"]) + with open(config["email_overrides_file"], "rb") as overrides_fh: + email_overrides = tomllib.load(overrides_fh) except Exception: print("Failed to load the file containing the email-overrides") raise @@ -293,7 +294,8 @@ def main(args): args = get_arguments(args) setup_logging(log_level=args.log_level) - config = toml.load(args.conf) + with open(args.conf, "rb") as conf_fh: + config = tomllib.load(conf_fh) PackagersWithoutBugzilla().process( config=config.get("consumer_config", {}).get("packagers_without_bugzilla", {}), message={}, diff --git a/toddlers/plugins/scm_commits.py_ b/toddlers/plugins/scm_commits.py_ new file mode 100644 index 0000000..135fc85 --- /dev/null +++ b/toddlers/plugins/scm_commits.py_ @@ -0,0 +1,89 @@ +""" +This script sends an email on each incoming git commit. +https://pagure.io/fedora-infrastructure/issue/11641 + +Authors: Aurélien Bompard + +""" +import argparse +import fnmatch +import json +import logging +import re +import sys +import smtplib +from email.message import EmailMessage +from tempfile import TemporaryDirectory +import traceback +from typing import Optional + +import arrow +import backoff +from fedora_messaging.api import Message +from pagure_messages.issue_schema import IssueNewV1 +import toml + +from toddlers.base import ToddlerBase +from toddlers.exceptions import ValidationError +from toddlers.utils import bugzilla_system, fedora_account, git, pagure, pdc, requests + +_log = logging.getLogger(__name__) + + +def backoff_hdlr(details): + _log.warning("Failed sending email. Retrying. %s", traceback.format_tb(sys.exc_info()[2])) + self = details["args"][0] + self._smtp.close() + + +def giveup_hdlr(details): + _log.error("Failed sending email. Giving up. %s", traceback.format_tb(sys.exc_info()[2])) + + +class SCMCommitsProcessor(ToddlerBase): + """ + Listens for commits in Dist-Git and sends an email to a configured address. + """ + + name: str = "scm_commits" + + amqp_topics: list = [ + "org.fedoraproject.*.git.receive", + ] + amqp_topics_re = re.compile(r"^org\.fedoraproject\.[^.]+\.git\.receive$") + + _smtp = None + + def accepts_topic(self, topic: str) -> bool: + return self.amqp_topics_re.match(topic) is not None + + def process( + self, + config: dict, + message: Message, + ) -> None: + _log.debug( + "Processing message:\n%s", json.dumps(message.body, indent=2) + ) + if self._smtp is None: + self._smtp = smtplib.SMTP(config.get("mail_server", ""), config.get("mail_server_port", 0)) + + subject = config.get("email_subject_prefix", "") + message.summary + self.send_email(config.get("email_from", config["admin_email"]), config["email_to"], subject, str(message)) + + @backoff.on_exception( + backoff.expo, + (smtplib.SMTPServerDisconnected), + max_tries=3, + on_backoff=backoff_hdlr, + on_giveup=giveup_hdlr, + ) + def send_email(self, sender: str, rcpt: str, subject: str, body: str): + email = EmailMessage() + email["From"] = sender + email["To"] = rcpt + email["Subject"] = subject + email.set_content(body) + _log.debug("Sending email to %s with subject %s", email["To"], email["Subject"]) + self._smtp.send_message(email) + _log.debug("The email was sent") diff --git a/toddlers/plugins/scm_request_processor.py b/toddlers/plugins/scm_request_processor.py index 6ef3bb4..4e9753f 100644 --- a/toddlers/plugins/scm_request_processor.py +++ b/toddlers/plugins/scm_request_processor.py @@ -20,7 +20,7 @@ from typing import Optional import arrow from fedora_messaging.api import Message from pagure_messages.issue_schema import IssueNewV1 -import toml +import tomllib from toddlers.base import ToddlerBase from toddlers.exceptions import ValidationError @@ -1052,7 +1052,8 @@ def main(args): args = _get_arguments(args) _setup_logging(log_level=args.log_level) - config = toml.load(args.config) + with open(args.config, "rb") as conf_fh: + config = tomllib.load(conf_fh) parsed_config = config.get("consumer_config", {}).get("default", {}) parsed_config.update( config.get("consumer_config", {}).get("scm_request_processor", {})