Merge "Report config errors as line comments"

This commit is contained in:
Zuul
2018-07-29 05:30:18 +00:00
committed by Gerrit Code Review
7 changed files with 49 additions and 19 deletions

View File

@@ -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):

View File

@@ -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):

View File

@@ -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

View File

@@ -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):

View File

@@ -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: