Merge #1662 Improve test coverage for MockModuleBuilder

module_build_service/builder/MockModuleBuilder.py

@@ -413,8 +413,8 @@ class MockModuleBuilder(GenericBuilder):
         # ...and inject modules.yaml there if asked.
         if include_module_yaml:
             mmd_path = os.path.join(path, "modules.yaml")
-            with open(mmd_path, "w") as f:
-                f.write(mmd_to_str(m1_mmd))
+            with open(mmd_path, "wb") as f:
+                f.write(mmd_to_str(m1_mmd).encode("utf-8"))
             execute_cmd(["/usr/bin/modifyrepo_c", "--mdtype=modules", mmd_path, repodata_path])
 
     def _add_repo(self, name, baseurl, extra=""):
@@ -536,7 +536,7 @@ class MockModuleBuilder(GenericBuilder):
             # If source starts with mock_resultdir, it means it is path to local
             # module build repository.
             if source.startswith(conf.mock_resultsdir):
-                repo_name = os.path.basename(source)
+                repo_name = os.path.basename(os.path.dirname(source))
                 if repo_name.startswith("module-"):
                     repo_name = repo_name[7:]
                 repo_dir = source
@@ -618,13 +618,6 @@ class MockModuleBuilder(GenericBuilder):
             if os.path.isfile(log_path) and os.path.getsize(log_path) == 0:
                 os.remove(log_path)
 
-            # Remove other files containing useless information
-            elif logf.endswith("-srpm-stdout.log"):
-                with open(log_path) as f:
-                    data = f.read(4096)
-                    if re.match("Downloading [^\n]*\n\n\nWrote: [^\n]", data):
-                        os.remove(log_path)
-
     def build_srpm(self, artifact_name, source, build_id, builder):
         """
         Builds the artifact from the SRPM.
@@ -779,6 +772,26 @@ class MockModuleBuilder(GenericBuilder):
             nvrs = set(kobo.rpmlib.make_nvr(rpm, force_epoch=True) for rpm in rpms)
             return list(nvrs)
 
+    def get_average_build_time(self, component):
+        """
+        Get the average build time of the component from Koji
+        :param component: a ComponentBuild object
+        :return: a float of the average build time in seconds
+        """
+        # We currently don't track build times in MBS directly, so we can use Koji to get a decent
+        # estimate
+        if not self.koji_session:
+            # If Koji is not configured on the system, then just return 0.0 for components
+            try:
+                self.koji_session = get_session(self.config, login=False)
+                # If the component has not been built before, then None is returned. Instead,
+                # let's return 0.0 so the type is consistent
+                return self.koji_session.getAverageBuildDuration(component.package) or 0.0
+            except Exception:
+                log.debug(
+                    "The Koji call to getAverageBuildDuration failed. Is Koji properly configured?")
+                return 0.0
+
 
 class BaseBuilder(object):
     def __init__(self, config, resultsdir):
@@ -836,33 +849,8 @@ class SCMBuilder(BaseBuilder):
                 src_dir = repo_path[len("file://"):]
                 f.write("config_opts['scm_opts']['ext_src_dir'] = '{}'\n".format(src_dir))
 
-    def _make_executable(self, path):
-        mode = os.stat(path).st_mode
-        mode |= (mode & 0o444) >> 2  # copy R bits to X
-        os.chmod(path, mode)
-
     def _get_distgit_commands(self, source):
         for host, cmds in conf.distgits.items():
             if source.startswith(host):
                 return cmds
         raise KeyError("No defined commands for {}".format(source))
-
-    def get_average_build_time(self, component):
-        """
-        Get the average build time of the component from Koji
-        :param component: a ComponentBuild object
-        :return: a float of the average build time in seconds
-        """
-        # We currently don't track build times in MBS directly, so we can use Koji to get a decent
-        # estimate
-        if not self.koji_session:
-            # If Koji is not configured on the system, then just return 0.0 for components
-            try:
-                self.koji_session = get_session(self.config, login=False)
-                # If the component has not been built before, then None is returned. Instead,
-                # let's return 0.0 so the type is consistent
-                return self.koji_session.getAverageBuildDuration(component.package) or 0.0
-            except Exception:
-                log.debug(
-                    "The Koji call to getAverageBuildDuration failed. Is Koji properly configured?")
-                return 0.0

tests/test_builder/test_mock.py

@@ -12,6 +12,7 @@ import mock
 import pytest
 
 from module_build_service.common.config import conf
+from module_build_service.builder.base import GenericBuilder
 from module_build_service.builder.MockModuleBuilder import (
     import_fake_base_module,
     import_builds_from_local_dnf_repos,
@@ -20,13 +21,28 @@ from module_build_service.builder.MockModuleBuilder import (
 )
 from module_build_service.common import models
 from module_build_service.common.models import ModuleBuild, ComponentBuild
-from module_build_service.common.utils import load_mmd, mmd_to_str
+from module_build_service.common.utils import import_mmd, load_mmd, mmd_to_str
 from module_build_service.scheduler import events
 from module_build_service.scheduler.db_session import db_session
 from tests import make_module_in_db, read_staged_data, staged_data_filename
 
 
-@pytest.mark.usefixtures("require_empty_database")
+class TestConfig(object):
+    def __init__(self, resultsdir):
+        self.resultsdir = resultsdir
+
+
+@pytest.fixture
+def testconfig():
+    resultsdir = tempfile.mkdtemp()
+
+    with mock.patch.multiple("module_build_service.common.conf",
+                             mock_resultsdir=resultsdir,
+                             system="mock"):
+        yield TestConfig(resultsdir)
+
+
+@pytest.mark.usefixtures("require_empty_database", "testconfig")
 class TestMockModuleBuilder:
     def setup_method(self, test_method):
         self.resultdir = tempfile.mkdtemp()
@@ -121,7 +137,6 @@ class TestMockModuleBuilder:
 
         return module
 
-    @mock.patch("module_build_service.common.conf.system", new="mock")
     def test_createrepo_filter_last_batch(self):
         module = self._create_module_with_filters(db_session, 3, koji.BUILD_STATES["COMPLETE"])
 
@@ -148,7 +163,6 @@ class TestMockModuleBuilder:
             rpm_names = [kobo.rpmlib.parse_nvr(rpm)["name"] for rpm in pkglist.split("\n")]
             assert "ed" not in rpm_names
 
-    @mock.patch("module_build_service.common.conf.system", new="mock")
     def test_createrepo_not_last_batch(self):
         module = self._create_module_with_filters(db_session, 2, koji.BUILD_STATES["COMPLETE"])
 
@@ -173,7 +187,6 @@ class TestMockModuleBuilder:
             rpm_names = [kobo.rpmlib.parse_nvr(rpm)["name"] for rpm in pkglist.split("\n")]
             assert "ed" in rpm_names
 
-    @mock.patch("module_build_service.common.conf.system", new="mock")
     def test_createrepo_empty_rmp_list(self):
         module = self._create_module_with_filters(db_session, 3, koji.BUILD_STATES["COMPLETE"])
 
@@ -189,26 +202,160 @@ class TestMockModuleBuilder:
             assert not pkglist
 
 
-@pytest.mark.usefixtures("require_empty_database")
-class TestMockModuleBuilderAddRepos:
+@pytest.mark.usefixtures("require_platform_and_default_arch", "testconfig")
+class TestMockModuleBuilderBuild:
 
-    @mock.patch("module_build_service.common.conf.system", new="mock")
-    @mock.patch(
-        "module_build_service.common.config.Config.base_module_repofiles",
-        new_callable=mock.PropertyMock,
-        return_value=["/etc/yum.repos.d/bar.repo", "/etc/yum.repos.d/bar-updates.repo"],
-        create=True,
-    )
-    @mock.patch("module_build_service.builder.MockModuleBuilder.open", create=True)
-    @mock.patch(
-        "module_build_service.builder.MockModuleBuilder.MockModuleBuilder._load_mock_config"
-    )
-    @mock.patch(
-        "module_build_service.builder.MockModuleBuilder.MockModuleBuilder._write_mock_config"
-    )
-    def test_buildroot_add_repos(
-        self, write_config, load_config, patched_open, base_module_repofiles
-    ):
+    @pytest.fixture(params=[{}])
+    def mock_external_commands(self, request):
+        def mock_popen(cmd, *args, **kwargs):
+            stdout = kwargs.get("stdout", None)
+
+            result = mock.MagicMock(name="Popen")
+            result.__enter__.return_value = result
+            result.communicate.return_value = (None, None)
+            result.poll.return_value = 0
+            result.returncode = 0
+
+            command = os.path.basename(cmd[0])
+            if command == "mock":
+                if "--init" in cmd:
+                    return result
+
+                result.returncode = 1 if "fail_build" in request.param else 0
+
+                resultdir = None
+                for arg in cmd:
+                    if arg.startswith("--resultdir="):
+                        resultdir = arg[len("--resultdir="):]
+                assert resultdir
+
+                result_files = [
+                    "perl-Tangerine-0.22-2.module_1589+0def5cd9.src.rpm",
+                    "perl-Tangerine-0.22-2.module_1589+0def5cd9.noarch.rpm"
+                ]
+
+                if stdout:
+                    stdout.write("Successfully built perl-Tangerine")
+                    stdout.flush()
+
+                for f in result_files:
+                    with open(os.path.join(resultdir, f), "w") as f:
+                        f.write("-")
+
+                return result
+            elif command == "createrepo_c":
+                # Expecting /usr/bin/createrepo_c --pkglist <pkglist> <path>
+                assert len(cmd) == 4
+                assert cmd[0:2] == ["/usr/bin/createrepo_c", "--pkglist"]
+                path = cmd[3]
+
+                repodata_path = os.path.join(path, "repodata")
+                if not os.path.exists(repodata_path):
+                    os.mkdir(repodata_path)
+                with open(os.path.join(repodata_path, "repomd.xml"), "w") as f:
+                    f.write("-")
+
+                return result
+            elif command == "modifyrepo_c":
+                return result
+            elif command == "rpm":
+                assert cmd[0:4] == [
+                    "rpm",
+                    "--queryformat",
+                    "%{NAME} %{EPOCHNUM} %{VERSION} %{RELEASE} %{ARCH}\n",
+                    "-qp"
+                ]
+                packages = cmd[4:]
+                ret = ""
+                for p in packages:
+                    nvr, arch, ext = p.rsplit(".", 2)
+                    n, v, r = nvr.rsplit("-", 2)
+                    ret += "{} 0 {} {} {}\n".format(n, v, r, arch)
+
+                result.communicate.return_value = (ret, None)
+
+                return result
+            else:
+                raise RuntimeError("Unsupported cmd {}".format(cmd))
+
+        with mock.patch("subprocess.Popen") as popen_mock:
+            popen_mock.side_effect = mock_popen
+            yield
+
+    @pytest.fixture
+    def builder(self, mock_external_commands, require_platform_and_default_arch):
+        mmd = load_mmd(read_staged_data("formatted_testmodule"))
+        xmd = mmd.get_xmd()
+        xmd["mbs"]["koji_tag"] = "module-testmodule-master-20180205135154-9c690d0e"
+        mmd.set_xmd(xmd)
+
+        import_mmd(db_session, mmd)
+
+        build = ModuleBuild.get_last_build_in_stream(db_session, "testmodule", "master")
+        build.batch = 2
+
+        comp_builds = [
+            {
+                "module_id": build.id,
+                "state": koji.BUILD_STATES["BUILDING"],
+                "package": "perl-Tangerine",
+                "format": "rpms",
+                "scmurl": (
+                    "https://src.fedoraproject.org/rpms/perl-Tangerine?#master"
+                ),
+                "batch": 2,
+                "ref": "01234567812fea798671925cc537f2f29b0bd216",
+            },
+        ]
+
+        for build in comp_builds:
+            db_session.add(ComponentBuild(**build))
+        db_session.commit()
+
+        # This is used to identify the first build of a module
+        MockModuleBuilder._build_id = 1
+
+        build = ModuleBuild.get_last_build_in_stream(db_session, "testmodule", "master")
+        yield GenericBuilder.create_from_module(db_session, build, conf, buildroot_connect=True)
+
+    def test_mock_module_builder_build(self, builder):
+        builder.buildroot_add_artifacts([
+            "module-build-macros-0.1-1.module+f33+2+cea92c88.noarch.rpm"
+        ])
+
+        builder.build("perl-Tangerine", "https://src.fedoraproject.org/rpms/perl-Tangerine?#master")
+
+        # Try building again to exercise code to clean up last build for this thread
+        builder.build("perl-Tangerine", "https://src.fedoraproject.org/rpms/perl-Tangerine?#master")
+
+        builder.finalize(succeeded=True)
+
+    @pytest.mark.parametrize("mock_external_commands", [{"fail_build": True}], indirect=True)
+    def test_mock_module_builder_build_failure(self, builder, caplog):
+        builder.build("perl-Tangerine", "file://opt/sources/fedora/perl-Tangerine?#master")
+        assert "Error while building artifact perl-Tangerine" in caplog.text
+
+    def test_mock_module_builder_build_local_repo(self, builder):
+        builder.build("perl-Tangerine", "file://opt/sources/fedora/perl-Tangerine?#master")
+
+    def test_mock_module_builder_build_srpm(self, builder):
+        builder.build("perl-Tangerine", "/opt/sources/perl-Tangerine-0.22-2.src.rpm")
+
+    def test_mock_module_builder_build_stale_builddir(self, builder):
+        builder.build("perl-Tangerine", "/opt/sources/perl-Tangerine-0.22-2.src.rpm")
+        builder.finalize(succeeded=True)
+
+        MockModuleBuilder._build_id = 1
+        builder2 = GenericBuilder.create_from_module(
+            db_session, builder.module, conf, buildroot_connect=True
+        )
+        builder2.build("perl-Tangerine", "/opt/sources/perl-Tangerine-0.22-2.src.rpm")
+
+
+@pytest.mark.usefixtures("require_empty_database", "testconfig")
+class TestMockModuleBuilderAddRepos:
+    @pytest.fixture
+    def testsetup(self):
         import_fake_base_module("platform:f29:1:000000")
 
         platform = ModuleBuild.get_last_build_in_stream(db_session, "platform", "f29")
@@ -219,14 +366,37 @@ class TestMockModuleBuilderAddRepos:
         foo = make_module_in_db("foo:1:1:1", module_deps)
         app = make_module_in_db("app:1:1:1", module_deps)
 
+        builder = MockModuleBuilder(db_session, "user", app, conf, "module-app", [])
+
+        with mock.patch.multiple("module_build_service.builder.MockModuleBuilder.MockModuleBuilder",
+                                 _load_mock_config=mock.MagicMock(),
+                                 _write_mock_config=mock.MagicMock()):
+            yield {
+                "builder": builder,
+                "platform": platform,
+                "app": app,
+                "foo": foo,
+            }
+
+    @mock.patch(
+        "module_build_service.common.config.Config.base_module_repofiles",
+        new_callable=mock.PropertyMock,
+        return_value=["/etc/yum.repos.d/bar.repo", "/etc/yum.repos.d/bar-updates.repo"],
+        create=True,
+    )
+    @mock.patch("module_build_service.builder.MockModuleBuilder.open", create=True)
+    def test_buildroot_add_repos_repofile(self, patched_open, base_module_repofiles, testsetup):
+        builder = testsetup["builder"]
+        platform = testsetup["platform"]
+        foo = testsetup["foo"]
+        app = testsetup["app"]
+
         patched_open.side_effect = [
             mock.mock_open(read_data="[fake]\nrepofile 1\n").return_value,
             mock.mock_open(read_data="[fake]\nrepofile 2\n").return_value,
             mock.mock_open(read_data="[fake]\nrepofile 3\n").return_value,
         ]
 
-        builder = MockModuleBuilder(db_session, "user", app, conf, "module-app", [])
-
         dependencies = {
             "repofile://": [platform.mmd()],
             "repofile:///etc/yum.repos.d/foo.repo": [foo.mmd(), app.mmd()],
@@ -240,6 +410,83 @@ class TestMockModuleBuilderAddRepos:
 
         assert set(builder.enabled_modules) == {"foo:1", "app:1"}
 
+    def test_buildroot_add_repos_local(self, testsetup):
+        builder = testsetup["builder"]
+        foo = testsetup["foo"]
+
+        dependencies = {
+            os.path.join(conf.mock_resultsdir, "module-foo/results"): [foo.mmd()]
+        }
+
+        builder.buildroot_add_repos(dependencies)
+
+        assert "[foo]" in builder.yum_conf
+
+        assert set(builder.enabled_modules) == set()
+
+    @mock.patch("koji.ClientSession")
+    def test_buildroot_add_repos_koji_tag_with_repo(self, client_session, testsetup):
+        builder = testsetup["builder"]
+        foo = testsetup["foo"]
+
+        client_session_mock = mock.MagicMock()
+        client_session.return_value = client_session_mock
+
+        client_session_mock.getRepo.return_value = {
+            "id": 2471511
+        }
+
+        dependencies = {
+            "module-foo": [foo.mmd()]
+        }
+
+        builder.buildroot_add_repos(dependencies)
+
+        assert "[module-foo]" in builder.yum_conf
+        assert ("baseurl=https://kojipkgs.stg.fedoraproject.org"
+                "//repos/module-foo/2471511/x86_64/" in builder.yum_conf)
+
+        assert set(builder.enabled_modules) == set()
+
+    @pytest.mark.parametrize("should_add_repo", (True, False))
+    @mock.patch("koji.ClientSession")
+    @mock.patch("module_build_service.builder.MockModuleBuilder.create_local_repo_from_koji_tag")
+    def test_buildroot_add_repos_koji_tag_create_local_repo(
+            self, create_local_repo, client_session, testsetup, should_add_repo
+    ):
+        builder = testsetup["builder"]
+        foo = testsetup["foo"]
+
+        import_fake_base_module("platform:f29:1:000000")
+
+        client_session_mock = mock.MagicMock()
+        client_session.return_value = client_session_mock
+
+        client_session_mock.getRepo.return_value = None
+        client_session_mock.getTagExternalRepos.return_value = [{
+            "external_repo_name": "extdep",
+            "url": "http://example.com/repos/extdep",
+        }]
+
+        create_local_repo.return_value = should_add_repo
+
+        dependencies = {
+            "module-foo": [foo.mmd()]
+        }
+
+        builder.buildroot_add_repos(dependencies)
+
+        if should_add_repo:
+            assert "[module-foo]" in builder.yum_conf
+            assert ("baseurl=file:///tmp/mbs/koji_tags/module-foo" in builder.yum_conf)
+        else:
+            assert "[module-foo]" not in builder.yum_conf
+
+        assert "[extdep]" in builder.yum_conf
+        assert ("baseurl=http://example.com/repos/extdep" in builder.yum_conf)
+
+        assert set(builder.enabled_modules) == set()
+
 
 @pytest.mark.usefixtures("require_empty_database_cls")
 class TestOfflineLocalBuilds: