This is an automated email from the git hooks/post-receive script.
praiskup pushed a commit to branch master in repository copr/copr.
The following commit(s) were added to refs/heads/master by this push: new a9b4c53 frontend, backend: fixed project forking a9b4c53 is described below
commit a9b4c53ecb4d1b64d7352bcf4fdda3d033d3c38d Author: Tomas Hrnciar thrnciar@reedhat.com AuthorDate: Mon Aug 26 10:10:44 2019 +0200
frontend, backend: fixed project forking
This should fix broken project forking. In order to fix it, I had to change the format of datas sending from frontend to backend. Instead of lists containing names of old and new directories I am sending dictionaries, where keys are old directories and values are the names of new forked directories.
There is also unit test for this.
Fixes: #120, #652 Merges: #943 --- backend/backend/actions.py | 44 ++++++++------- backend/tests/test_action.py | 62 +++++++++++++++++++++- .../coprs_frontend/coprs/logic/complex_logic.py | 9 +++- frontend/coprs_frontend/tests/coprs_test_case.py | 47 ++++++++++++++++ .../tests/test_logic/test_complex_logic.py | 15 +++++- 5 files changed, 153 insertions(+), 24 deletions(-)
diff --git a/backend/backend/actions.py b/backend/backend/actions.py index f8ccca2..2b1b93d 100644 --- a/backend/backend/actions.py +++ b/backend/backend/actions.py @@ -132,32 +132,38 @@ class Action(object):
chroot_paths = set() for chroot, src_dst_dir in builds_map.items(): - src_dir, dst_dir = src_dst_dir[0], src_dst_dir[1] - if not chroot or not src_dir or not dst_dir: + + if not chroot or not src_dst_dir: continue
- old_chroot_path = os.path.join(old_path, chroot) - new_chroot_path = os.path.join(new_path, chroot) - chroot_paths.add(new_chroot_path) + for old_dir_name, new_dir_name in src_dst_dir.items(): + src_dir, dst_dir = old_dir_name, new_dir_name
- src_path = os.path.join(old_chroot_path, src_dir) - dst_path = os.path.join(new_chroot_path, dst_dir) + if not src_dir or not dst_dir: + continue
- if not os.path.exists(dst_path): - os.makedirs(dst_path) + old_chroot_path = os.path.join(old_path, chroot) + new_chroot_path = os.path.join(new_path, chroot) + chroot_paths.add(new_chroot_path)
- try: - copy_tree(src_path, dst_path) - except DistutilsFileError as e: - self.log.error(str(e)) - continue + src_path = os.path.join(old_chroot_path, src_dir) + dst_path = os.path.join(new_chroot_path, dst_dir) + + if not os.path.exists(dst_path): + os.makedirs(dst_path) + + try: + copy_tree(src_path, dst_path) + except DistutilsFileError as e: + self.log.error(str(e)) + continue
- # Drop old signatures coming from original repo and re-sign. - unsign_rpms_in_dir(dst_path, opts=self.opts, log=self.log) - if sign: - sign_rpms_in_dir(data["user"], data["copr"], dst_path, opts=self.opts, log=self.log) + # Drop old signatures coming from original repo and re-sign. + unsign_rpms_in_dir(dst_path, opts=self.opts, log=self.log) + if sign: + sign_rpms_in_dir(data["user"], data["copr"], dst_path, opts=self.opts, log=self.log)
- self.log.info("Forked build %s as %s", src_path, dst_path) + self.log.info("Forked build %s as %s", src_path, dst_path)
for chroot_path in chroot_paths: createrepo(path=chroot_path, front_url=self.front_url, diff --git a/backend/tests/test_action.py b/backend/tests/test_action.py index 9abc2df..907d608 100644 --- a/backend/tests/test_action.py +++ b/backend/tests/test_action.py @@ -16,6 +16,8 @@ from backend.actions import Action, ActionType, ActionResult from backend.exceptions import CreateRepoError, CoprKeygenRequestError from requests import RequestException
+import os + RESULTS_ROOT_URL = "http://example.com/results" STDOUT = "stdout" STDERR = "stderr" @@ -43,13 +45,14 @@ class TestAction(object): redis_db=9, redis_port=7777,
- destdir=None, + destdir="/var/lib/copr/public_html/results/", frontend_base_url=None, results_baseurl=RESULTS_ROOT_URL,
- do_sign=True, + do_sign=False,
build_deleting_without_createrepo="", + keygen_host="example.com" )
def teardown_method(self, method): @@ -98,6 +101,61 @@ class TestAction(object):
self.dummy = str(test_action)
+ @mock.patch("backend.actions.copy_tree") + @mock.patch("backend.actions.os.path.exists") + @mock.patch("backend.actions.unsign_rpms_in_dir") + @mock.patch("backend.actions.createrepo") + def test_action_handle_forks(self, mc_createrepo, mc_unsign_rpms_in_dir, mc_exists, mc_copy_tree, mc_time): + mc_time.time.return_value = self.test_time + mc_front_cb = MagicMock() + mc_exists = True + test_action = Action( + opts=self.opts, + action={ + "action_type": ActionType.FORK, + "id": 1, + "object_type": "copr", + "data": json.dumps( + { + 'builds_map': + { + 'srpm-builds': { + '00000002': '00000009', '00000005': '00000010'}, + 'fedora-17-x86_64': { + '00000002-pkg1': '00000009-pkg1', '00000005-pkg2': '00000010-pkg2'}, + 'fedora-17-i386': { + '00000002-pkg1': '00000009-pkg1', '00000005-pkg2': '00000010-pkg2'} + }, + "user": "thrnciar", + "copr": "source-copr" + }), + "old_value": "thrnciar/source-copr", + "new_value": "thrnciar/destination-copr", + }, + frontend_client=mc_front_cb, + ) + test_action.run() + calls = mc_copy_tree.call_args_list + assert len(calls) == 6 + assert calls[0][0] == ( + "/var/lib/copr/public_html/results/thrnciar/source-copr/srpm-builds/00000002", + "/var/lib/copr/public_html/results/thrnciar/destination-copr/srpm-builds/00000009") + assert calls[1][0] == ( + "/var/lib/copr/public_html/results/thrnciar/source-copr/srpm-builds/00000005", + "/var/lib/copr/public_html/results/thrnciar/destination-copr/srpm-builds/00000010") + assert calls[2][0] == ( + "/var/lib/copr/public_html/results/thrnciar/source-copr/fedora-17-x86_64/00000002-pkg1", + "/var/lib/copr/public_html/results/thrnciar/destination-copr/fedora-17-x86_64/00000009-pkg1") + assert calls[3][0] == ( + "/var/lib/copr/public_html/results/thrnciar/source-copr/fedora-17-x86_64/00000005-pkg2", + "/var/lib/copr/public_html/results/thrnciar/destination-copr/fedora-17-x86_64/00000010-pkg2") + assert calls[4][0] == ( + "/var/lib/copr/public_html/results/thrnciar/source-copr/fedora-17-i386/00000002-pkg1", + "/var/lib/copr/public_html/results/thrnciar/destination-copr/fedora-17-i386/00000009-pkg1") + assert calls[5][0] == ( + "/var/lib/copr/public_html/results/thrnciar/source-copr/fedora-17-i386/00000005-pkg2", + "/var/lib/copr/public_html/results/thrnciar/destination-copr/fedora-17-i386/00000010-pkg2") + @unittest.skip("Fixme, test doesn't work.") def test_action_run_rename(self, mc_time):
diff --git a/frontend/coprs_frontend/coprs/logic/complex_logic.py b/frontend/coprs_frontend/coprs/logic/complex_logic.py index ee45021..a5890cb 100644 --- a/frontend/coprs_frontend/coprs/logic/complex_logic.py +++ b/frontend/coprs_frontend/coprs/logic/complex_logic.py @@ -87,6 +87,7 @@ class ComplexLogic(object): raise exceptions.DuplicateException("Source project should not be same as destination")
builds_map = {} + srpm_builds_src,srpm_builds_dst, chroots_src, chroots_dst = ([] for i in range(4)) for package in copr.main_dir.packages: fpackage = forking.fork_package(package, fcopr) build = package.last_build(successful=True) @@ -96,11 +97,15 @@ class ComplexLogic(object): fbuild = forking.fork_build(build, fcopr, fpackage)
if build.result_dir: - builds_map['srpm-builds'] = (build.result_dir, fbuild.result_dir) + srpm_builds_src.append(build.result_dir) + srpm_builds_dst.append(fbuild.result_dir)
for chroot, fchroot in zip(build.build_chroots, fbuild.build_chroots): if chroot.result_dir: - builds_map[chroot.name] = (chroot.result_dir, fchroot.result_dir) + chroots_src.append(chroot.result_dir) + chroots_dst.append(fchroot.result_dir) + builds_map['srpm-builds'] = dict(zip(srpm_builds_src, srpm_builds_dst)) + builds_map[chroot.name] = dict(zip(chroots_src, chroots_dst))
db.session.commit() ActionsLogic.send_fork_copr(copr, fcopr, builds_map) diff --git a/frontend/coprs_frontend/tests/coprs_test_case.py b/frontend/coprs_frontend/tests/coprs_test_case.py index b1425da..9c10214 100644 --- a/frontend/coprs_frontend/tests/coprs_test_case.py +++ b/frontend/coprs_frontend/tests/coprs_test_case.py @@ -282,6 +282,53 @@ class CoprsTestCase(object): self.db.session.add_all([self.b1, self.b2, self.b3, self.b4])
@pytest.fixture + def f_fork_prepare(self, f_coprs, f_mock_chroots, f_builds): + + self.p4 = models.Package( + copr=self.c2, copr_dir=self.c2_dir, name="hello-world", source_type=0) + self.b5 = models.Build( + copr=self.c2, copr_dir=self.c2_dir, package=self.p4, + user=self.u1, submitted_on=50, srpm_url="http://somesrpm", + source_status=StatusEnum("succeeded"), result_dir='00000005-hello-world') + self.b6 = models.Build( + copr=self.c2, copr_dir=self.c2_dir, package=self.p4, + user=self.u1, submitted_on=10, srpm_url="http://somesrpm", + source_status=StatusEnum("failed"), result_dir='00000006-hello-world', + source_json='{}') + self.b7 = models.Build( + copr=self.c2, copr_dir=self.c2_dir, package=self.p2, + user=self.u1, submitted_on=10, srpm_url="http://somesrpm", + source_status=StatusEnum("succeeded"), result_dir='00000007-whatsupthere-world') + self.b8 = models.Build( + copr=self.c2, copr_dir=self.c2_dir, package=self.p2, + user=self.u1, submitted_on=100, srpm_url="http://somesrpm", + source_status=StatusEnum("succeeded"), result_dir='00000008-whatsupthere-world') + + self.basic_builds = [self.b5, self.b6, self.b7, self.b8] + self.b5_bc = [] + self.b6_bc = [] + self.b7_bc = [] + self.b8_bc = [] + self.db.session.flush() + + for build, build_chroots in zip( + [self.b5, self.b6, self.b7, self.b8], + [self.b5_bc, self.b6_bc, self.b7_bc, self.b8_bc]): + + status = StatusEnum("succeeded") + for chroot in build.copr.active_chroots: + buildchroot = models.BuildChroot( + build=build, + mock_chroot=chroot, + status=status, + git_hash="12345", + result_dir="{}-{}".format(build.id, build.package.name), + ) + build_chroots.append(buildchroot) + self.db.session.add(buildchroot) + self.db.session.add_all([self.b5, self.b6, self.b7, self.b8]) + + @pytest.fixture def f_hook_package(self, f_users, f_coprs, f_mock_chroots, f_builds): self.c1.webhook_secret = str(uuid.uuid4()) self.db.session.add(self.c1) diff --git a/frontend/coprs_frontend/tests/test_logic/test_complex_logic.py b/frontend/coprs_frontend/tests/test_logic/test_complex_logic.py index 7b4a7dd..e5f0a7b 100644 --- a/frontend/coprs_frontend/tests/test_logic/test_complex_logic.py +++ b/frontend/coprs_frontend/tests/test_logic/test_complex_logic.py @@ -23,8 +23,21 @@ class TestComplexLogic(CoprsTestCase): data = json.loads(actions[0].data) assert data["user"] == self.u2.name assert data["copr"] == "dstname" - assert data["builds_map"] == {'fedora-18-x86_64': ['bar', '00000005-hello-world'], 'srpm-builds': ['bar', '00000005']} + assert data["builds_map"] == {'srpm-builds': {'bar': '00000005'},'fedora-18-x86_64': {'bar': '00000005-hello-world'}}
+ @mock.patch("flask.g") + def test_fork_copr_projects_with_more_builds(self, mc_flask_g, f_users, f_fork_prepare, f_db): + mc_flask_g.user.name = self.u2.name + fc2, created = ComplexLogic.fork_copr(self.c2, self.u2, u"dstname") + self.db.session.commit() + actions = ActionsLogic.get_many(ActionTypeEnum("fork")).all() + assert len(actions) == 1 + data = json.loads(actions[0].data) + assert data["user"] == self.u2.name + assert data["copr"] == "dstname" + assert data["builds_map"] == {'srpm-builds': {'00000008-whatsupthere-world': '00000009', '00000005-hello-world': '00000010'}, + 'fedora-17-x86_64': {'8-whatsupthere-world': '00000009-whatsupthere-world', '5-hello-world': '00000010-hello-world'}, + 'fedora-17-i386': {'8-whatsupthere-world': '00000009-whatsupthere-world', '5-hello-world': '00000010-hello-world'}}
def test_delete_expired_coprs(self, f_users, f_mock_chroots, f_coprs, f_builds, f_db): query = self.db.session.query(models.Copr)
copr-commits@lists.fedorahosted.org