Bug 1316925 - Keep track of failed linters in stylish formatter summary, r?jgraham
This replaces the "return_code" property on the LintRoller object with a list of "failed"
linters. This is a bit more useful as it lets us know exactly which linters had a problem
(whereas previously we just knew *something* went wrong). This patch pushes determining
the return code back into cli.py, which I think is fine.
In addition, we now pass the list of failed linters into the formatter. This allows us to
clarify exactly how many linters hit a failure which is a lot better than a seemingly
"successful" summary message.
Finally I also removed the "no files to lint" message because I've seen several people
confuse it for an error. I'll probably add it back as a debug log message when we switch
to using mozlog for output.
MozReview-Commit-ID: 4wyCeOZdOf8
--- a/python/mozlint/mozlint/cli.py
+++ b/python/mozlint/mozlint/cli.py
@@ -100,16 +100,16 @@ def run(paths, linters, fmt, rev, workdi
# run all linters
results = lint.roll(paths, rev=rev, workdir=workdir)
formatter = formatters.get(fmt)
# Explicitly utf-8 encode the output as some of the formatters make
# use of unicode characters. This will prevent a UnicodeEncodeError
# on environments where utf-8 isn't the default
- print(formatter(results).encode('utf-8', 'replace'))
- return lint.return_code
+ print(formatter(results, failed=lint.failed).encode('utf-8', 'replace'))
+ return 1 if results or lint.failed else 0
if __name__ == '__main__':
parser = MozlintParser()
args = vars(parser.parse_args())
sys.exit(run(**args))
--- a/python/mozlint/mozlint/formatters/__init__.py
+++ b/python/mozlint/mozlint/formatters/__init__.py
@@ -5,17 +5,17 @@
import json
from ..result import ResultEncoder
from .stylish import StylishFormatter
from .treeherder import TreeherderFormatter
class JSONFormatter(object):
- def __call__(self, results):
+ def __call__(self, results, **kwargs):
return json.dumps(results, cls=ResultEncoder)
all_formatters = {
'json': JSONFormatter,
'stylish': StylishFormatter,
'treeherder': TreeherderFormatter,
}
--- a/python/mozlint/mozlint/formatters/stylish.py
+++ b/python/mozlint/mozlint/formatters/stylish.py
@@ -38,17 +38,17 @@ class StylishFormatter(object):
_colors = {
'grey': [247, 8, 7],
'red': [1],
'yellow': [3],
'brightred': [9, 1],
'brightyellow': [11, 3],
}
fmt = " {c1}{lineno}{column} {c2}{level}{normal} {message} {c1}{rule}({linter}){normal}"
- fmt_summary = "{t.bold}{c}\u2716 {problem} ({error}, {warning}){t.normal}"
+ fmt_summary = "{t.bold}{c}\u2716 {problem} ({error}, {warning}{failure}){t.normal}"
def __init__(self, disable_colors=None):
if disable_colors or not blessings:
self.term = NullTerminal()
else:
self.term = blessings.Terminal()
self.num_colors = self.term.number_of_colors
@@ -72,18 +72,19 @@ class StylishFormatter(object):
self.max_level = max(self.max_level, len(str(err.level)))
self.max_message = max(self.max_message, len(err.message))
def _pluralize(self, s, num):
if num != 1:
s += 's'
return str(num) + ' ' + s
- def __call__(self, result):
+ def __call__(self, result, failed=None, **kwargs):
message = []
+ failed = failed or []
num_errors = 0
num_warnings = 0
for path, errors in sorted(result.iteritems()):
self._reset_max()
message.append(self.term.underline(path))
# Do a first pass to calculate required padding
@@ -105,18 +106,26 @@ class StylishFormatter(object):
level=err.level.ljust(self.max_level),
message=err.message.ljust(self.max_message),
rule='{} '.format(err.rule) if err.rule else '',
linter=err.linter.lower(),
))
message.append('') # newline
+ # If there were failures, make it clear which linters failed
+ for fail in failed:
+ message.append("{c}A failure occured in the {name} linter.".format(
+ c=self.color('brightred'),
+ name=fail,
+ ))
+
# Print a summary
message.append(self.fmt_summary.format(
t=self.term,
- c=self.color('brightred') if num_errors else self.color('brightyellow'),
- problem=self._pluralize('problem', num_errors + num_warnings),
+ c=self.color('brightred') if num_errors or failed else self.color('brightyellow'),
+ problem=self._pluralize('problem', num_errors + num_warnings + len(failed)),
error=self._pluralize('error', num_errors),
warning=self._pluralize('warning', num_warnings),
+ failure=', {}'.format(self._pluralize('failure', len(failed))) if failed else '',
))
return '\n'.join(message)
--- a/python/mozlint/mozlint/formatters/treeherder.py
+++ b/python/mozlint/mozlint/formatters/treeherder.py
@@ -11,17 +11,17 @@ class TreeherderFormatter(object):
"""Formatter for treeherder friendly output.
This formatter looks ugly, but prints output such that
treeherder is able to highlight the errors and warnings.
This is a stop-gap until bug 1276486 is fixed.
"""
fmt = "TEST-UNEXPECTED-{level} | {path}:{lineno}{column} | {message} ({rule})"
- def __call__(self, result):
+ def __call__(self, result, **kwargs):
message = []
for path, errors in sorted(result.iteritems()):
for err in errors:
assert isinstance(err, ResultContainer)
d = {s: getattr(err, s) for s in err.__slots__}
d["column"] = ":%s" % d["column"] if d["column"] else ""
d['level'] = d['level'].upper()
--- a/python/mozlint/mozlint/roller.py
+++ b/python/mozlint/mozlint/roller.py
@@ -20,40 +20,40 @@ from .errors import LintersNotConfigured
from .types import supported_types
from .parser import Parser
from .vcs import VCSFiles
def _run_linters(queue, paths, **lintargs):
parse = Parser()
results = defaultdict(list)
- return_code = 0
+ failed = []
while True:
try:
# The astute reader may wonder what is preventing the worker from
# grabbing the next linter from the queue after a SIGINT. Because
# this is a Manager.Queue(), it is itself in a child process which
# also received SIGINT. By the time the worker gets back here, the
# Queue is dead and IOError is raised.
linter_path = queue.get(False)
except (Empty, IOError):
- return results, return_code
+ return results, failed
# Ideally we would pass the entire LINTER definition as an argument
# to the worker instead of re-parsing it. But passing a function from
# a dynamically created module (with imp) does not seem to be possible
# with multiprocessing on Windows.
linter = parse(linter_path)
func = supported_types[linter['type']]
res = func(paths, linter, **lintargs) or []
if not isinstance(res, (list, tuple)):
if res:
- return_code = 1
+ failed.append(linter['name'])
continue
for r in res:
results[r.path].append(r)
def _run_worker(*args, **lintargs):
try:
@@ -79,17 +79,18 @@ class LintRoller(object):
def __init__(self, root=None, **lintargs):
self.parse = Parser()
self.vcs = VCSFiles()
self.linters = []
self.lintargs = lintargs
self.lintargs['root'] = root or self.vcs.root or os.getcwd()
- self.return_code = None
+ # linters that return non-zero
+ self.failed = None
def read(self, paths):
"""Parse one or more linters and add them to the registry.
:param paths: A path or iterable of paths to linter definitions.
"""
if isinstance(paths, basestring):
paths = (paths,)
@@ -138,17 +139,17 @@ class LintRoller(object):
for i in range(num_procs):
workers.append(
pool.apply_async(_run_worker, args=(queue, paths), kwds=self.lintargs))
pool.close()
# ignore SIGINT in parent so we can still get partial results
# from child processes. These should shutdown quickly anyway.
signal.signal(signal.SIGINT, signal.SIG_IGN)
- self.return_code = 0
+ self.failed = []
for worker in workers:
# parent process blocks on worker.get()
- results, return_code = worker.get()
- if results or return_code:
- self.return_code = 1
+ results, failed = worker.get()
+ if failed:
+ self.failed.extend(failed)
for k, v in results.iteritems():
all_results[k].extend(v)
return all_results
--- a/python/mozlint/mozlint/types.py
+++ b/python/mozlint/mozlint/types.py
@@ -26,17 +26,16 @@ class BaseType(object):
:param paths: Paths to lint. Can be a file or directory.
:param linter: Linter definition paths are being linted against.
:param lintargs: External arguments to the linter not defined in
the definition, but passed in by a consumer.
:returns: A list of :class:`~result.ResultContainer` objects.
"""
paths = filterpaths(paths, linter, **lintargs)
if not paths:
- print("{}: no files to lint in specified paths".format(linter['name']))
return
if self.batch:
return self._lint(paths, linter, **lintargs)
errors = []
try:
for p in paths:
--- a/python/mozlint/test/test_roller.py
+++ b/python/mozlint/test/test_roller.py
@@ -22,17 +22,17 @@ def test_roll_no_linters_configured(lint
lint.roll(files)
def test_roll_successful(lint, linters, files):
lint.read(linters)
result = lint.roll(files)
assert len(result) == 1
- assert lint.return_code == 1
+ assert lint.failed == []
path = result.keys()[0]
assert os.path.basename(path) == 'foobar.js'
errors = result[path]
assert isinstance(errors, list)
assert len(errors) == 6
@@ -54,29 +54,29 @@ def test_roll_catch_exception(lint, lint
def test_roll_with_excluded_path(lint, linters, files):
lint.lintargs.update({'exclude': ['**/foobar.js']})
lint.read(linters)
result = lint.roll(files)
assert len(result) == 0
- assert lint.return_code == 0
+ assert lint.failed == []
def test_roll_with_invalid_extension(lint, lintdir, filedir):
lint.read(os.path.join(lintdir, 'external.lint'))
result = lint.roll(os.path.join(filedir, 'foobar.py'))
assert len(result) == 0
- assert lint.return_code == 0
+ assert lint.failed == []
def test_roll_with_failure_code(lint, lintdir, files):
lint.read(os.path.join(lintdir, 'badreturncode.lint'))
- assert lint.return_code is None
+ assert lint.failed is None
result = lint.roll(files)
assert len(result) == 0
- assert lint.return_code == 1
+ assert lint.failed == ['BadReturnCodeLinter']
if __name__ == '__main__':
sys.exit(pytest.main(['--verbose', __file__]))