From b189cdaff2bb42d7cb1cf5521f6bcd055513b37b Mon Sep 17 00:00:00 2001 From: Samyak Jain Date: Oct 04 2024 08:29:20 +0000 Subject: Fix Releng#12311: Dynamically exclude Rawhide branch from fedpkg branching Signed-off-by: Samyak Jain --- diff --git a/fedpkg/utils.py b/fedpkg/utils.py index 0669c3f..d5621d3 100644 --- a/fedpkg/utils.py +++ b/fedpkg/utils.py @@ -24,24 +24,40 @@ from urllib.parse import urlparse def query_bodhi(server_url, timeout=60): query_arg = '/?exclude_archived=True' - api_url = '{0}/releases/{1}'.format(server_url.rstrip('/'), query_arg) + api_url = f'{server_url.rstrip("/")}/releases/{query_arg}' try: - rv = requests.get(api_url, timeout=60) + rv = requests.get(api_url, timeout=timeout) except ConnectionError as error: - error_msg = ('The connection to BODHI failed while trying to get ' - 'the active release branches. The error was: {0}' - .format(str(error))) + error_msg = (f'The connection to BODHI failed while trying to get ' + f'the active release branches. The error was: {str(error)}') raise rpkgError(error_msg) if not rv.ok: - base_error_msg = ('The following error occurred while trying to ' - 'get the active release branches in Bodhi: {0}') - raise rpkgError(base_error_msg.format(rv.text)) + base_error_msg = (f'The following error occurred while trying to ' + f'get the active release branches in Bodhi: {rv.text}') + raise rpkgError(base_error_msg) rv_json = rv.json() - if rv_json['releases']: - for branch in rv_json['releases']: - yield branch['branch'] + rawhide_branch_name = None + branches = [] + + # Find the rawhide version and construct its branch name (e.g., "f42") + for release in rv_json['releases']: + if release['branch'] == 'rawhide': + rawhide_branch_name = f"f{release['version']}" + break # Stop once we've found the first rawhide branch + + # Collect all branches, including the dynamically constructed rawhide branch, maintaining order + seen_branches = set() + for release in rv_json['releases']: + branch = release['branch'] + if branch == 'rawhide': + branch = rawhide_branch_name # Replace 'rawhide' with the constructed branch name + if branch not in seen_branches: + branches.append(branch) + seen_branches.add(branch) + + return branches def new_pagure_issue(logger, url, token, title, body, cli_name): @@ -242,20 +258,35 @@ def get_pagure_branches(logger, url, namespace, repo_name): def get_release_branches(server_url): """ - Get the active Fedora release branches from Bodhi + Get the active Fedora release branches from Bodhi. - :param str url: a string of the URL to Bodhi - :return: a mapping containing the active Fedora releases and EPEL branches. + :param str server_url: The URL to Bodhi API. + :return: A mapping containing the active Fedora releases and EPEL branches, excluding rawhide. :rtype: dict """ + # Fetch all branches from Bodhi + all_branches = query_bodhi(server_url) + releases = {} - for product_version in query_bodhi(server_url): - if product_version == "rawhide": + rawhide_branch = None + + # Traverse the branches once to categorize and find the rawhide branch + for branch in all_branches: + # Assume the rawhide branch is the latest f-version branch + if branch.startswith('f') and branch[1:].isdigit(): + # Check if this is the highest f-version, hence rawhide + if rawhide_branch is None or int(branch[1:]) > int(rawhide_branch[1:]): + rawhide_branch = branch + + # Categorize branches into Fedora and EPEL, excluding rawhide + for branch in all_branches: + if branch == rawhide_branch: continue - short_name = "fedora" if product_version.startswith("f") else "epel" - releases.setdefault(short_name, set()).add(product_version) - return {key: list(value) for key, value in releases.items()} + short_name = "fedora" if branch.startswith("f") else "epel" + releases.setdefault(short_name, set()).add(branch) + + return {key: sorted(value) for key, value in releases.items()} def sl_list_to_dict(sls): diff --git a/test/test_utils.py b/test/test_utils.py index 09ddbdb..134918a 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -14,12 +14,11 @@ from configparser import NoOptionError, NoSectionError import json import unittest from unittest.mock import Mock, patch - from requests.exceptions import ConnectionError - from fedpkg import utils from freezegun import freeze_time from pyrpkg.errors import rpkgError +import requests class TestUtils(unittest.TestCase): @@ -100,11 +99,18 @@ class TestUtils(unittest.TestCase): {'name': 'F39', 'branch': 'f39'}, {'name': 'F39C', 'branch': 'f39'}, {'name': 'F39F', 'branch': 'f39'}, + {'name': 'F40', 'branch': 'f40'}, + {'name': 'F40C', 'branch': 'f40'}, + {'name': 'F40F', 'branch': 'f40'}, + {'name': 'F41', 'branch': 'f41'}, + {'name': 'F41C', 'branch': 'f41'}, + {'name': 'F41F', 'branch': 'f41'}, + {'name': 'F42', 'branch': 'f42'}, ], 'page': 1, 'pages': 1, 'rows_per_page': 20, 'total': 12} mock_request_get.return_value = mock_rv expected = { 'epel': ['epel7', 'epel8', 'epel8-next', 'epel9', 'epel9-next'], - 'fedora': ['f38', 'f38m', 'f39'], + 'fedora': ['f38', 'f38m', 'f39', 'f40', 'f41'], } actual = utils.get_release_branches('http://src.local') actual_sorted = {key: sorted(value) for key, value in sorted(actual.items())} @@ -433,43 +439,124 @@ class TestNewPagureIssue(unittest.TestCase): @patch("requests.get") class TestQueryBodhi(unittest.TestCase): - """Test utils.query_bodhi""" + """Test suite for utils.query_bodhi""" - def test_connection_error(self, get): - get.side_effect = ConnectionError + def test_connection_error(self, mock_get): + """Test that a ConnectionError raises an rpkgError with the correct message.""" + # Simulate a connection error when making the get request + mock_get.side_effect = requests.exceptions.ConnectionError('Mocked connection error') + + with self.assertRaises(rpkgError) as cm: + utils.query_bodhi('http://localhost/') + + self.assertIn( + 'The connection to BODHI failed while trying to get the active release branches. ' + 'The error was: Mocked connection error', + str(cm.exception), + "Expected error message is missing or incorrect" + ) + + def test_response_not_ok(self, mock_get): + """Test that a non-OK response raises an rpkgError with the correct message.""" + # Mock a response with .ok as False and a text message + mock_rv = Mock() + mock_rv.ok = False + mock_rv.text = 'Mocked error message' + mock_get.return_value = mock_rv + + with self.assertRaises(rpkgError) as cm: + utils.query_bodhi('http://localhost/') + + self.assertIn( + 'The following error occurred while trying to get the active release ' + 'branches in Bodhi: Mocked error message', + str(cm.exception), + "Expected error message is missing or incorrect" + ) + def test_read_data_normally(self, mock_get): + """Test that query_bodhi returns the correct branch list from a normal response.""" + # Mock a valid response from the Bodhi API + mock_rv = Mock() + mock_rv.ok = True + mock_rv.json.return_value = { + 'releases': [ + {'name': 'F40', 'branch': 'f40'}, + {'name': 'F41', 'branch': 'f41'}, + {'name': 'Rawhide Release', 'branch': 'rawhide', 'version': '42'}, + {'name': 'EPEL9', 'branch': 'epel9'}, + {'name': 'EPEL8', 'branch': 'epel8'} + ] + } + mock_get.return_value = mock_rv + + # The expected branch list to be returned by query_bodhi + expected = ['f40', 'f41', 'f42', 'epel9', 'epel8'] + + # Call query_bodhi and compare the result to the expected list result = utils.query_bodhi('http://localhost/') - self.assertRaisesRegex( - rpkgError, 'The connection to BODHI failed', - list, result) + self.assertEqual(result, expected, f"Expected branches {expected}, but got {result}") + + def test_rawhide_not_present(self, mock_get): + """Test that if rawhide is not present in the releases, + it does not construct a rawhide branch.""" + # Mock a response without a rawhide entry + mock_rv = Mock() + mock_rv.ok = True + mock_rv.json.return_value = { + 'releases': [ + {'name': 'F40', 'branch': 'f40'}, + {'name': 'F41', 'branch': 'f41'}, + {'name': 'EPEL9', 'branch': 'epel9'}, + {'name': 'EPEL8', 'branch': 'epel8'} + ] + } + mock_get.return_value = mock_rv - def test_response_not_ok(self, get): - get.return_value.ok = False + # The expected branch list without a rawhide branch + expected = ['f40', 'f41', 'epel9', 'epel8'] + # Call query_bodhi and compare the result to the expected list result = utils.query_bodhi('http://localhost/') - self.assertRaisesRegex( - rpkgError, 'The following error occurred', - list, result) + self.assertEqual(result, expected, f"Expected branches {expected}, but got {result}") - def test_read_yield_data_normally(self, get): - rv = Mock() - rv.ok = True - rv.json.side_effect = [ - {'releases': [ - {'name': 'item1', 'branch': 'item2'}, - {'name': 'item5', 'branch': 'item6'}, - {'name': 'item3', 'branch': 'item4'}, - ]} - ] - get.return_value = rv + def test_multiple_rawhide_entries(self, mock_get): + """Test that only the first rawhide entry is used to construct the rawhide branch name.""" + # Mock a response with multiple rawhide entries + mock_rv = Mock() + mock_rv.ok = True + mock_rv.json.return_value = { + 'releases': [ + {'name': 'F40', 'branch': 'f40'}, + {'name': 'F41', 'branch': 'f41'}, + {'name': 'Rawhide Release', 'branch': 'rawhide', 'version': '42'}, + {'name': 'Another Rawhide', 'branch': 'rawhide', 'version': '43'}, + {'name': 'EPEL9', 'branch': 'epel9'} + ] + } + mock_get.return_value = mock_rv + + # The expected branch list should only include 'f42' as the rawhide branch + expected = ['f40', 'f41', 'f42', 'epel9'] + # Call query_bodhi and compare the result to the expected list result = utils.query_bodhi('http://localhost/') - v = next(result) - self.assertEqual('item2', v) - v = next(result) - self.assertEqual('item6', v) - v = next(result) - self.assertEqual('item4', v) + self.assertEqual(result, expected, f"Expected branches {expected}, but got {result}") + + def test_empty_releases(self, mock_get): + """Test that an empty releases list returns an empty branches list.""" + # Mock a response with no releases + mock_rv = Mock() + mock_rv.ok = True + mock_rv.json.return_value = {'releases': []} + mock_get.return_value = mock_rv + + # Expected branches should be an empty list + expected = [] + + # Call query_bodhi and compare the result to the expected empty list + result = utils.query_bodhi('http://localhost/') + self.assertEqual(result, expected, "Expected an empty list of branches, but got some") class TestGetStreamBranches(unittest.TestCase): @@ -481,13 +568,18 @@ class TestGetStreamBranches(unittest.TestCase): logger = Mock() apibaseurl = "https://bodhiurl" rv = Mock(ok=True) - rv.json.return_value = {'releases': [ - {'name': 'ELN', 'branch': 'eln'}, - {'name': 'F40', 'branch': 'rawhide'}, - {'name': 'F40C', 'branch': 'f40'}, - {'name': 'epel8', 'branch': 'epel8'}, - ], 'page': 1, 'pages': 1, 'rows_per_page': 20, 'total': 3} - {'releases': [], 'page': 1, 'pages': 0, 'rows_per_page': 20, 'total': 0} + rv.json.return_value = { + 'releases': [ + {'name': 'ELN', 'branch': 'eln', 'version': '8'}, + {'name': 'F40', 'branch': 'rawhide', 'version': '40'}, + {'name': 'F40C', 'branch': 'f40', 'version': '40'}, + {'name': 'epel8', 'branch': 'epel8', 'version': '8'}, + ], + 'page': 1, + 'pages': 1, + 'rows_per_page': 20, + 'total': 4 + } get.return_value = rv pagure_branches.return_value = ["epel7", "epel8", "epel9", "f38", "f39"] @@ -572,3 +664,23 @@ class TestGetFedoraReleaseState(unittest.TestCase): self.assertRaisesRegex(rpkgError, r"Could not get release state for Fedora \(F30M\): " "No option 'releases_service' in section: 'fedpkg.bodhi'.", utils.get_fedora_release_state, config, 'fedpkg', 'F30M') + + +class TestGetReleaseBranches(unittest.TestCase): + """Test utils.get_release_branches""" + + @patch('fedpkg.utils.query_bodhi') + def test_get_release_branches_excludes_rawhide(self, mock_query_bodhi): + """Test that get_release_branches correctly excludes the rawhide branch.""" + # Mocking the branches returned by query_bodhi + mock_query_bodhi.return_value = ['f40', 'f41', 'f42', 'epel9', 'epel8'] # f42 is rawhide + + expected_output = { + 'fedora': ['f40', 'f41'], + 'epel': ['epel8', 'epel9'] + } + + # Run the method and assert that rawhide branch (f42) is excluded + result = utils.get_release_branches('https://bodhi.fedoraproject.org/releases/') + self.assertEqual(result, expected_output, + msg=f"Expected branches {expected_output}, but got {result}") diff --git a/test/utils.py b/test/utils.py index b4ce20c..f4e71f3 100644 --- a/test/utils.py +++ b/test/utils.py @@ -17,7 +17,6 @@ import shutil import subprocess import tempfile import unittest - import fedpkg.cli import pyrpkg from fedpkg import Commands