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)