From deaee51138945b591dc87ba72d9d20d776d661d9 Mon Sep 17 00:00:00 2001 From: charlesprost <7427667+charlesprost@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:34:44 +0000 Subject: [PATCH] feat(status): report all blockers in the /status command /status already existed (BERTE-539), but it named only the worst failing build, hid bypass notes and left out rows it couldn't check yet. It now lists every failing build, marks unchecked rows as pending, and reports the wait option and merge-queue state. Dispatch-Run: 01M3S829EH1E08T79VVGG1Z0EN Dispatch-Attempt: 1 --- bert_e/docs/USER_DOC.md | 24 +++++ bert_e/templates/status_report.md | 4 +- bert_e/tests/test_bert_e.py | 96 +++++++++++++++++++ bert_e/tests/unit/test_status_report.py | 88 +++++++++++++++++ bert_e/workflow/gitwaterflow/commands.py | 114 +++++++++++++++++------ 5 files changed, 295 insertions(+), 31 deletions(-) create mode 100644 bert_e/tests/unit/test_status_report.py diff --git a/bert_e/docs/USER_DOC.md b/bert_e/docs/USER_DOC.md index 664a6124..68b1a590 100644 --- a/bert_e/docs/USER_DOC.md +++ b/bert_e/docs/USER_DOC.md @@ -159,6 +159,7 @@ __Bert-E__: | command name | description | requires admin rights? | |:--------------------- |:------------------------ |:----------------------:| | help | Print __Bert-E__'s manual in the pull request | no +| status | Print everything still missing before this pull request can be merged (see below) | no | reset | Let __Bert-E__ reset the integration branches associated to the current pull request with a warning if the developer manually modified one of the the integration branches | no | force_reset | Let __Bert-E__ reset the integration branches associated to the current pull request **without warning**. | no @@ -170,6 +171,29 @@ option -- that way, comments addressed to other bots (e.g. that explicitly address __Bert-E__ via its ``@`` mention are always dispatched normally and will still report unknown commands. +### The status command + +``/status`` (or ``@bert-e status``) posts a report listing every check at +once, instead of only the next blocker. It is read-only: it never creates +integration branches, merges or queues anything, and any user can run it. A +new report is posted each time the command is issued. + +Each row is marked :sunny: (satisfied, possibly with a note such as +"bypassed"), :exclamation: (missing) or :hourglass: (not yet evaluated). The +report covers: + +* approvals (author, peer, leader, unanimity, requested changes); +* the ``wait`` option, when it is set; +* integration builds: every integration branch whose build is not + successful, with a link to its build when available. Before the integration + branches exist, this row shows as not yet evaluated; +* Jira fix versions, when Jira checks are configured; +* integration branch history (whether a reset may be needed); +* the merge queue, when queues are enabled. + +This command is unrelated to the web status page served by __Bert-E__, which +shows the merge queue and recent merges for the whole repository. + Integration branches... ----------------------- __*Bert-E* creates temporary branches during the merge process. These are diff --git a/bert_e/templates/status_report.md b/bert_e/templates/status_report.md index 201937d0..1824ca8f 100644 --- a/bert_e/templates/status_report.md +++ b/bert_e/templates/status_report.md @@ -3,9 +3,11 @@ check | status ---------|-------- {% for item in status -%} -:arrow_right: **{{status[item].display_name}}** | {% if status[item].pass %}:sunny:{% else %}:exclamation: {{ status[item].details | join(' — ') }}{% endif %} +:arrow_right: **{{status[item].display_name}}** | {% if status[item].pending %}:hourglass: {{ status[item].details | join(' — ') }}{% elif status[item].pass %}:sunny:{% if status[item].details %} {{ status[item].details | join(' — ') }}{% endif %}{% else %}:exclamation: {{ status[item].details | join(' — ') }}{% endif %} {% endfor %} +:sunny: satisfied — :exclamation: missing — :hourglass: not yet evaluated + {% else %} *Status report is not available.* diff --git a/bert_e/tests/test_bert_e.py b/bert_e/tests/test_bert_e.py index 8b6126c5..a8e3b2fe 100644 --- a/bert_e/tests/test_bert_e.py +++ b/bert_e/tests/test_bert_e.py @@ -3021,6 +3021,102 @@ def test_status_command_robot_commit_in_history(self): self.assertIn('history', report) self.assertTrue(getattr(report['history'], 'pass')) + def test_status_command_lists_every_failing_build(self): + """Status report lists every failing integration build, not only the + worst one.""" + pr = self.create_pr('bugfix/TEST-00001', 'development/4.3') + self.handle(pr.id, options=['bypass_jira_check']) + self.gitrepo.cmd('git fetch --prune') + wbranch_refs = self.gitrepo.cmd( + 'git branch -r --list origin/w/*/bugfix/TEST-00001' + ).strip().split() + self.assertGreaterEqual(len(wbranch_refs), 2) + ok_ref, failing_refs = wbranch_refs[0], wbranch_refs[1:] + sha = self.gitrepo.cmd('git rev-parse %s' % ok_ref).strip() + self.set_build_status(sha, 'SUCCESSFUL') + for ref in failing_refs: + sha = self.gitrepo.cmd('git rev-parse %s' % ref).strip() + self.set_build_status(sha, 'FAILED') + pr.add_comment('@%s status' % self.args.robot_username) + with self.assertRaises(exns.StatusReport) as ctx: + self.handle(pr.id, options=['bypass_jira_check'], backtrace=True) + builds = ctx.exception.kwargs['status']['builds'] + self.assertFalse(getattr(builds, 'pass')) + self.assertEqual(len(builds.details), len(failing_refs)) + for ref in failing_refs: + name = ref[len('origin/'):] + self.assertTrue(any(d.startswith(name + ': FAILED') + for d in builds.details)) + self.assertIn('FAILED', ctx.exception.msg) + + def test_status_command_builds_not_yet_evaluated(self): + """Status report marks builds as pending when integration branches + do not exist yet, and does not create them.""" + pr = self.create_pr('bugfix/my-feature', 'development/4.3') + # Stops at the Jira check, before integration branches are created. + self.handle(pr.id) + pr.add_comment('@%s status' % self.args.robot_username) + with self.assertRaises(exns.StatusReport) as ctx: + self.handle(pr.id, backtrace=True) + report = ctx.exception.kwargs['status'] + self.assertIn('builds', report) + self.assertTrue(report['builds'].pending) + self.assertFalse(getattr(report['builds'], 'pass')) + self.assertTrue( + any('not yet evaluated' in d for d in report['builds'].details)) + self.assertNotIn('history', report) + self.assertIn(':hourglass:', ctx.exception.msg) + self.gitrepo.cmd('git fetch --prune') + self.assertEqual(self.gitrepo.cmd( + 'git branch -r --list origin/w/*/bugfix/my-feature').strip(), '') + + def test_status_command_approvals_bypassed_details(self): + """Status report says approvals are bypassed, not just satisfied.""" + pr = self.create_pr('bugfix/TEST-00001', 'development/4.3') + self.handle(pr.id, options=['bypass_jira_check']) + pr.add_comment('@%s status' % self.args.robot_username) + with self.assertRaises(exns.StatusReport) as ctx: + self.handle(pr.id, options=['bypass_jira_check', + 'bypass_author_approval', + 'bypass_peer_approval', + 'bypass_leader_approval'], + backtrace=True) + approvals = ctx.exception.kwargs['status']['approvals'] + self.assertTrue(getattr(approvals, 'pass')) + self.assertTrue(any('bypassed' in d for d in approvals.details)) + self.assertIn('bypassed', ctx.exception.msg) + + def test_status_command_wait_option(self): + """Status report lists the wait option as a blocker.""" + pr = self.create_pr('bugfix/TEST-00001', 'development/4.3') + self.handle(pr.id, options=['bypass_jira_check']) + pr.add_comment('@%s status' % self.args.robot_username) + with self.assertRaises(exns.StatusReport) as ctx: + self.handle(pr.id, options=['bypass_jira_check', 'wait'], + backtrace=True) + report = ctx.exception.kwargs['status'] + self.assertIn('wait', report) + self.assertFalse(getattr(report['wait'], 'pass')) + + def test_status_command_no_wait_row_by_default(self): + """Status report omits the wait row when the option is not set.""" + pr = self.create_pr('bugfix/TEST-00001', 'development/4.3') + self.handle(pr.id, options=['bypass_jira_check']) + pr.add_comment('@%s status' % self.args.robot_username) + with self.assertRaises(exns.StatusReport) as ctx: + self.handle(pr.id, options=['bypass_jira_check'], backtrace=True) + self.assertNotIn('wait', ctx.exception.kwargs['status']) + + def test_status_command_help_text(self): + """The status command has a real description in the help page.""" + pr = self.create_pr('bugfix/TEST-00001', 'development/4.3') + self.handle(pr.id, options=['bypass_jira_check']) + pr.add_comment('@%s help' % self.args.robot_username) + with self.assertRaises(exns.HelpMessage) as ctx: + self.handle(pr.id, options=['bypass_jira_check'], backtrace=True) + self.assertIn('Print everything still missing before this pull ' + 'request can be merged.', ctx.exception.msg) + def test_bypass_options(self): # test bypass all approvals through an incorrect bitbucket comment pr = self.create_pr('bugfix/TEST-00001', 'development/4.3') diff --git a/bert_e/tests/unit/test_status_report.py b/bert_e/tests/unit/test_status_report.py new file mode 100644 index 00000000..e7d37b3e --- /dev/null +++ b/bert_e/tests/unit/test_status_report.py @@ -0,0 +1,88 @@ +"""Unit tests for the rows of the /status report.""" +from types import SimpleNamespace +from unittest.mock import Mock, patch + +from bert_e import exceptions as exns +from bert_e.workflow.gitwaterflow import commands + + +def _job(**settings): + defaults = dict(build_key='pre-merge', wait=False, use_queue=False, + bypass_build_status=False) + defaults.update(settings) + return SimpleNamespace(settings=SimpleNamespace(**defaults), + project_repo=Mock(), active_options=[], + author_bypass={}) + + +def _branch(name, sha): + return SimpleNamespace(name=name, get_latest_commit=lambda: sha) + + +def test_builds_lists_every_failing_branch_with_link(): + job = _job() + states = {'a': 'SUCCESSFUL', 'b': 'FAILED', 'c': 'INPROGRESS'} + job.project_repo.get_build_status.side_effect = lambda sha, key: states[ + sha] + job.project_repo.get_build_url.side_effect = ( + lambda sha, key: 'https://ci/{}'.format(sha) if sha == 'b' else None) + branches = [_branch('w/1', 'a'), _branch('w/2', 'b'), + _branch('w/3', 'c')] + with patch.object(commands, 'get_integration_branches', + return_value=branches): + item = commands._check_builds_status(job) + assert getattr(item, 'pass') is False + assert not item.pending + assert item.details == ['w/2: FAILED ([build](https://ci/b))', + 'w/3: INPROGRESS'] + + +def test_builds_pending_without_integration_branches(): + job = _job() + with patch.object(commands, 'get_integration_branches', + return_value=[]): + item = commands._check_builds_status(job) + assert item.pending + assert 'not yet evaluated' in item.details[0] + + +def test_wait_row(): + assert commands._check_wait_status(_job()) is None + item = commands._check_wait_status(_job(wait=True)) + assert getattr(item, 'pass') is False + + +def test_queue_row_disabled(): + assert commands._check_queue_status(_job()) is None + + +def test_queue_row_states(): + job = _job(use_queue=True) + branches = [_branch('w/1', 'a')] + with patch.object(commands, 'get_integration_branches', + return_value=branches), \ + patch('bert_e.workflow.gitwaterflow.queueing.already_in_queue', + return_value=True): + item = commands._check_queue_status(job) + assert getattr(item, 'pass') and item.details == ['queued'] + with patch.object(commands, 'get_integration_branches', + return_value=branches), \ + patch('bert_e.workflow.gitwaterflow.queueing.already_in_queue', + return_value=False): + item = commands._check_queue_status(job) + assert item.pending + + +def test_template_renders_every_state(): + report = { + 'ok': commands._StatusItem('Satisfied', True), + 'bypassed': commands._StatusItem('Bypassed', True, ['bypassed']), + 'ko': commands._StatusItem('Missing', False, ['author approval']), + 'wait': commands._StatusItem('Pending', False, ['not yet'], + pending=True), + } + msg = exns.StatusReport(status=report, active_options=[]).msg + assert '**Satisfied** | :sunny:\n' in msg + assert '**Bypassed** | :sunny: bypassed' in msg + assert '**Missing** | :exclamation: author approval' in msg + assert '**Pending** | :hourglass: not yet' in msg diff --git a/bert_e/workflow/gitwaterflow/commands.py b/bert_e/workflow/gitwaterflow/commands.py index a3cbf26a..f0d9cc3c 100644 --- a/bert_e/workflow/gitwaterflow/commands.py +++ b/bert_e/workflow/gitwaterflow/commands.py @@ -34,10 +34,13 @@ class _StatusItem: """A single row in the pull request status report.""" - def __init__(self, display_name, passed, details=None): + def __init__(self, display_name, passed, details=None, pending=False): self.display_name = display_name setattr(self, 'pass', passed) self.details = details or [] + # A pending item is neither satisfied nor failing: it cannot be + # evaluated yet (e.g. integration branches do not exist yet). + self.pending = pending def _check_approvals_status(job): @@ -59,7 +62,16 @@ def _check_approvals_status(job): peer_count >= required_peer and leader_count >= required_leader and not job.settings.unanimity): - return _StatusItem('Approvals', True) + bypassed = [ + name for name, active in ( + ('author', bypass_author_approval(job)), + ('peer', bypass_peer_approval(job)), + ('leader', bypass_leader_approval(job)), + ) if active + ] + details = (["{} approval bypassed".format('/'.join(bypassed))] + if bypassed else []) + return _StatusItem('Approvals', True, details=details) participants = set(job.pull_request.get_participants()) - {robot} approvals = set(job.pull_request.get_approvals()) @@ -107,42 +119,40 @@ def _check_approvals_status(job): def _check_builds_status(job): """Return a _StatusItem for integration branch build status, or None if - no integration branches exist or no build key is configured. + no build key is configured. + + Every integration branch whose build is not successful is listed (with a + link to its build when the git host provides one), not only the worst. + When integration branches do not exist yet, the item is pending. """ key = job.settings.build_key if not key: return None from .utils import bypass_build_status - wbranches = list(get_integration_branches(job)) - if not wbranches: - return None - if bypass_build_status(job): return _StatusItem('Integration builds', True, ['bypassed']) - ordered = { - 'SUCCESSFUL': 0, 'INPROGRESS': 1, - 'NOTSTARTED': 2, 'STOPPED': 3, 'FAILED': 4, - } - worst_rank = 0 - worst_branch = None - worst_state = 'SUCCESSFUL' + wbranches = list(get_integration_branches(job)) + if not wbranches: + return _StatusItem( + 'Integration builds', False, pending=True, + details=["not yet evaluated - will be checked once Bert-E " + "creates integration branches"]) + + details = [] for branch in wbranches: - state = job.project_repo.get_build_status( - branch.get_latest_commit(), key) - rank = ordered.get(state, 5) - if rank > worst_rank: - worst_rank = rank - worst_branch = branch - worst_state = state - - if worst_state == 'SUCCESSFUL': - return _StatusItem('Integration builds', True) - return _StatusItem( - 'Integration builds', False, - details=["{}: {}".format(worst_branch.name, worst_state)] - ) + commit = branch.get_latest_commit() + state = job.project_repo.get_build_status(commit, key) + if state == 'SUCCESSFUL': + continue + detail = "{}: {}".format(branch.name, state) + build_url = job.project_repo.get_build_url(commit, key) + if build_url: + detail += " ([build]({}))".format(build_url) + details.append(detail) + + return _StatusItem('Integration builds', not details, details=details) def _check_fix_versions_status(job): @@ -232,13 +242,52 @@ def _check_history_status(job): return _StatusItem('History', True) +def _check_wait_status(job): + """Return a failing _StatusItem when the `wait` option is active, or + None otherwise. + """ + if not job.settings.wait: + return None + return _StatusItem('Wait', False, + details=['`wait` option is set: Bert-E will not ' + 'merge until it is removed']) + + +def _check_queue_status(job): + """Return a _StatusItem describing whether the pull request is in the + merge queue, or None if queues are disabled or integration branches do + not exist yet. + """ + if not job.settings.use_queue: + return None + + from .queueing import already_in_queue + wbranches = list(get_integration_branches(job)) + if not wbranches: + return None + + if already_in_queue(job, wbranches): + return _StatusItem('Queue', True, details=['queued']) + return _StatusItem('Queue', False, pending=True, + details=['not queued yet']) + + def _build_status_report(job): - """Collect all available status checks for the pull request.""" + """Collect all available status checks for the pull request. + + Checks are read-only and are not short-circuited: every check that can + be evaluated is reported, so the author sees everything that is still + missing rather than only the next blocker. + """ from .branches import build_branch_cascade report = {} report['approvals'] = _check_approvals_status(job) + wait = _check_wait_status(job) + if wait is not None: + report['wait'] = wait + try: clone_git_repo(job) build_branch_cascade(job) @@ -259,6 +308,10 @@ def _build_status_report(job): if history is not None: report['history'] = history + queue = _check_queue_status(job) + if queue is not None: + report['queue'] = queue + return report @@ -292,7 +345,8 @@ def print_help(job, *args): @Reactor.command def status(job, *args): - """Print Bert-E's current status in the pull request.""" + """Print everything still missing before this pull request can be + merged.""" report = _build_status_report(job) raise StatusReport(status=report, active_options=job.active_options)