Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add progressive mode #1065

Merged
merged 1 commit into from Oct 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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)