Merge #518 Migrate test phase to dnf repoclosure

doc/configuration.rst

@@ -152,6 +152,12 @@ Options
     ([*str*]) -- list of variants which should be included; if undefined, all
     variants from variants.xml will be included
 
+**repoclosure_backend**
+    (*str*) -- Select which tool should be used to run repoclosure over created
+    repositories. By default ``yum`` is used, but you can switch to ``dnf``.
+    Please note that when ``dnf`` is used, the build dependencies check is
+    skipped.
+
 
 Example
 -------

pungi/checks.py

@@ -468,6 +468,11 @@ def _make_schema():
                 "type": "boolean",
                 "default": False,
             },
+            "repoclosure_backend": {
+                "type": "string",
+                "default": "yum",
+                "enum": ["yum", "dnf"],
+            },
 
             "hashed_directories": {
                 "type": "boolean",

pungi/phases/test.py

@@ -18,7 +18,7 @@ import os
 
 from kobo.shortcuts import run
 
-from pungi.wrappers.repoclosure import RepoclosureWrapper
+from pungi.wrappers import repoclosure
 from pungi.arch import get_valid_arches
 from pungi.phases.base import PhaseBase
 from pungi.phases.gather import get_lookaside_repos
@@ -34,8 +34,6 @@ class TestPhase(PhaseBase):
 
 
 def run_repoclosure(compose):
-    repoclosure = RepoclosureWrapper()
-
     # TODO: Special handling for src packages (use repoclosure param builddeps)
 
     msg = "Running repoclosure"
@@ -66,7 +64,8 @@ def run_repoclosure(compose):
                 for i, lookaside_url in enumerate(get_lookaside_repos(compose, arch, variant)):
                     lookaside["lookaside-%s.%s-%s" % (variant.uid, arch, i)] = lookaside_url
 
-            cmd = repoclosure.get_repoclosure_cmd(repos=repos, lookaside=lookaside, arch=arches)
+            cmd = repoclosure.get_repoclosure_cmd(backend=compose.conf['repoclosure_backend'],
+                                                  repos=repos, lookaside=lookaside, arch=arches)
             # Use temp working directory directory as workaround for
             # https://bugzilla.redhat.com/show_bug.cgi?id=795137
             tmp_dir = compose.mkdtemp(prefix="repoclosure_")
@@ -89,7 +88,11 @@ def run_repoclosure(compose):
     # In this case, it's an obvious bug in the test.
 
     # check BuildRequires (self-hosting)
-    cmd = repoclosure.get_repoclosure_cmd(repos=all_repos, arch=all_arches, builddeps=True)
+    try:
+        cmd = repoclosure.get_repoclosure_cmd(backend=compose.conf['repoclosure_backend'],
+                                              repos=all_repos, arch=all_arches, builddeps=True)
+    except RuntimeError as exc:
+        compose.log_error('%s, skipping builddeps check...' % str(exc))
     # Use temp working directory directory as workaround for
     # https://bugzilla.redhat.com/show_bug.cgi?id=795137
     tmp_dir = compose.mkdtemp(prefix="repoclosure_")

pungi/wrappers/repoclosure.py

@@ -19,56 +19,48 @@ import os
 from kobo.shortcuts import force_list
 
 
-class RepoclosureWrapper(object):
+def get_repoclosure_cmd(backend='yum', arch=None, builddeps=False,
+                        repos=None, lookaside=None):
+    if backend == 'dnf' and builddeps:
+        raise RuntimeError('dnf repoclosure does not support builddeps')
 
-    def __init__(self):
+    cmds = {
-        self.actual_id = 0
+        'yum': {'cmd': ['/usr/bin/repoclosure'], 'repoarg': '--repoid=%s', 'lookaside': '--lookaside=%s'},
+        'dnf': {'cmd': ['dnf', 'repoclosure'], 'repoarg': '--repo=%s', 'lookaside': '--repo=%s'},
+    }
+    try:
+        cmd = cmds[backend]['cmd']
+    except KeyError:
+        raise RuntimeError('Unknown repoclosure backend: %s' % backend)
 
-    def get_repoclosure_cmd(self, config=None, arch=None, basearch=None, builddeps=False,
+    # There are options that are not exposed here, because we don't need
-                            repos=None, lookaside=None, tempcache=False, quiet=False, newest=False, pkg=None, group=None):
+    # them.
 
-        cmd = ["/usr/bin/repoclosure"]
+    for i in force_list(arch or []):
-
-        if config:
-            cmd.append("--config=%s" % config)
-
-        if arch:
-            for i in force_list(arch):
         cmd.append("--arch=%s" % i)
 
-        if basearch:
-            cmd.append("--basearch=%s" % basearch)
-
     if builddeps:
         cmd.append("--builddeps")
 
-        if tempcache:
-            cmd.append("--tempcache")
-
-        if quiet:
-            cmd.append("--quiet")
-
-        if newest:
-            cmd.append("--newest")
-
     repos = repos or {}
     for repo_id, repo_path in repos.iteritems():
-            if "://" not in repo_path:
+        cmd.append("--repofrompath=%s,%s" % (repo_id, _to_url(repo_path)))
-                repo_path = "file://%s" % os.path.abspath(repo_path)
+        cmd.append(cmds[backend]['repoarg'] % repo_id)
-            cmd.append("--repofrompath=%s,%s" % (repo_id, repo_path))
+        if backend == 'dnf':
-            cmd.append("--repoid=%s" % repo_id)
+            # For dnf we want to add all repos with the --repo option (which
+            # enables only those and not any system repo), and the repos to
+            # check are also listed with the --check option.
+            cmd.append('--check=%s' % repo_id)
 
     lookaside = lookaside or {}
     for repo_id, repo_path in lookaside.iteritems():
-            if "://" not in repo_path:
+        cmd.append("--repofrompath=%s,%s" % (repo_id, _to_url(repo_path)))
-                repo_path = "file://%s" % os.path.abspath(repo_path)
+        cmd.append(cmds[backend]['lookaside'] % repo_id)
-            cmd.append("--repofrompath=%s,%s" % (repo_id, repo_path))
-            cmd.append("--lookaside=%s" % repo_id)
-
-        if pkg:
-            cmd.append("--pkg=%s" % pkg)
-
-        if group:
-            cmd.append("--group=%s" % group)
 
     return cmd
+
+
+def _to_url(path):
+    if "://" not in path:
+        return "file://%s" % os.path.abspath(path)
+    return path

tests/helpers.py

@@ -30,6 +30,10 @@ class PungiTestCase(unittest.TestCase):
 
 
 class MockVariant(mock.Mock):
+    def __init__(self, *args, **kwargs):
+        super(MockVariant, self).__init__(*args, **kwargs)
+        self.parent = kwargs.get('parent', None)
+
     def __str__(self):
         return self.uid
 

tests/test_config.py

@@ -389,5 +389,17 @@ class TestRegexValidation(ConfigTestCase):
             [])
 
 
+class RepoclosureTestCase(ConfigTestCase):
+    def test_invalid_backend(self):
+        cfg = load_config(
+            PKGSET_REPOS,
+            repoclosure_backend='fnd',  # Intentionally with a typo
+        )
+
+        self.assertValidation(
+            cfg,
+            ["Failed validation in repoclosure_backend: 'fnd' is not one of ['yum', 'dnf']"])
+
+
 if __name__ == '__main__':
     unittest.main()

tests/test_repoclosure_wrapper.py

@@ -0,0 +1,93 @@
+# -*- coding: utf-8 -*-
+
+try:
+    import unittest2 as unittest
+except ImportError:
+    import unittest
+
+import os
+import sys
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
+
+from pungi.wrappers import repoclosure as rc
+
+
+class RepoclosureWrapperTestCase(unittest.TestCase):
+    def test_minimal_command(self):
+        self.assertEqual(rc.get_repoclosure_cmd(),
+                         ['/usr/bin/repoclosure'])
+
+    def test_minimal_dnf_command(self):
+        self.assertEqual(rc.get_repoclosure_cmd(backend='dnf'),
+                         ['dnf', 'repoclosure'])
+
+    def test_unknown_backend(self):
+        with self.assertRaises(RuntimeError) as ctx:
+            rc.get_repoclosure_cmd(backend='rpm')
+
+        self.assertEqual(str(ctx.exception), 'Unknown repoclosure backend: rpm')
+
+    def test_multiple_arches(self):
+        self.assertEqual(rc.get_repoclosure_cmd(arch=['x86_64', 'ppc64']),
+                         ['/usr/bin/repoclosure', '--arch=x86_64', '--arch=ppc64'])
+
+    def test_full_command(self):
+        repos = {'my-repo': '/mnt/koji/repo'}
+        lookaside = {'fedora': 'http://kojipkgs.fp.o/repo'}
+
+        cmd = rc.get_repoclosure_cmd(arch='x86_64', builddeps=True,
+                                     repos=repos, lookaside=lookaside)
+        self.assertEqual(cmd[0], '/usr/bin/repoclosure')
+        self.assertItemsEqual(
+            cmd[1:],
+            ['--arch=x86_64',
+             '--builddeps',
+             '--repofrompath=my-repo,file:///mnt/koji/repo',
+             '--repofrompath=fedora,http://kojipkgs.fp.o/repo',
+             '--repoid=my-repo',
+             '--lookaside=fedora'])
+
+    def test_full_dnf_command(self):
+        repos = {'my-repo': '/mnt/koji/repo'}
+        lookaside = {'fedora': 'http://kojipkgs.fp.o/repo'}
+
+        cmd = rc.get_repoclosure_cmd(backend='dnf', arch='x86_64',
+                                     repos=repos, lookaside=lookaside)
+        self.assertEqual(cmd[:2], ['dnf', 'repoclosure'])
+        self.assertItemsEqual(
+            cmd[2:],
+            ['--arch=x86_64',
+             '--repofrompath=my-repo,file:///mnt/koji/repo',
+             '--repofrompath=fedora,http://kojipkgs.fp.o/repo',
+             '--repo=my-repo',
+             '--check=my-repo',
+             '--repo=fedora'])
+
+    def test_expand_repo(self):
+        repos = {
+            'local': '/mnt/koji/repo',
+            'remote': 'http://kojipkgs.fp.o/repo',
+        }
+        cmd = rc.get_repoclosure_cmd(repos=repos)
+        self.assertEqual(cmd[0], '/usr/bin/repoclosure')
+        self.assertItemsEqual(
+            cmd[1:],
+            ['--repofrompath=local,file:///mnt/koji/repo',
+             '--repofrompath=remote,http://kojipkgs.fp.o/repo',
+             '--repoid=local',
+             '--repoid=remote'])
+
+    def test_expand_lookaside(self):
+        repos = {
+            'local': '/mnt/koji/repo',
+            'remote': 'http://kojipkgs.fp.o/repo',
+        }
+        cmd = rc.get_repoclosure_cmd(lookaside=repos)
+        self.assertEqual(cmd[0], '/usr/bin/repoclosure')
+        self.assertItemsEqual(
+            cmd[1:],
+            ['--repofrompath=local,file:///mnt/koji/repo',
+             '--repofrompath=remote,http://kojipkgs.fp.o/repo',
+             '--lookaside=local',
+             '--lookaside=remote'])

tests/test_test_phase.py

@@ -40,7 +40,7 @@ class TestCheckImageSanity(PungiTestCase):
 
         try:
             test_phase.check_image_sanity(compose)
-        except:
+        except Exception:
             self.fail('Failable deliverable must not raise')
 
     def test_correct_iso_does_not_raise(self):
@@ -51,7 +51,7 @@ class TestCheckImageSanity(PungiTestCase):
 
         try:
             test_phase.check_image_sanity(compose)
-        except:
+        except Exception:
             self.fail('Correct unbootable image must not raise')
 
     def test_incorrect_iso_raises(self):
@@ -87,7 +87,7 @@ class TestCheckImageSanity(PungiTestCase):
 
         try:
             test_phase.check_image_sanity(compose)
-        except:
+        except Exception:
             self.fail('Failable deliverable must not raise')
 
     def test_failable_bootable_iso_without_mbr_gpt_doesnt_raise(self):
@@ -100,7 +100,7 @@ class TestCheckImageSanity(PungiTestCase):
 
         try:
             test_phase.check_image_sanity(compose)
-        except:
+        except Exception:
             self.fail('Failable deliverable must not raise')
 
     def test_bootable_iso_with_mbr_does_not_raise(self):
@@ -111,7 +111,7 @@ class TestCheckImageSanity(PungiTestCase):
 
         try:
             test_phase.check_image_sanity(compose)
-        except:
+        except Exception:
             self.fail('Bootable image with MBR must not raise')
 
     def test_bootable_iso_with_gpt_does_not_raise(self):
@@ -122,7 +122,7 @@ class TestCheckImageSanity(PungiTestCase):
 
         try:
             test_phase.check_image_sanity(compose)
-        except:
+        except Exception:
             self.fail('Bootable image with GPT must not raise')
 
     def test_bootable_iso_with_mbr_and_gpt_does_not_raise(self):
@@ -133,7 +133,7 @@ class TestCheckImageSanity(PungiTestCase):
 
         try:
             test_phase.check_image_sanity(compose)
-        except:
+        except Exception:
             self.fail('Bootable image with MBR and GPT must not raise')
 
     def test_bootable_iso_with_el_torito_does_not_raise(self):
@@ -144,7 +144,7 @@ class TestCheckImageSanity(PungiTestCase):
 
         try:
             test_phase.check_image_sanity(compose)
-        except:
+        except Exception:
             self.fail('Bootable image with El Torito must not raise')
 
     def test_checks_with_optional_variant(self):
@@ -163,9 +163,70 @@ class TestCheckImageSanity(PungiTestCase):
 
         try:
             test_phase.check_image_sanity(compose)
-        except:
+        except Exception:
             self.fail('Checking optional variant must not raise')
 
 
+class TestRepoclosure(PungiTestCase):
+
+    def _get_repo(self, variant, arch, path=None):
+        path = path or arch + '/os'
+        return {
+            'repoclosure-%s.%s' % (variant, arch): self.topdir + '/compose/%s/%s' % (variant, path)
+        }
+
+    @mock.patch('pungi.wrappers.repoclosure.get_repoclosure_cmd')
+    @mock.patch('pungi.phases.test.run')
+    def test_repoclosure_default_backend(self, mock_run, mock_grc):
+        compose = DummyCompose(self.topdir, {})
+        test_phase.run_repoclosure(compose)
+        self.maxDiff = None
+        all_repos = {}
+        for variant in compose.variants.itervalues():
+            for arch in variant.arches:
+                all_repos.update(self._get_repo(variant.uid, arch))
+            all_repos.update(self._get_repo(variant.uid, 'src', 'source/tree'))
+
+        self.assertItemsEqual(
+            mock_grc.call_args_list,
+            [mock.call(backend='yum', arch=['amd64', 'x86_64', 'noarch'], lookaside={},
+                       repos=self._get_repo('Everything', 'amd64')),
+             mock.call(backend='yum', arch=['amd64', 'x86_64', 'noarch'], lookaside={},
+                       repos=self._get_repo('Client', 'amd64')),
+             mock.call(backend='yum', arch=['amd64', 'x86_64', 'noarch'], lookaside={},
+                       repos=self._get_repo('Server', 'amd64')),
+             mock.call(backend='yum', arch=['x86_64', 'noarch'], lookaside={},
+                       repos=self._get_repo('Server', 'x86_64')),
+             mock.call(backend='yum', arch=['x86_64', 'noarch'], lookaside={},
+                       repos=self._get_repo('Everything', 'x86_64')),
+             mock.call(backend='yum', arch={'x86_64', 'amd64', 'noarch'}, builddeps=True, repos=all_repos)])
+
+    @mock.patch('pungi.wrappers.repoclosure.get_repoclosure_cmd')
+    @mock.patch('pungi.phases.test.run')
+    def test_repoclosure_dnf_backend(self, mock_run, mock_grc):
+        compose = DummyCompose(self.topdir, {'repoclosure_backend': 'dnf'})
+        test_phase.run_repoclosure(compose)
+        self.maxDiff = None
+        all_repos = {}
+        for variant in compose.variants.itervalues():
+            for arch in variant.arches:
+                all_repos.update(self._get_repo(variant.uid, arch))
+            all_repos.update(self._get_repo(variant.uid, 'src', 'source/tree'))
+
+        self.assertItemsEqual(
+            mock_grc.call_args_list,
+            [mock.call(backend='dnf', arch=['amd64', 'x86_64', 'noarch'], lookaside={},
+                       repos=self._get_repo('Everything', 'amd64')),
+             mock.call(backend='dnf', arch=['amd64', 'x86_64', 'noarch'], lookaside={},
+                       repos=self._get_repo('Client', 'amd64')),
+             mock.call(backend='dnf', arch=['amd64', 'x86_64', 'noarch'], lookaside={},
+                       repos=self._get_repo('Server', 'amd64')),
+             mock.call(backend='dnf', arch=['x86_64', 'noarch'], lookaside={},
+                       repos=self._get_repo('Server', 'x86_64')),
+             mock.call(backend='dnf', arch=['x86_64', 'noarch'], lookaside={},
+                       repos=self._get_repo('Everything', 'x86_64')),
+             mock.call(backend='dnf', arch={'x86_64', 'amd64', 'noarch'}, builddeps=True, repos=all_repos)])
+
+
 if __name__ == "__main__":
     unittest.main()