From a0063f9df091ef901cfb055b002e9cf4107688bc Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Mon, 10 Sep 2018 17:18:25 +0200 Subject: [PATCH] [IMP] runbot_merge: provide more useful message on some staging failures If CI fails on a non-PR'd branch of a staging (e.g. given repos A and B and a PR to A, CI fails on the staging branch to B), the error message (log and comment on the PR) is unhelpful as it states that the staging failed for "unknown reason". Improve this by providing the failed CI context and the commit, which should allow finding out the branch & CI logs, and understanding the why of the failure. Fixes #36 --- runbot_merge/models/pull_requests.py | 4 +++- runbot_merge/tests/test_multirepo.py | 33 ++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index 827dc031..d2990d1b 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -871,7 +871,9 @@ class Stagings(models.Model): ), None) if pr: self.fail(reason, pr) - return False + else: + self.fail('%s failed on %s' % (reason, head)) + return False # the staging failed but we don't have a specific culprit, fail # everything diff --git a/runbot_merge/tests/test_multirepo.py b/runbot_merge/tests/test_multirepo.py index b21b1325..cfbd541e 100644 --- a/runbot_merge/tests/test_multirepo.py +++ b/runbot_merge/tests/test_multirepo.py @@ -266,6 +266,39 @@ def test_one_failed(env, project, repo_a, repo_b, owner): assert not pr_a.staging_id, \ "pr_a should not have been staged as companion is not ready" +def test_other_failed(env, project, repo_a, repo_b, owner, users): + """ In a non-matched-branch scenario, if the companion staging (copy of + targets) fails when built with the PR, it should provide a non-useless + message + """ + c_a = repo_a.make_commit(None, 'initial', None, tree={'a': 'a_0'}) + repo_a.make_ref('heads/master', c_a) + # pr_a is born ready + pr_a = make_pr(repo_a, 'A', [{'a': 'a_1'}], label='do-a-thing') + repo_a.post_status(pr_a.head, 'success', 'ci/runbot') + repo_a.post_status(pr_a.head, 'success', 'legal/cla') + + c_b = repo_b.make_commit(None, 'initial', None, tree={'a': 'b_0'}) + repo_b.make_ref('heads/master', c_b) + + env['runbot_merge.project']._check_progress() + pr = to_pr(env, pr_a) + assert pr.staging_id + + repo_a.post_status('heads/staging.master', 'success', 'legal/cla') + repo_a.post_status('heads/staging.master', 'success', 'ci/runbot') + repo_b.post_status('heads/staging.master', 'success', 'legal/cla') + repo_b.post_status('heads/staging.master', 'failure', 'ci/runbot') + env['runbot_merge.project']._check_progress() + + sth = repo_b.commit('heads/staging.master').id + assert not pr.staging_id + assert pr.state == 'error' + assert pr_a.comments == [ + (users['reviewer'], 'hansen r+'), + (users['user'], 'Staging failed: ci/runbot failed on %s' % sth) + ] + def test_batching(env, project, repo_a, repo_b): """ If multiple batches (label groups) are ready they should get batched together (within the limits of teh project's batch limit)