From f54c016ef9a4db16471b9ec8485eb4ab2086776f Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 29 Jul 2021 15:15:17 +0200 Subject: [PATCH] [FIX] runbot_merge: link warning on PRs of different projects If two PRs have the same label *in different projects entirely*, the mergebot should not consider them to be linked, but it did as shown by the warning message on odoo-dev/odoo#905 (two PRs created from the same branch in different projects were seen as linked by the status checker). 3b417b16a1f3af7f705cc66e602432a1f7bb2e68 fixed the actual staging selection, it's only the warning which did not properly segregate PRs. Only group PRs which target the same branch (therefore are within the same project). Fixes #487 --- runbot_merge/models/pull_requests.py | 3 +- runbot_merge/tests/test_multirepo.py | 97 +++++++++++++++++++++++++++- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index 885b605c..28eae653 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -1254,6 +1254,7 @@ class PullRequests(models.Model): pr.state != 'merged' AND pr.state != 'closed' GROUP BY + pr.target, CASE WHEN pr.label SIMILAR TO '%%:patch-[[:digit:]]+' THEN pr.id::text @@ -1264,7 +1265,7 @@ class PullRequests(models.Model): bool_or(pr.state = 'ready' AND NOT pr.link_warned) -- one of the others should be unready AND bool_or(pr.state != 'ready') - -- but ignore batches with one of the prs at p- + -- but ignore batches with one of the prs at p0 AND bool_and(pr.priority != 0) """) for [ids] in self.env.cr.fetchall(): diff --git a/runbot_merge/tests/test_multirepo.py b/runbot_merge/tests/test_multirepo.py index 182621e3..5d4f64a0 100644 --- a/runbot_merge/tests/test_multirepo.py +++ b/runbot_merge/tests/test_multirepo.py @@ -12,7 +12,7 @@ import pytest import requests from lxml.etree import XPath, tostring -from utils import seen, re_matches, get_partner, pr_page, to_pr +from utils import seen, re_matches, get_partner, pr_page, to_pr, Commit @pytest.fixture @@ -972,3 +972,98 @@ class TestSubstitutions: assert prb_id.staging_id, "PR B should be staged" assert pra_id.staging_id == prb_id.staging_id, "both prs should be staged together" assert pra_id.batch_id == prb_id.batch_id, "both prs should be part of the same batch" + +def test_multi_project(env, make_repo, setreviewers, users, config, + tunnel): + """ There should be no linking of PRs across projects, even if there is some + structural overlap between the two. + + Here we have two projects on different forks, then a user creates a PR from + a third fork (or one of the forks should not matter) to *both*. + + The two PRs should be independent. + """ + Projects = env['runbot_merge.project'] + gh_token = config['github']['token'] + + r1 = make_repo("repo_a") + with r1: + r1.make_commits( + None, Commit('root', tree={'a': 'a'}), + ref='heads/default') + r1_dev = r1.fork() + p1 = Projects.create({ + 'name': 'Project 1', + 'github_token': gh_token, + 'github_prefix': 'hansen', + 'repo_ids': [(0, 0, { + 'name': r1.name, + 'group_id': False, + 'required_statuses': 'a', + })], + 'branch_ids': [(0, 0, {'name': 'default'})], + }) + setreviewers(*p1.repo_ids) + + r2 = make_repo('repo_b') + with r2: + r2.make_commits( + None, Commit('root', tree={'b': 'a'}), + ref='heads/default' + ) + r2_dev = r2.fork() + p2 = Projects.create({ + 'name': "Project 2", + 'github_token': gh_token, + 'github_prefix': 'hansen', + 'repo_ids': [(0, 0, { + 'name': r2.name, + 'group_id': False, + 'required_statuses': 'a', + })], + 'branch_ids': [(0, 0, {'name': 'default'})], + }) + setreviewers(*p2.repo_ids) + + assert r1_dev.owner == r2_dev.owner + + with r1, r1_dev: + r1_dev.make_commits('default', Commit('new', tree={'a': 'b'}), ref='heads/other') + + # create, validate, and approve pr1 + pr1 = r1.make_pr(title='pr 1', target='default', head=r1_dev.owner + ':other') + r1.post_status(pr1.head, 'success', 'a') + pr1.post_comment('hansen r+', config['role_reviewer']['token']) + + with r2, r2_dev: + r2_dev.make_commits('default', Commit('new', tree={'b': 'b'}), ref='heads/other') + + # create second PR with the same label *in a different project*, don't + # approve it + pr2 = r2.make_pr(title='pr 2', target='default', head=r2_dev.owner + ':other') + r2.post_status(pr2.head, 'success', 'a') + env.run_crons() + + pr1_id = to_pr(env, pr1) + pr2_id = to_pr(env, pr2) + + print( + pr1.repo.name, pr1.number, pr1_id.display_name, pr1_id.label, + '\n', + pr2.repo.name, pr2.number, pr2_id.display_name, pr2_id.label, + flush=True, + ) + + assert pr1_id.state == 'ready' and not pr1_id.blocked + assert pr2_id.state == 'validated' + + assert pr1_id.staging_id + assert not pr2_id.staging_id + + assert pr1.comments == [ + (users['reviewer'], 'hansen r+'), + (users['user'], f'[Pull request status dashboard]({pr1_id.url}).'), + ] + assert pr2.comments == [ + (users['user'], f'[Pull request status dashboard]({pr2_id.url}).'), + ]