Skip to content

Commit

Permalink
Enable progressive mode
Browse files Browse the repository at this point in the history
Adds a new option --progressive that ease adoption of the linter as
it will only report failure if it detects an increase number
of violations since previous commit.
  • Loading branch information
ssbarnea committed Oct 31, 2020
1 parent 295ae7f commit 1eb9e1c
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 19 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Expand Up @@ -41,3 +41,6 @@ pip-wheel-metadata

# mypy
.mypy_cache

# .cache is used by progressive mode
.cache
19 changes: 19 additions & 0 deletions README.rst
Expand Up @@ -94,6 +94,8 @@ The following is the output from ``ansible-lint --help``, providing an overview
-q quieter, although not silent output
-p parseable output in the format of pep8
--parseable-severity parseable output including severity of rule
--progressive Return success if it detects a reduction in number of violations compared with
previous git commit. This feature works only on git repository clones.
-r RULESDIR Specify custom rule directories. Add -R to keep using embedded rules from
/usr/local/lib/python3.8/site-packages/ansiblelint/rules
-R Keep default rules when using -r
Expand All @@ -111,6 +113,23 @@ The following is the output from ``ansible-lint --help``, providing an overview
-c CONFIG_FILE Specify configuration file to use. Defaults to ".ansible-lint"
--version show program's version number and exit
Progressive mode
----------------

In order to ease tool adoption, git users can enable the progressive mode using
``--progressive`` option. This makes the linter return a success even if
some failures are found, as long the total number of violations did not
increase since the previous commit.

As expected, this mode makes the linter run twice if it finds any violations.
The second run is performed against a temporary git working copy that contains
the previous commit. All the violations that were already present are removed
from the list and the final result is displayed.

The most notable benefit introduced by this mode it does not prevent merging
new code while allowing developer to address historical violation at his own
speed.

CI/CD
-----

Expand Down
115 changes: 96 additions & 19 deletions lib/ansiblelint/__main__.py
Expand Up @@ -24,13 +24,16 @@
import logging
import os
import pathlib
import subprocess
import sys
from typing import TYPE_CHECKING, List, Set, Type
from contextlib import contextmanager
from typing import TYPE_CHECKING, Any, List, Set, Type, Union

from rich.markdown import Markdown

from ansiblelint import cli, formatters
from ansiblelint.color import console, console_stderr
from ansiblelint.file_utils import cwd
from ansiblelint.generate_docs import rules_as_rich, rules_as_rst
from ansiblelint.rules import RulesCollection
from ansiblelint.runner import Runner
Expand Down Expand Up @@ -96,8 +99,9 @@ def report_outcome(matches: List["MatchError"], options) -> int:
# .ansible-lint
warn_list: # or 'skip_list' to silence them completely
"""
matches_unignored = [match for match in matches if not match.ignored]

matched_rules = {match.rule.id: match.rule for match in matches}
matched_rules = {match.rule.id: match.rule for match in matches_unignored}
for id in sorted(matched_rules.keys()):
if {id, *matched_rules[id].tags}.isdisjoint(options.warn_list):
msg += f" - '{id}' # {matched_rules[id].shortdesc}\n"
Expand Down Expand Up @@ -151,6 +155,76 @@ def main() -> int:
skip.update(str(s).split(','))
options.skip_list = frozenset(skip)

matches = _get_matches(rules, options)

# Assure we do not print duplicates and the order is consistent
matches = sorted(set(matches))

mark_as_success = False
if matches and options.progressive:
_logger.info(
"Matches found, running again on previous revision in order to detect regressions")
with _previous_revision():
old_matches = _get_matches(rules, options)
# remove old matches from current list
matches_delta = list(set(matches) - set(old_matches))
if len(matches_delta) == 0:
_logger.warning(
"Total violations not increased since previous "
"commit, will mark result as success. (%s -> %s)",
len(old_matches), len(matches_delta))
mark_as_success = True

ignored = 0
for match in matches:
# if match is not new, mark is as ignored
if match not in matches_delta:
match.ignored = True
ignored += 1
if ignored:
_logger.warning(
"Marked %s previously known violation(s) as ignored due to"
" progressive mode.", ignored)

_render_matches(matches, options, formatter, cwd)

if matches and not mark_as_success:
return report_outcome(matches, options=options)
else:
return 0


def _render_matches(
matches: List,
options: "Namespace",
formatter: Any,
cwd: Union[str, pathlib.Path]):

ignored_matches = [match for match in matches if match.ignored]
fatal_matches = [match for match in matches if not match.ignored]
# Displayed ignored matches first
if ignored_matches:
_logger.warning(
"Listing %s violation(s) marked as ignored, likely already known",
len(ignored_matches))
for match in ignored_matches:
if match.ignored:
print(formatter.format(match, options.colored))
if fatal_matches:
_logger.warning("Listing %s violation(s) that are fatal", len(fatal_matches))
for match in fatal_matches:
if not match.ignored:
print(formatter.format(match, options.colored))

# If run under GitHub Actions we also want to emit output recognized by it.
if os.getenv('GITHUB_ACTIONS') == 'true' and os.getenv('GITHUB_WORKFLOW'):
formatter = formatters.AnnotationsFormatter(cwd, True)
for match in matches:
print(formatter.format(match))


def _get_matches(rules: RulesCollection, options: "Namespace") -> list:

if not options.playbook:
# no args triggers auto-detection mode
playbooks = get_playbooks_and_roles(options=options)
Expand All @@ -164,23 +238,26 @@ def main() -> int:
options.skip_list, options.exclude_paths,
options.verbosity, checked_files)
matches.extend(runner.run())

# Assure we do not print duplicates and the order is consistent
matches = sorted(set(matches))

for match in matches:
print(formatter.format(match, options.colored))

# If run under GitHub Actions we also want to emit output recognized by it.
if os.getenv('GITHUB_ACTIONS') == 'true' and os.getenv('GITHUB_WORKFLOW'):
formatter = formatters.AnnotationsFormatter(cwd, True)
for match in matches:
print(formatter.format(match))

if matches:
return report_outcome(matches, options=options)
else:
return 0
return matches


@contextmanager
def _previous_revision():
"""Create or update a temporary workdir containing the previous revision."""
worktree_dir = ".cache/old-rev"
revision = subprocess.run(
["git", "rev-parse", "HEAD^1"],
check=True,
universal_newlines=True,
stdout=subprocess.PIPE,
stderr=subprocess.DEVNULL,
).stdout
p = pathlib.Path(worktree_dir)
p.mkdir(parents=True, exist_ok=True)
os.system(f"git worktree add -f {worktree_dir} 2>/dev/null")
with cwd(worktree_dir):
os.system(f"git checkout {revision}")
yield


if __name__ == "__main__":
Expand Down
6 changes: 6 additions & 0 deletions lib/ansiblelint/cli.py
Expand Up @@ -111,6 +111,12 @@ def get_cli_parser() -> argparse.ArgumentParser:
default=False,
action='store_true',
help="parseable output including severity of rule")
parser.add_argument('--progressive', dest='progressive',
default=False,
action='store_true',
help="Return success if it detects a reduction in number"
" of violations compared with previous git commit. This "
"feature works only in git repositories.")
parser.add_argument('-r', action=AbspathArgAction, dest='rulesdir',
default=[], type=Path,
help="Specify custom rule directories. Add -R "
Expand Down
1 change: 1 addition & 0 deletions lib/ansiblelint/errors.py
Expand Up @@ -41,6 +41,7 @@ def __init__(
self.details = details
self.filename = normpath(filename) if filename else None
self.rule = rule
self.ignored = False # If set it will be displayed but not counted as failure

def __repr__(self):
"""Return a MatchError instance representation."""
Expand Down
12 changes: 12 additions & 0 deletions lib/ansiblelint/file_utils.py
@@ -1,5 +1,6 @@
"""Utility functions related to file operations."""
import os
from contextlib import contextmanager


def normpath(path) -> str:
Expand All @@ -11,3 +12,14 @@ def normpath(path) -> str:
"""
# convertion to string in order to allow receiving non string objects
return os.path.relpath(str(path))


@contextmanager
def cwd(path):
"""Context manager for temporary changing current working directory."""
old_pwd = os.getcwd()
os.chdir(path)
try:
yield
finally:
os.chdir(old_pwd)

0 comments on commit 1eb9e1c

Please sign in to comment.