Scope config line comments to change
Only add line comments for configuration errors if they are for files in the current change. Put this in the reporter base class so that all drivers can use it. Add a test case for config errors in line comments in general, and for this case as well. Change-Id: I441474b93ba70c055f7f839ecf76547ac3356e0e
This commit is contained in:
parent
822c581784
commit
f87a97ae80
|
@ -13,6 +13,7 @@
|
|||
# under the License.
|
||||
|
||||
import os
|
||||
import textwrap
|
||||
from unittest import mock
|
||||
|
||||
import tests.base
|
||||
|
@ -101,9 +102,63 @@ class TestGerritWeb(ZuulTestCase):
|
|||
self.assertEqual(self.getJobFromHistory('project-test2').node,
|
||||
'label1')
|
||||
|
||||
def test_dynamic_line_comment(self):
|
||||
in_repo_conf = textwrap.dedent(
|
||||
"""
|
||||
- job:
|
||||
name: garbage-job
|
||||
garbage: True
|
||||
""")
|
||||
|
||||
file_dict = {'.zuul.yaml': in_repo_conf}
|
||||
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
|
||||
files=file_dict)
|
||||
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
|
||||
self.assertEqual(A.patchsets[0]['approvals'][0]['value'], "-1")
|
||||
self.assertIn('Zuul encountered a syntax error',
|
||||
A.messages[0])
|
||||
comments = sorted(A.comments, key=lambda x: x['line'])
|
||||
self.assertEqual(comments[0],
|
||||
{'file': '.zuul.yaml',
|
||||
'line': 4,
|
||||
'message': "extra keys not allowed @ "
|
||||
"data['garbage']",
|
||||
'range': {'end_character': 0,
|
||||
'end_line': 4,
|
||||
'start_character': 2,
|
||||
'start_line': 2},
|
||||
'reviewer': {'email': 'zuul@example.com',
|
||||
'name': 'Zuul',
|
||||
'username': 'jenkins'}}
|
||||
)
|
||||
|
||||
def test_dependent_dynamic_line_comment(self):
|
||||
in_repo_conf = textwrap.dedent(
|
||||
"""
|
||||
- job:
|
||||
name: garbage-job
|
||||
garbage: True
|
||||
""")
|
||||
|
||||
file_dict = {'.zuul.yaml': in_repo_conf}
|
||||
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
|
||||
files=file_dict)
|
||||
B = self.fake_gerrit.addFakeChange('org/project1', 'master', 'B')
|
||||
B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
|
||||
B.subject, A.data['id'])
|
||||
|
||||
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
|
||||
self.assertEqual(B.patchsets[0]['approvals'][0]['value'], "-1")
|
||||
self.assertIn('Zuul encountered a syntax error',
|
||||
B.messages[0])
|
||||
self.assertEqual(B.comments, [])
|
||||
|
||||
|
||||
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'
|
||||
|
||||
|
|
|
@ -34,19 +34,7 @@ class GerritReporter(BaseReporter):
|
|||
for fn, comments in fc.items():
|
||||
existing_comments = ret.setdefault(fn, [])
|
||||
existing_comments += comments
|
||||
for err in item.getConfigErrors():
|
||||
context = err.key.context
|
||||
mark = err.key.mark
|
||||
if not (context and mark and err.short_error):
|
||||
continue
|
||||
existing_comments = ret.setdefault(context.path, [])
|
||||
existing_comments.append(dict(line=mark.end_line,
|
||||
message=err.short_error,
|
||||
range=dict(
|
||||
start_line=mark.line + 1,
|
||||
start_character=mark.column,
|
||||
end_line=mark.end_line,
|
||||
end_character=mark.end_column)))
|
||||
self.addConfigurationErrorComments(item, ret)
|
||||
return ret
|
||||
|
||||
def report(self, item):
|
||||
|
|
|
@ -49,6 +49,39 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
|
|||
def postConfig(self):
|
||||
"""Run tasks after configuration is reloaded"""
|
||||
|
||||
def addConfigurationErrorComments(self, item, comments):
|
||||
"""Add file comments for configuration errors.
|
||||
|
||||
Updates the comments dictionary with additional file comments
|
||||
for any relevant configuration errors for this item's change.
|
||||
|
||||
:arg QueueItem item: The queue item
|
||||
:arg dict comments: a file comments dictionary
|
||||
|
||||
"""
|
||||
|
||||
for err in item.getConfigErrors():
|
||||
context = err.key.context
|
||||
mark = err.key.mark
|
||||
if not (context and mark and err.short_error):
|
||||
continue
|
||||
if context.project != item.change.project:
|
||||
continue
|
||||
if not hasattr(item.change, 'branch'):
|
||||
continue
|
||||
if context.branch != item.change.branch:
|
||||
continue
|
||||
if context.path not in item.change.files:
|
||||
continue
|
||||
existing_comments = comments.setdefault(context.path, [])
|
||||
existing_comments.append(dict(line=mark.end_line,
|
||||
message=err.short_error,
|
||||
range=dict(
|
||||
start_line=mark.line + 1,
|
||||
start_character=mark.column,
|
||||
end_line=mark.end_line,
|
||||
end_character=mark.end_column)))
|
||||
|
||||
def _getFormatter(self):
|
||||
format_methods = {
|
||||
'start': self._formatItemReportStart,
|
||||
|
|
Loading…
Reference in New Issue