Small refactors

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))

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):