From e0ce04154dcd0ec07a9851e443fe977419c4978d Mon Sep 17 00:00:00 2001 From: Siddharth Sharma Date: Mon, 31 Jul 2023 16:51:40 -0400 Subject: [PATCH] Small refactors --- elliottlib/cli/find_builds_cli.py | 23 +++++++++++++-------- tests/test_find_builds_cli.py | 34 +++++++------------------------ 2 files changed, 21 insertions(+), 36 deletions(-) diff --git a/elliottlib/cli/find_builds_cli.py b/elliottlib/cli/find_builds_cli.py index 6a5d91ed..934e55c4 100644 --- a/elliottlib/cli/find_builds_cli.py +++ b/elliottlib/cli/find_builds_cli.py @@ -10,7 +10,7 @@ from errata_tool import ErrataException import elliottlib -from elliottlib import Runtime, brew, constants, errata, logutil +from elliottlib import Runtime, brew, constants, logutil from elliottlib import exectools from elliottlib.assembly import assembly_metadata_config, assembly_rhcos_config from elliottlib.build_finder import BuildFinder @@ -24,6 +24,10 @@ isolate_el_version_in_brew_tag, parallel_results_with_progress, pbar_header, progress_func, red_print, yellow_print) +from elliottlib.errata import (get_metadata_comments_json, + get_brew_build, + Advisory, + get_brew_builds) LOGGER = logutil.getLogger(__name__) @@ -159,7 +163,7 @@ async def find_builds_cli(runtime: Runtime, advisory, default_advisory_type, bui green_prefix('Fetching builds...') unshipped_nvrps = _fetch_nvrps_by_nvr_or_id(builds, tag_pv_map, ignore_product_version=remove, brew_session=brew_session) elif clean: - unshipped_builds = errata.get_brew_builds(advisory) + unshipped_builds = get_brew_builds(advisory) elif from_diff: unshipped_nvrps = _fetch_builds_from_diff(from_diff[0], from_diff[1], tag_pv_map) else: @@ -198,7 +202,7 @@ async def find_builds_cli(runtime: Runtime, advisory, default_advisory_type, bui 'Hold on a moment, fetching buildinfos from Errata Tool...', unshipped_builds if clean else unshipped_nvrps) - if not clean and not remove: + if not (clean or remove): # if is --clean then batch fetch from Erratum no need to fetch them individually # if is not for --clean fetch individually using nvrp tuples then get specific # elliottlib.brew.Build Objects by get_brew_build() @@ -207,10 +211,11 @@ async def find_builds_cli(runtime: Runtime, advisory, default_advisory_type, bui # Build(atomic-openshift-descheduler-container-v4.3.23-202005250821). unshipped_builds = parallel_results_with_progress( unshipped_nvrps, - lambda nvrp: elliottlib.errata.get_brew_build('{}-{}-{}'.format(nvrp[0], nvrp[1], nvrp[2]), nvrp[3], session=requests.Session()) + lambda nvrp: get_brew_build(f'{nvrp[0]}-{nvrp[1]}-{nvrp[2]}', + nvrp[3], session=requests.Session()) ) previous = len(unshipped_builds) - unshipped_builds = _filter_out_inviable_builds(kind, unshipped_builds, elliottlib.errata) + unshipped_builds = _filter_out_inviable_builds(unshipped_builds) if len(unshipped_builds) != previous: click.echo(f'Filtered out {previous - len(unshipped_builds)} inviable build(s)') @@ -237,7 +242,7 @@ async def find_builds_cli(runtime: Runtime, advisory, default_advisory_type, bui return try: - erratum = elliottlib.errata.Advisory(errata_id=advisory) + erratum = Advisory(errata_id=advisory) erratum.ensure_state('NEW_FILES') if remove: to_remove = [f"{nvrp[0]}-{nvrp[1]}-{nvrp[2]}" for nvrp in unshipped_nvrps] @@ -456,15 +461,15 @@ async def _fetch_builds_by_kind_rpm(runtime: Runtime, tag_pv_map: Dict[str, str] return nvrps -def _filter_out_inviable_builds(kind, results, errata): +def _filter_out_inviable_builds(build_objects): unshipped_builds = [] errata_version_cache = {} # avoid reloading the same errata for multiple builds - for b in results: + for b in build_objects: # check if build is attached to any existing advisory for this version in_same_version = False for eid in [e['id'] for e in b.all_errata]: if eid not in errata_version_cache: - metadata_comments_json = errata.get_metadata_comments_json(eid) + metadata_comments_json = get_metadata_comments_json(eid) if not metadata_comments_json: # Does not contain ART metadata; consider it unversioned red_print("Errata {} Does not contain ART metadata\n".format(eid)) diff --git a/tests/test_find_builds_cli.py b/tests/test_find_builds_cli.py index bd0d91aa..4cd7d704 100644 --- a/tests/test_find_builds_cli.py +++ b/tests/test_find_builds_cli.py @@ -1,7 +1,7 @@ import unittest from elliottlib.cli.find_builds_cli import _filter_out_inviable_builds, _find_shipped_builds from elliottlib.brew import Build -import elliottlib +from elliottlib.cli import find_builds_cli as build_cli from flexmock import flexmock import json from unittest import mock @@ -12,43 +12,23 @@ class TestFindBuildsCli(unittest.TestCase): Test elliott find-builds command and internal functions """ - def test_attached_errata_failed(self): - """ - Test the internal wrapper function _attached_to_open_erratum_with_correct_product_version - attached_to_open_erratum = True, product_version is also the same: - _attached_to_open_erratum_with_correct_product_version() should return [] - """ - metadata_json_list = [] + def test_filter_out_inviable_builds_inviable(self): metadata = json.loads('''{"release": "4.1", "kind": "rpm", "impetus": "cve"}''') - metadata_json_list.append(metadata) - - fake_errata = flexmock(model=elliottlib.errata, get_metadata_comments_json=lambda: 1234) - fake_errata.should_receive("get_metadata_comments_json").and_return(metadata_json_list) + flexmock(build_cli).should_receive("get_metadata_comments_json").and_return([metadata]) builds = flexmock(Build(nvr="test-1.1.1", product_version="RHEL-7-OSE-4.1")) builds.should_receive("all_errata").and_return([{"id": 12345}]) - # expect return empty list [] - self.assertEqual([], _filter_out_inviable_builds("image", [builds], fake_errata)) + self.assertEqual([], _filter_out_inviable_builds([builds])) - def test_attached_errata_succeed(self): - """ - Test the internal wrapper function _attached_to_open_erratum_with_correct_product_version - attached_to_open_erratum = True but product_version is not same: - _attached_to_open_erratum_with_correct_product_version() should return [Build("test-1.1.1")] - """ - metadata_json_list = [] + def test_filter_out_inviable_builds_viable(self): metadata = json.loads('''{"release": "4.1", "kind": "rpm", "impetus": "cve"}''') - metadata_json_list.append(metadata) - - fake_errata = flexmock(model=elliottlib.errata, get_metadata_comments_json=lambda: 1234) - fake_errata.should_receive("get_metadata_comments_json").and_return(metadata_json_list) + flexmock(build_cli).should_receive("get_metadata_comments_json").and_return([metadata]) builds = flexmock(Build(nvr="test-1.1.1", product_version="RHEL-7-OSE-4.5")) builds.should_receive("all_errata").and_return([{"id": 12345}]) - # expect return list with one build - self.assertEqual([Build("test-1.1.1")], _filter_out_inviable_builds("image", [builds], fake_errata)) + self.assertEqual([Build("test-1.1.1")], _filter_out_inviable_builds([builds])) @mock.patch("elliottlib.brew.get_builds_tags") def test_find_shipped_builds(self, get_builds_tags: mock.MagicMock):