Report config errors as line comments
Change-Id: I17f827d661a46f75b72d85e7890237e76ebeb0e6
This commit is contained in:
parent
22e8e5b4f4
commit
bcfcf4f396
|
@ -17,6 +17,7 @@ driver=gerrit
|
|||
server=review.example.com
|
||||
user=jenkins
|
||||
sshkey=none
|
||||
password=badpassword
|
||||
|
||||
[connection github]
|
||||
driver=github
|
||||
|
|
|
@ -1033,6 +1033,16 @@ class TestInRepoConfig(ZuulTestCase):
|
|||
self.assertIn('Job non-existent-job not defined', A.messages[0],
|
||||
"A should have failed the check pipeline")
|
||||
self.assertHistory([])
|
||||
self.assertEqual(len(A.comments), 1)
|
||||
comments = sorted(A.comments, key=lambda x: x['line'])
|
||||
self.assertEqual(comments[0],
|
||||
{'file': '.zuul.yaml',
|
||||
'line': 9,
|
||||
'message': 'Job non-existent-job not defined',
|
||||
'reviewer': {'email': 'zuul@example.com',
|
||||
'name': 'Zuul',
|
||||
'username': 'jenkins'}}
|
||||
)
|
||||
|
||||
def test_dynamic_config_non_existing_job_in_template(self):
|
||||
"""Test that requesting a non existent job fails"""
|
||||
|
|
|
@ -233,7 +233,7 @@ def configuration_exceptions(stanza, conf, accumulator):
|
|||
content=indent(start_mark.snippet.rstrip()),
|
||||
start_mark=str(start_mark))
|
||||
|
||||
accumulator.addError(context, start_mark, m)
|
||||
accumulator.addError(context, start_mark, m, str(e))
|
||||
|
||||
|
||||
@contextmanager
|
||||
|
@ -269,7 +269,7 @@ def reference_exceptions(stanza, obj, accumulator):
|
|||
content=indent(start_mark.snippet.rstrip()),
|
||||
start_mark=str(start_mark))
|
||||
|
||||
accumulator.addError(context, start_mark, m)
|
||||
accumulator.addError(context, start_mark, m, str(e))
|
||||
|
||||
|
||||
class ZuulMark(object):
|
||||
|
@ -280,7 +280,10 @@ class ZuulMark(object):
|
|||
self.name = start_mark.name
|
||||
self.index = start_mark.index
|
||||
self.line = start_mark.line
|
||||
self.end_line = end_mark.line
|
||||
self.end_index = end_mark.index
|
||||
self.column = start_mark.column
|
||||
self.end_column = end_mark.column
|
||||
self.snippet = stream[start_mark.index:end_mark.index]
|
||||
|
||||
def __str__(self):
|
||||
|
|
|
@ -34,6 +34,14 @@ 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))
|
||||
return ret
|
||||
|
||||
def report(self, item):
|
||||
|
|
|
@ -208,8 +208,8 @@ class PipelineManager(object):
|
|||
# Similarly, reset the item state.
|
||||
if item.current_build_set.unable_to_merge:
|
||||
item.setUnableToMerge()
|
||||
if item.current_build_set.config_error:
|
||||
item.setConfigError(item.current_build_set.config_error)
|
||||
if item.current_build_set.config_errors:
|
||||
item.setConfigErrors(item.current_build_set.config_errors)
|
||||
if item.dequeued_needing_change:
|
||||
item.setDequeuedNeedingChange()
|
||||
|
||||
|
@ -480,14 +480,17 @@ class PipelineManager(object):
|
|||
# Find a layout loading error that match
|
||||
# the current item.change and only report
|
||||
# if one is found.
|
||||
relevant_errors = []
|
||||
for err in layout.loading_errors.errors:
|
||||
econtext = err.key.context
|
||||
if ((err.key not in
|
||||
parent_layout.loading_errors.error_keys) or
|
||||
(econtext.project == item.change.project.name and
|
||||
econtext.branch == item.change.branch)):
|
||||
item.setConfigError(err.error)
|
||||
return None
|
||||
relevant_errors.append(err)
|
||||
if relevant_errors:
|
||||
item.setConfigErrors(relevant_errors)
|
||||
return None
|
||||
self.log.info(
|
||||
"Configuration syntax error not related to "
|
||||
"change context. Error won't be reported.")
|
||||
|
@ -557,7 +560,7 @@ class PipelineManager(object):
|
|||
return False
|
||||
if build_set.unable_to_merge:
|
||||
return False
|
||||
if build_set.config_error:
|
||||
if build_set.config_errors:
|
||||
return False
|
||||
return True
|
||||
|
||||
|
@ -650,7 +653,7 @@ class PipelineManager(object):
|
|||
if (not item.live) and (not dequeued):
|
||||
self.dequeueItem(item)
|
||||
changed = dequeued = True
|
||||
if item.current_build_set.config_error:
|
||||
if item.current_build_set.config_errors:
|
||||
failing_reasons.append("it has an invalid configuration")
|
||||
if (not item.live) and (not dequeued):
|
||||
self.dequeueItem(item)
|
||||
|
@ -808,7 +811,7 @@ class PipelineManager(object):
|
|||
item.change.project, self.pipeline, item.change))
|
||||
project_in_pipeline = False
|
||||
actions = []
|
||||
elif item.getConfigError():
|
||||
elif item.getConfigErrors():
|
||||
self.log.debug("Invalid config for change %s" % item.change)
|
||||
# TODOv3(jeblair): consider a new reporter action for this
|
||||
actions = self.pipeline.merge_failure_actions
|
||||
|
|
|
@ -129,8 +129,9 @@ class ConfigurationErrorKey(object):
|
|||
class ConfigurationError(object):
|
||||
|
||||
"""A configuration error"""
|
||||
def __init__(self, context, mark, error):
|
||||
def __init__(self, context, mark, error, short_error=None):
|
||||
self.error = str(error)
|
||||
self.short_error = short_error
|
||||
self.key = ConfigurationErrorKey(context, mark, self.error)
|
||||
|
||||
|
||||
|
@ -141,8 +142,8 @@ class LoadingErrors(object):
|
|||
self.errors = []
|
||||
self.error_keys = set()
|
||||
|
||||
def addError(self, context, mark, error):
|
||||
e = ConfigurationError(context, mark, error)
|
||||
def addError(self, context, mark, error, short_error=None):
|
||||
e = ConfigurationError(context, mark, error, short_error)
|
||||
self.errors.append(e)
|
||||
self.error_keys.add(e.key)
|
||||
|
||||
|
@ -1641,7 +1642,7 @@ class BuildSet(object):
|
|||
self.dependent_changes = None
|
||||
self.merger_items = None
|
||||
self.unable_to_merge = False
|
||||
self.config_error = None # None or an error message string.
|
||||
self.config_errors = [] # list of ConfigurationErrors
|
||||
self.failing_reasons = []
|
||||
self.debug_messages = []
|
||||
self.merge_state = self.NEW
|
||||
|
@ -1869,7 +1870,7 @@ class QueueItem(object):
|
|||
return True
|
||||
|
||||
def areAllJobsComplete(self):
|
||||
if (self.current_build_set.config_error or
|
||||
if (self.current_build_set.config_errors or
|
||||
self.current_build_set.unable_to_merge):
|
||||
return True
|
||||
if not self.hasJobGraph():
|
||||
|
@ -1937,8 +1938,8 @@ class QueueItem(object):
|
|||
def didMergerFail(self):
|
||||
return self.current_build_set.unable_to_merge
|
||||
|
||||
def getConfigError(self):
|
||||
return self.current_build_set.config_error
|
||||
def getConfigErrors(self):
|
||||
return self.current_build_set.config_errors
|
||||
|
||||
def wasDequeuedNeedingChange(self):
|
||||
return self.dequeued_needing_change
|
||||
|
@ -2131,7 +2132,11 @@ class QueueItem(object):
|
|||
self._setAllJobsSkipped()
|
||||
|
||||
def setConfigError(self, error):
|
||||
self.current_build_set.config_error = error
|
||||
err = ConfigurationError(None, None, error)
|
||||
self.setConfigErrors([err])
|
||||
|
||||
def setConfigErrors(self, errors):
|
||||
self.current_build_set.config_errors = errors
|
||||
self._setAllJobsSkipped()
|
||||
|
||||
def _setAllJobsSkipped(self):
|
||||
|
|
|
@ -90,8 +90,8 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
|
|||
msg = 'This change depends on a change that failed to merge.\n'
|
||||
elif item.didMergerFail():
|
||||
msg = item.pipeline.merge_failure_message
|
||||
elif item.getConfigError():
|
||||
msg = item.getConfigError()
|
||||
elif item.getConfigErrors():
|
||||
msg = str(item.getConfigErrors()[0].error)
|
||||
else:
|
||||
msg = item.pipeline.failure_message
|
||||
if with_jobs:
|
||||
|
|
Loading…
Reference in New Issue