PR#211: Add EPEL 10 support to scm_request_processor with some adjusted tests - fedora-infra/toddlers

fedora-infra / toddlers

#211 Add EPEL 10 support to scm_request_processor with some adjusted tests

Merged 6 months ago by zlopez. Opened 6 months ago by carlwgeorge.

fedora-infra/ carlwgeorge/toddlers epel10 into main

Add EPEL 10 support to scm_request_processor

Carl George • 6 months ago

6b4f8e0

Carl George • 6 months ago

2f3e457

Combine similar bad response scm_request_processor tests

Carl George • 6 months ago

2f533e7

Remove epel6/epel7 code and tests

Carl George • 6 months ago

cc8f269

tests/plugins/test_clean_retired_packages.py

file modified

+4 -4

		`@@ -73,13 +73,13 @@`
		`pdc_branches = MagicMock()`
		`pdc_branches.json.return_value = {`
		`"flatpak": {`
		`- "bluez-gnome": [["master", False], ["epel8", False], ["epel7", False]],`
		`+ "bluez-gnome": [["master", False], ["epel9", False], ["epel8", False]],`
		`},`
		`"rpm": {`
		`"trojan": [["master", True], ["f33", True], ["f32", True]],`
		`- "pigment": [["master", False], ["epel8", True], ["epel7", True]],`
		`- "bluez-gnome": [["master", False], ["epel8", False], ["epel7", False]],`
		`- "guake": [["master", False], ["epel8", False], ["epel7", False]],`
		`+ "pigment": [["master", False], ["epel9", True], ["epel8", True]],`
		`+ "bluez-gnome": [["master", False], ["epel9", False], ["epel8", False]],`
		`+ "guake": [["master", False], ["epel9", False], ["epel8", False]],`
		`},`
		`}`
		`distgit_info_bluez_gnome = MagicMock()`

tests/plugins/test_distgit_bugzilla_sync.py

file modified

+9 -9

		`@@ -35,8 +35,8 @@`
		`"products": {`
		`"Fedora": {"namespace": "rpms", "versions": ["rawhide", 33, 32, 31]},`
		`"Fedora EPEL": {`
		`- "branch_regex": r"^e(pe)?l\d+$",`
		`- "versions": ["epel8", "epel7", "el6"],`
		`+ "branch_regex": r"^epel\d+$",`
		`+ "versions": ["epel9", "epel8"],`
		`},`
		`},`
		`}`
		`@@ -144,11 +144,11 @@`
		`assert toddler.namespace_to_product == {"rpms": "Fedora"}`

		`assert toddler.product_to_branch_regex == {`
		`- "Fedora EPEL": re.compile(r"^e(pe)?l\d+$")`
		`+ "Fedora EPEL": re.compile(r"^epel\d+$")`
		`}`

		`assert toddler.branch_regex_to_product == {`
		`- re.compile(r"^e(pe)?l\d+$"): "Fedora EPEL"`
		`+ re.compile(r"^epel\d+$"): "Fedora EPEL"`
		`}`

		`assert len(toddler.errors) == 0`
		`@@ -215,7 +215,7 @@`
		`package="foo",`
		`component="dummy",`
		`cc_list=["slaanesh@fedoraproject.org", "tzeentch@fedoraproject.org"],`
		`- versions=["epel8", "epel7", "el6"],`
		`+ versions=["epel9", "epel8"],`
		`description="Summary",`
		`fas_users_info={`
		`"khorne@fedoraproject.org": "Khorne",`
		`@@ -284,11 +284,11 @@`
		`assert toddler.namespace_to_product == {"rpms": "Fedora"}`

		`assert toddler.product_to_branch_regex == {`
		`- "Fedora EPEL": re.compile(r"^e(pe)?l\d+$")`
		`+ "Fedora EPEL": re.compile(r"^epel\d+$")`
		`}`

		`assert toddler.branch_regex_to_product == {`
		`- re.compile(r"^e(pe)?l\d+$"): "Fedora EPEL"`
		`+ re.compile(r"^epel\d+$"): "Fedora EPEL"`
		`}`

		`assert len(toddler.errors) == 0`
		`@@ -1140,7 +1140,7 @@`
		`package="foo",`
		`component="dummy",`
		`cc_list=["chaos@fedoraproject.org", "tzeentch@fedoraproject.org"],`
		`- versions=["epel8", "epel7", "el6"],`
		`+ versions=["epel9", "epel8"],`
		`description="Summary",`
		`fas_users_info={`
		`"orphan@fedoraproject.org": "orphan",`
		`@@ -1232,7 +1232,7 @@`
		`package="foo",`
		`component="dummy",`
		`cc_list=["tzeentch@fedoraproject.org"],`
		`- versions=["epel8", "epel7", "el6"],`
		`+ versions=["epel9", "epel8"],`
		`description="Summary",`
		`fas_users_info={`
		`"khorne@fedoraproject.org": "Khorne",`

tests/plugins/test_scm_request_processor.py

file modified

+65 -151

		`@@ -2799,36 +2799,23 @@`
		`self.toddler.dist_git = Mock()`
		`self.toddler.requests_session = Mock()`

		`- def test_valid_epel_package_bad_response(self):`
		`- """`
		`- Assert that method will raise error when bad response is retrieved.`
		`- """`
		`- # Preparation`
		`- name = "package"`
		`- branch = "epel8"`
		`-`
		`- mock_response = Mock()`
		`- mock_response.status_code = 500`
		`- self.toddler.requests_session.get.return_value = mock_response`
		`-`
		`- error = "Couldn't retrieve the list of official EL packages"`
		`-`
		`- # Method to test`
		`- with pytest.raises(ValidationError, match=error):`
		`- self.toddler.valid_epel_package(name=name, branch=branch)`
		`-`
		`- # Asserts`
		`- self.toddler.requests_session.get.assert_called_with(`
		`- "https://infrastructure.fedoraproject.org/repo/json/pkg_el8.json"`
		`- )`
		`-`
		`- def test_valid_epel_package_bad_response_epel9(self):`
		`+ @pytest.mark.parametrize(`
		`+ "branch",`
		`+ [`
		`+ "epel8",`
		`+ "epel9",`
		`+ "epel9-next",`
		`+ "epel10",`
		`+ "epel10.0",`
		`+ ],`
		`+ )`
		`+ def test_valid_epel_package_bad_response(self, branch):`
		`"""`
		`Assert that method will raise error when bad response is retrieved.`
		`"""`
		`# Preparation`
		`name = "package"`
		`- branch = "epel9"`
		`+ major_version = int(re.findall(r"^epel(\d+)(?:\.\d+)?(?:-next)?$", branch)[0])`

		`mock_response = Mock()`
		`mock_response.status_code = 500`
		`@@ -2841,149 +2828,76 @@`
		`self.toddler.valid_epel_package(name=name, branch=branch)`

		`# Asserts`
		`- self.toddler.requests_session.get.assert_called_with(`
		`- "https://composes.stream.centos.org/production/latest-CentOS-Stream/compose/metadata/rpms.json" # NOQA E501`
		`- )`
		`-`
		`- def test_valid_epel_package_epel6_noarch(self):`
		`- """`
		`- Assert that method will return False when epel6 noarch package is introduced.`
		`- """`
		`- # Preparation`
		`- name = "package"`
		`- branch = "epel6"`
		`-`
		`- mock_response = Mock()`
		`- mock_response.status_code = 200`
		`- mock_response.json.return_value = {`
		`- "arches": ["noarch", "ppc", "i386"],`
		`- "packages": {name: {"arch": ["noarch"]}},`
		`- }`
		`- self.toddler.requests_session.get.return_value = mock_response`
		`-`
		`- # Method to test`
		`- result = self.toddler.valid_epel_package(name=name, branch=branch)`
		`-`
		`- # Asserts`
		`- self.toddler.requests_session.get.assert_called_with(`
		`- "https://infrastructure.fedoraproject.org/repo/json/pkg_el6.json"`
		`- )`
		`-`
		`- assert result is False`
		`-`
		`- def test_valid_epel_package_epel7_noarch(self):`
		`- """`
		`- Assert that method will return False when epel7 noarch package is introduced.`
		`- """`
		`- # Preparation`
		`- name = "package"`
		`- branch = "epel7"`
		`-`
		`- mock_response = Mock()`
		`- mock_response.status_code = 200`
		`- mock_response.json.return_value = {`
		`- "arches": ["noarch", "ppc", "i386"],`
		`- "packages": {name: {"arch": ["noarch"]}},`
		`- }`
		`- self.toddler.requests_session.get.return_value = mock_response`
		`-`
		`- # Method to test`
		`- result = self.toddler.valid_epel_package(name=name, branch=branch)`
		`-`
		`- # Asserts`
		`- self.toddler.requests_session.get.assert_called_with(`
		`- "https://infrastructure.fedoraproject.org/repo/json/pkg_el7.json"`
		`- )`
		`-`
		`- assert result is False`
		`-`
		`- def test_valid_epel_package_correct_package(self):`
		`- """`
		`- Assert that method will return True when package is valid.`
		`- """`
		`- # Preparation`
		`- name = "package"`
		`- branch = "epel8"`
		`-`
		`- mock_response = Mock()`
		`- mock_response.status_code = 200`
		`- mock_response.json.return_value = {`
		`- "arches": ["noarch", "ppc", "i386"],`
		`- "packages": {name: {"arch": ["x86_64"]}},`
		`- }`
		`- self.toddler.requests_session.get.return_value = mock_response`
		`-`
		`- # Method to test`
		`- result = self.toddler.valid_epel_package(name=name, branch=branch)`
		`-`
		`- # Asserts`
		`- self.toddler.requests_session.get.assert_called_with(`
		`- "https://infrastructure.fedoraproject.org/repo/json/pkg_el8.json"`
		`- )`
		`-`
		`- assert result is True`
		`+ if major_version == 8:`
		`+ url = "https://infrastructure.fedoraproject.org/repo/json/pkg_el8.json" # NOQA E501`
		`+ elif major_version == 9:`
		`+ url = "https://composes.stream.centos.org/production/latest-CentOS-Stream/compose/metadata/rpms.json" # NOQA E501`
		`+ else:`
		`+ url = f"https://composes.stream.centos.org/stream-{major_version}/production/latest-CentOS-Stream/compose/metadata/rpms.json" # NOQA E501`
		`+ self.toddler.requests_session.get.assert_called_with(url)`

		`- def test_valid_epel_package_epel9(self):`
		`+ @pytest.mark.parametrize(`
		`+ "name, branch, expected",`
		`+ [`
		`+ ("hello", "epel8", True),`
		`+ ("hello", "epel9", True),`
		`+ ("hello", "epel9-next", True),`
		`+ ("hello", "epel10", True),`
		`+ ("hello", "epel10.0", True),`
		`+ ("bash", "epel8", False),`
		`+ ("bash", "epel9", False),`
		`+ ("bash", "epel9-next", False),`
		`+ ("bash", "epel10", False),`
		`+ ("bash", "epel10.0", False),`
		`+ ],`
		`+ )`
		`+ def test_valid_epel_package_correct_package(self, name, branch, expected):`
		`"""`
		`- Assert that method will return False when epel9 package already exists.`
		`+ Assert that method will return correct response about package validity.`
		`"""`
		`# Preparation`
		`- name = "package"`
		`- branch = "epel9"`
		`+ major_version = int(re.findall(r"^epel(\d+)(?:\.\d+)?(?:-next)?$", branch)[0])`

		`mock_response = Mock()`
		`mock_response.status_code = 200`
		`- mock_response.json.return_value = {`
		`- "payload": {`
		`- "rpms": {`
		`- "AppStream": {"noarch": {"package-0:2.2-1.el9.src": {}}},`
		`- "BaseOS": {"noarch": {}},`
		`- "CRB": {"noarch": {}},`
		`+ if major_version == 8:`
		`+ metadata = {"packages": {"bash": {}}}`
		`+ elif major_version == 9:`
		`+ metadata = {`
		`+ "payload": {`
		`+ "rpms": {`
		`+ "BaseOS": {"x86_64": {"bash-0:5.1.8-9.el9.src": {}}},`
		`+ "AppStream": {"x86_64": {}},`
		`+ "CRB": {"x86_64": {}},`
		`+ }`
		`}`
		`}`
		`- }`
		`- self.toddler.requests_session.get.return_value = mock_response`
		`-`
		`- # Method to test`
		`- result = self.toddler.valid_epel_package(name=name, branch=branch)`
		`-`
		`- # Asserts`
		`- self.toddler.requests_session.get.assert_called_with(`
		`- "https://composes.stream.centos.org/production/latest-CentOS-Stream/compose/metadata/rpms.json" # NOQA E501`
		`- )`
		`-`
		`- assert result is False`
		`-`
		`- def test_valid_epel_package_epel9_noarch_correct_package(self):`
		`- """`
		`- Assert that method will return True when package is valid.`
		`- """`
		`- # Preparation`
		`- name = "package"`
		`- branch = "epel9"`
		`-`
		`- mock_response = Mock()`
		`- mock_response.status_code = 200`
		`- mock_response.json.return_value = {`
		`- "payload": {`
		`- "rpms": {`
		`- "AppStream": {"noarch": {}},`
		`- "BaseOS": {"noarch": {}},`
		`- "CRB": {"noarch": {}},`
		`+ else:`
		`+ metadata = {`
		`+ "payload": {`
		`+ "rpms": {`
		`+ "BaseOS": {"x86_64": {"bash-0:5.2.26-4.el10.src": {}}},`
		`+ "AppStream": {"x86_64": {}},`
		`+ "CRB": {"x86_64": {}},`
		`+ }`
		`}`
		`}`
		`- }`
		`+ mock_response.json.return_value = metadata`
		`self.toddler.requests_session.get.return_value = mock_response`

		`# Method to test`
		`result = self.toddler.valid_epel_package(name=name, branch=branch)`

		`# Asserts`
		`- self.toddler.requests_session.get.assert_called_with(`
		`- "https://composes.stream.centos.org/production/latest-CentOS-Stream/compose/metadata/rpms.json" # NOQA E501`
		`- )`
		`-`
		`- assert result is True`
		`+ if major_version == 8:`
		`+ url = "https://infrastructure.fedoraproject.org/repo/json/pkg_el8.json" # NOQA E501`
		`+ elif major_version == 9:`
		`+ url = "https://composes.stream.centos.org/production/latest-CentOS-Stream/compose/metadata/rpms.json" # NOQA E501`
		`+ else:`
		`+ url = f"https://composes.stream.centos.org/stream-{major_version}/production/latest-CentOS-Stream/compose/metadata/rpms.json" # NOQA E501`
		`+ self.toddler.requests_session.get.assert_called_with(url)`
		`+`
		`+ assert result is expected`


		`class TestMain:`

toddlers.toml.example

file modified

+2 -3

		`@@ -156,9 +156,8 @@`
		`versions = []`

		`[consumer_config.distgit_bugzilla_sync.products."Fedora EPEL"]`
		`- # To simplify to '^epel\d+$' once EL6 is EOL.`
		`- branch_regex = '^e(pe)?l\d+$'`
		`- versions = ["epel8", "epel7", "el6"]`
		`+ branch_regex = '^epel\d+$'`
		`+ versions = ["epel9", "epel8"]`

		`[consumer_config.packager_bugzilla_sync]`

toddlers/plugins/scm_request_processor.py

file modified

+12 -26

		`@@ -31,7 +31,7 @@`
		`# Regex for project name validation`
		`PROJECT_NAME_REGEX = r"^[a-zA-Z0-9_][a-zA-Z0-9-_.+]*$"`
		`# Regex for epel branch validation`
		`- EPEL_REGEX = r"^epel\d+(?:-playground\|-next)?$"`
		`+ EPEL_REGEX = r"^epel\d+(?:\.\d+)?(?:-next)?$"`

		`# Where to look for the scm-requests tickets`
		`PROJECT_NAMESPACE = "releng/fedora-scm-requests"`
		`@@ -993,7 +993,7 @@`

		`Params:`
		`name: a string of the package name`
		`- branch: a string of the EPEL branch name (e.g. epel7)`
		`+ branch: a string of the EPEL branch name (e.g. epel9)`

		`Returns:`
		`If package is valid EPEL package.`
		`@@ -1001,15 +1001,16 @@`
		`Raises:`
		`ValidationError`: When we can't retrieve list of official EL packages.
		`"""`
		`- # Extract any digits in the branch name to determine the EL version`
		`- version = "".join([i for i in branch if re.match(r"\d", i)])`
		`+ # Extract the major version`
		`+ major_version = int(re.findall(r"^epel(\d+)(?:\.\d+)?(?:-next)?$", branch)[0])`

		`# Starting with epel9 and epel9-next, check against CentOS compose metadata.`
		`- if int(version) >= 9:`
		`- # Currently we only have a latest symlink. In the future we'll need`
		`- # separate latest symlinks that include the major version.`
		`+ if major_version >= 9:`
		`# https://bugzilla.redhat.com/show_bug.cgi?id=2005139`
		`- url = "https://composes.stream.centos.org/production/latest-CentOS-Stream/compose/metadata/rpms.json" # NOQA E501`
		`+ if major_version == 9:`
		`+ url = "https://composes.stream.centos.org/production/latest-CentOS-Stream/compose/metadata/rpms.json" # NOQA E501`
		`+ else:`
		`+ url = f"https://composes.stream.centos.org/stream-{major_version}/production/latest-CentOS-Stream/compose/metadata/rpms.json" # NOQA E501`
		`response = self.requests_session.get(url)`

		`if response.status_code != 200:`
		`@@ -1028,7 +1029,7 @@`
		`if name in pkg_names:`
		`return False`
		`else:`
		`- url = f"https://infrastructure.fedoraproject.org/repo/json/pkg_el{version}.json"`
		`+ url = f"https://infrastructure.fedoraproject.org/repo/json/pkg_el{major_version}.json"`
		`response = self.requests_session.get(url)`

		`if response.status_code != 200:`
		`@@ -1037,23 +1038,8 @@`
		`)`
		`rv_json = response.json()`

		`- # Remove noarch from this because noarch is treated specially`
		`- all_arches = set(rv_json["arches"]) - set(["noarch"])`
		`- # On EL6, also remove ppc and i386 as many packages will`
		`- # have these arches missing and cause false positives`
		`- if int(version) == 6:`
		`- all_arches = all_arches - set(["ppc", "i386"])`
		`- # On EL7 and later, also remove ppc and i686 as many packages will`
		`- # have these arches missing and cause false positives`
		`- elif int(version) >= 7:`
		`- all_arches = all_arches - set(["ppc", "i686"])`
		`- for pkg_name, pkg_info in rv_json["packages"].items():`
		`- # If the EL package is noarch only or is available on all supported`
		`- # arches, then don't allow an EPEL branch`
		`- if pkg_name == name:`
		`- pkg_arches = set(pkg_info["arch"])`
		`- if pkg_arches == set(["noarch"]) or not (all_arches - pkg_arches):`
		`- return False`
		`+ if name in rv_json["packages"].keys():`
		`+ return False`
		`return True`

carlwgeorge commented 6 months ago

This PR primarily adds EPEL 10 support to scm_request_processor. To make those changes testable, it also parametrizes a few tests. See individual commit messages for more details.

zuul commented 6 months ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/ef366042235e4d81b068b720e137f7e0

fi-tox-mypy : FAILURE in 9m 49s
fi-tox-format : SUCCESS in 3m 56s
fi-tox-lint : SUCCESS in 4m 26s
fi-tox-python39 : SUCCESS in 9m 28s
fi-tox-python310 : SUCCESS in 9m 03s
fi-tox-python311 : SUCCESS in 8m 18s
fi-tox-python312 : SUCCESS in 8m 27s

carlwgeorge commented 6 months ago

toddlers/plugins/scm_request_processor.py:1006: error: Item "None" of "Match[str] | None" has no attribute "group"  [union-attr]

I'm not very familiar with mypy, any suggestions for fixing this one?

Edit: Switching from re.match to re.findall seems to work around it.

Edited 6 months ago by carlwgeorge

rebased onto f254995

6 months ago

zuul commented 6 months ago

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/76120be238074ea89e13943a79793980

fi-tox-mypy : SUCCESS in 8m 56s
fi-tox-format : SUCCESS in 4m 04s
fi-tox-lint : SUCCESS in 4m 48s
fi-tox-python39 : SUCCESS in 9m 03s
fi-tox-python310 : SUCCESS in 8m 34s
fi-tox-python311 : SUCCESS in 7m 57s
fi-tox-python312 : SUCCESS in 8m 36s

Pull-Request has been merged by zlopez

6 months ago

Metadata

Assignee

None

Tags

No Tags

Changes Summary 5

+4 -4

file changed

tests/plugins/test_clean_retired_packages.py

+9 -9

file changed

tests/plugins/test_distgit_bugzilla_sync.py

+65 -151

file changed

tests/plugins/test_scm_request_processor.py

+2 -3

file changed

toddlers.toml.example

+12 -26

file changed

toddlers/plugins/scm_request_processor.py

fedora-infra / toddlers

Source Code

#211 Add EPEL 10 support to scm_request_processor with some adjusted tests Merged 6 months ago by zlopez. Opened 6 months ago by carlwgeorge. fedora-infra/ carlwgeorge/toddlers epel10 into main

Metadata

Changes Summary 5

#211 Add EPEL 10 support to scm_request_processor with some adjusted tests

Merged 6 months ago by zlopez. Opened 6 months ago by carlwgeorge.

fedora-infra/ carlwgeorge/toddlers epel10 into main