Merge "Support line comments in Gerrit"
This commit is contained in:
commit
095cf04777
|
@ -678,6 +678,27 @@ zuul.child_jobs is empty, all jobs will be marked as SKIPPED. Invalid child jobs
|
|||
are stripped and ignored, if only invalid jobs are listed it is the same as
|
||||
providing an empty list to zuul.child_jobs.
|
||||
|
||||
Leaving file comments
|
||||
~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
To instruct the reporters to leave line comments on files in the
|
||||
change, set the **zuul.file_comments** value. For example:
|
||||
|
||||
.. code-block:: yaml
|
||||
|
||||
tasks:
|
||||
- zuul_return:
|
||||
data:
|
||||
zuul:
|
||||
file_comments:
|
||||
path/to/file.py:
|
||||
- line: 42
|
||||
message: "Line too long"
|
||||
- line: 82
|
||||
message: "Line too short"
|
||||
|
||||
Not all reporters currently support line comments; in these cases,
|
||||
reporters will simply ignore this data.
|
||||
|
||||
.. _build_status:
|
||||
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
features:
|
||||
- |
|
||||
Zuul now supports leaving file comments, though currently only when using
|
||||
the Gerrit driver. See the documentation for ``zuul_return`` for details.
|
|
@ -177,9 +177,10 @@ class FakeGerritChange(object):
|
|||
self.needed_by_changes = []
|
||||
self.fail_merge = False
|
||||
self.messages = []
|
||||
self.comments = []
|
||||
self.data = {
|
||||
'branch': branch,
|
||||
'comments': [],
|
||||
'comments': self.comments,
|
||||
'commitMessage': subject,
|
||||
'createdOn': time.time(),
|
||||
'id': 'I' + random_sha1(),
|
||||
|
@ -272,6 +273,19 @@ class FakeGerritChange(object):
|
|||
self.patchsets.append(d)
|
||||
self.data['submitRecords'] = self.getSubmitRecords()
|
||||
|
||||
def addComment(self, filename, line, message, name, email, username):
|
||||
comment = {
|
||||
'file': filename,
|
||||
'line': int(line),
|
||||
'reviewer': {
|
||||
'name': name,
|
||||
'email': email,
|
||||
'username': username,
|
||||
},
|
||||
'message': message,
|
||||
}
|
||||
self.comments.append(comment)
|
||||
|
||||
def getPatchsetCreatedEvent(self, patchset):
|
||||
event = {"type": "patchset-created",
|
||||
"change": {"project": self.project,
|
||||
|
@ -525,8 +539,9 @@ class GerritWebServer(object):
|
|||
|
||||
message = data['message']
|
||||
action = data['labels']
|
||||
comments = data.get('comments', {})
|
||||
fake_gerrit._test_handle_review(
|
||||
int(change.data['number']), message, action)
|
||||
int(change.data['number']), message, action, comments)
|
||||
self.send_response(200)
|
||||
self.end_headers()
|
||||
|
||||
|
@ -655,13 +670,14 @@ class FakeGerritConnection(gerritconnection.GerritConnection):
|
|||
}
|
||||
return event
|
||||
|
||||
def review(self, change, message, action):
|
||||
def review(self, change, message, action, file_comments):
|
||||
if self.web_server:
|
||||
return super(FakeGerritConnection, self).review(
|
||||
change, message, action)
|
||||
change, message, action, file_comments)
|
||||
self._test_handle_review(int(change.number), message, action)
|
||||
|
||||
def _test_handle_review(self, change_number, message, action):
|
||||
def _test_handle_review(self, change_number, message, action,
|
||||
file_comments=None):
|
||||
# Handle a review action from a test
|
||||
change = self.changes[change_number]
|
||||
|
||||
|
@ -682,6 +698,12 @@ class FakeGerritConnection(gerritconnection.GerritConnection):
|
|||
if message:
|
||||
change.messages.append(message)
|
||||
|
||||
if file_comments:
|
||||
for filename, commentlist in file_comments.items():
|
||||
for comment in commentlist:
|
||||
change.addComment(filename, comment['line'],
|
||||
comment['message'], 'Zuul',
|
||||
'zuul@example.com', self.user)
|
||||
if 'submit' in action:
|
||||
change.setMerged()
|
||||
if message:
|
||||
|
|
|
@ -0,0 +1,17 @@
|
|||
- pipeline:
|
||||
name: check
|
||||
manager: independent
|
||||
post-review: true
|
||||
trigger:
|
||||
gerrit:
|
||||
- event: patchset-created
|
||||
success:
|
||||
gerrit:
|
||||
Verified: 1
|
||||
failure:
|
||||
gerrit:
|
||||
Verified: -1
|
||||
|
||||
- job:
|
||||
name: base
|
||||
parent: null
|
|
@ -0,0 +1 @@
|
|||
test
|
17
tests/fixtures/config/gerrit-file-comments/git/org_project/playbooks/file-comments.yaml
vendored
Normal file
17
tests/fixtures/config/gerrit-file-comments/git/org_project/playbooks/file-comments.yaml
vendored
Normal file
|
@ -0,0 +1,17 @@
|
|||
- hosts: all
|
||||
tasks:
|
||||
- zuul_return:
|
||||
data:
|
||||
zuul:
|
||||
file_comments:
|
||||
path/to/file.py:
|
||||
- line: 42
|
||||
message: line too long
|
||||
- line: 82
|
||||
message: line too short
|
||||
otherfile.txt:
|
||||
- line: 21
|
||||
message: |
|
||||
This is a much longer message.
|
||||
|
||||
With multiple paragraphs.
|
|
@ -0,0 +1,10 @@
|
|||
- job:
|
||||
parent: base
|
||||
name: file-comments
|
||||
run: playbooks/file-comments.yaml
|
||||
|
||||
- project:
|
||||
name: org/project
|
||||
check:
|
||||
jobs:
|
||||
- file-comments
|
|
@ -0,0 +1,8 @@
|
|||
- tenant:
|
||||
name: tenant-one
|
||||
source:
|
||||
gerrit:
|
||||
config-projects:
|
||||
- common-config
|
||||
untrusted-projects:
|
||||
- org/project
|
|
@ -16,7 +16,7 @@ import os
|
|||
from unittest import mock
|
||||
|
||||
import tests.base
|
||||
from tests.base import BaseTestCase, ZuulTestCase
|
||||
from tests.base import BaseTestCase, ZuulTestCase, AnsibleZuulTestCase
|
||||
from zuul.driver.gerrit import GerritDriver
|
||||
from zuul.driver.gerrit.gerritconnection import GerritConnection
|
||||
|
||||
|
@ -100,3 +100,44 @@ class TestGerritWeb(ZuulTestCase):
|
|||
'label1')
|
||||
self.assertEqual(self.getJobFromHistory('project-test2').node,
|
||||
'label1')
|
||||
|
||||
|
||||
class TestFileComments(AnsibleZuulTestCase):
|
||||
# A temporary class to hold new tests while others are disabled
|
||||
config_file = 'zuul-gerrit-web.conf'
|
||||
tenant_config_file = 'config/gerrit-file-comments/main.yaml'
|
||||
|
||||
def test_multiple_tenants(self):
|
||||
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
|
||||
A.addApproval('Code-Review', 2)
|
||||
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
self.assertEqual(self.getJobFromHistory('file-comments').result,
|
||||
'SUCCESS')
|
||||
self.assertEqual(len(A.comments), 3)
|
||||
comments = sorted(A.comments, key=lambda x: x['line'])
|
||||
self.assertEqual(comments[0],
|
||||
{'file': 'otherfile.txt',
|
||||
'line': 21,
|
||||
'message': 'This is a much longer message.\n\n'
|
||||
'With multiple paragraphs.\n',
|
||||
'reviewer': {'email': 'zuul@example.com',
|
||||
'name': 'Zuul',
|
||||
'username': 'jenkins'}}
|
||||
)
|
||||
self.assertEqual(comments[1],
|
||||
{'file': 'path/to/file.py',
|
||||
'line': 42,
|
||||
'message': 'line too long',
|
||||
'reviewer': {'email': 'zuul@example.com',
|
||||
'name': 'Zuul',
|
||||
'username': 'jenkins'}}
|
||||
)
|
||||
self.assertEqual(comments[2],
|
||||
{'file': 'path/to/file.py',
|
||||
'line': 82,
|
||||
'message': 'line too short',
|
||||
'reviewer': {'email': 'zuul@example.com',
|
||||
'name': 'Zuul',
|
||||
'username': 'jenkins'}}
|
||||
)
|
||||
|
|
|
@ -776,14 +776,16 @@ class GerritConnection(BaseConnection):
|
|||
def eventDone(self):
|
||||
self.event_queue.task_done()
|
||||
|
||||
def review(self, change, message, action={}):
|
||||
def review(self, change, message, action={},
|
||||
file_comments={}):
|
||||
if self.session:
|
||||
meth = self.review_http
|
||||
else:
|
||||
meth = self.review_ssh
|
||||
return meth(change, message, action)
|
||||
return meth(change, message, action, file_comments)
|
||||
|
||||
def review_ssh(self, change, message, action={}):
|
||||
def review_ssh(self, change, message, action={},
|
||||
file_comments={}):
|
||||
project = change.project.name
|
||||
cmd = 'gerrit review --project %s' % project
|
||||
if message:
|
||||
|
|
|
@ -25,6 +25,17 @@ class GerritReporter(BaseReporter):
|
|||
name = 'gerrit'
|
||||
log = logging.getLogger("zuul.GerritReporter")
|
||||
|
||||
def _getFileComments(self, item):
|
||||
ret = {}
|
||||
for build in item.current_build_set.getBuilds():
|
||||
fc = build.result_data.get('zuul', {}).get('file_comments')
|
||||
if not fc:
|
||||
continue
|
||||
for fn, comments in fc.items():
|
||||
existing_comments = ret.setdefault(fn, [])
|
||||
existing_comments += comments
|
||||
return ret
|
||||
|
||||
def report(self, item):
|
||||
"""Send a message to gerrit."""
|
||||
|
||||
|
@ -39,13 +50,16 @@ class GerritReporter(BaseReporter):
|
|||
return
|
||||
|
||||
message = self._formatItemReport(item)
|
||||
comments = self._getFileComments(item)
|
||||
|
||||
self.log.debug("Report change %s, params %s, message: %s" %
|
||||
(item.change, self.config, message))
|
||||
self.log.debug("Report change %s, params %s,"
|
||||
" message: %s, comments: %s" %
|
||||
(item.change, self.config, message, comments))
|
||||
item.change._ref_sha = item.change.project.source.getRefSha(
|
||||
item.change.project, 'refs/heads/' + item.change.branch)
|
||||
|
||||
return self.connection.review(item.change, message, self.config)
|
||||
return self.connection.review(item.change, message, self.config,
|
||||
comments)
|
||||
|
||||
def getSubmitAllowNeeds(self):
|
||||
"""Get a list of code review labels that are allowed to be
|
||||
|
|
Loading…
Reference in New Issue