Skip to content

Commit

Permalink
fix discover_lintables: strip ./ from WcMatch return value
Browse files Browse the repository at this point in the history
calling WcMatch('.', ...) causes the resulting file names to be prefixed
with `./`.
If the tests are run in a git-repository, this is not an issue, as git
is preferred of WcMatch in the code.

Therefore remove the `./` prefix from the WcMatch return value to get
the identical expected output.

fixes ansible#1836
  • Loading branch information
sebix committed Feb 1, 2022
1 parent cea0415 commit 2a900ef
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 27 deletions.
4 changes: 2 additions & 2 deletions .ansible-lint
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
# and not relative to the CWD of execution. CLI arguments passed to the --exclude
# option will be parsed relative to the CWD of execution.
exclude_paths:
- .cache/ # implicit unless exclude_paths is defined in config
- .github/
- .cache/ # implicit unless exclude_paths is defined in config
- .github/
# parseable: true
# quiet: true
# verbosity: 1
Expand Down
4 changes: 4 additions & 0 deletions examples/playbooks/ematcher-rule.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@
- name: foo
debug:
msg: A 3rd BANNED line

- name: bar
command: echo something
changed_when: false
8 changes: 7 additions & 1 deletion src/ansiblelint/_internal/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class BaseRule:
severity: str = ""
link: str = ""
has_dynamic_tags: bool = False
needs_raw_task: bool = False

def getmatches(self, file: "Lintable") -> List["MatchError"]:
"""Return all matches while ignoring exceptions."""
Expand All @@ -48,7 +49,12 @@ def matchlines(self, file: "Lintable") -> List["MatchError"]:
def matchtask(
self, task: Dict[str, Any], file: "Optional[Lintable]" = None
) -> Union[bool, str]:
"""Confirm if current rule is matching a specific task."""
"""Confirm if current rule is matching a specific task.
If ``needs_raw_task`` (a class level attribute) is ``True``, then
the original task (before normalization) will be made available under
``task["__raw_task__"]``.
"""
return False

def matchtasks(self, file: "Lintable") -> List["MatchError"]:
Expand Down
11 changes: 9 additions & 2 deletions src/ansiblelint/file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,15 +261,22 @@ def discover_lintables(options: Namespace) -> Dict[str, Any]:
if out is None:
exclude_pattern = "|".join(str(x) for x in options.exclude_paths)
_logger.info("Looking up for files, excluding %s ...", exclude_pattern)
out = set(
# remove './' prefix from output of WcMatch
out = {
strip_dotslash_prefix(fname) for fname in
WcMatch(
'.', exclude_pattern=exclude_pattern, flags=RECURSIVE, limit=256
).match()
)
}

return OrderedDict.fromkeys(sorted(out))


def strip_dotslash_prefix(fname: str) -> str:
"""Remove ./ leading from filenames"""
return fname[2:] if fname.startswith('./') else fname


def guess_project_dir(config_file: Optional[str]) -> str:
"""Return detected project dir or current working directory."""
path = None
Expand Down
3 changes: 3 additions & 0 deletions src/ansiblelint/rules/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ def matchtasks(self, file: Lintable) -> List[MatchError]:
if skipped or 'action' not in task:
continue

if self.needs_raw_task:
task["__raw_task__"] = raw_task

result = self.matchtask(task, file=file)

if not result:
Expand Down
33 changes: 19 additions & 14 deletions test/TestLintRule.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,28 @@
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.

# pylint: disable=preferred-module # FIXME: remove once migrated per GH-725
import unittest
import pytest

from ansiblelint.file_utils import Lintable

from .rules import EMatcherRule, UnsetVariableMatcherRule
from .rules import EMatcherRule, RawTaskRule


class TestRule(unittest.TestCase):
def test_rule_matching(self) -> None:
ematcher = EMatcherRule.EMatcherRule()
lintable = Lintable('examples/playbooks/ematcher-rule.yml', kind="playbook")
matches = ematcher.matchlines(lintable)
assert len(matches) == 3
@pytest.fixture
def lintable() -> Lintable:
"""Return a playbook Lintable for use in this file's tests."""
return Lintable('examples/playbooks/ematcher-rule.yml', kind="playbook")

def test_rule_postmatching(self) -> None:
rule = UnsetVariableMatcherRule.UnsetVariableMatcherRule()
lintable = Lintable('examples/playbooks/bracketsmatchtest.yml', kind="playbook")
matches = rule.matchlines(lintable)
assert len(matches) == 2

def test_rule_matching(lintable: Lintable) -> None:
"""Test rule.matchlines() on a plyabook."""
ematcher = EMatcherRule.EMatcherRule()
matches = ematcher.matchlines(lintable)
assert len(matches) == 3


def test_raw_rule_matching(lintable: Lintable) -> None:
"""Test rule.matchlines() on a plyabook."""
ematcher = RawTaskRule.RawTaskRule()
matches = ematcher.matchtasks(lintable)
assert len(matches) == 1
18 changes: 11 additions & 7 deletions test/TestRulesCollection.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,15 @@ def bracketsmatchtestfile() -> Lintable:
def test_load_collection_from_directory(test_rules_collection: RulesCollection) -> None:
"""Test that custom rules extend the default ones."""
# two detected rules plus the internal ones
assert len(test_rules_collection) == 5
assert len(test_rules_collection) == 6


def test_run_collection(
test_rules_collection: RulesCollection, ematchtestfile: Lintable
) -> None:
"""Test that default rules match pre-meditated violations."""
matches = test_rules_collection.run(ematchtestfile)
assert len(matches) == 3 # 3 occurrences of BANNED using TEST0001
assert len(matches) == 4 # 3 occurrences of BANNED using TEST0001 + 1 for TEST0003
assert matches[0].linenumber == 2


Expand All @@ -85,9 +85,9 @@ def test_skip_tags(
bracketsmatchtestfile: Lintable,
) -> None:
"""Test that tags can be skipped."""
matches = test_rules_collection.run(ematchtestfile, skip_list=['test1'])
matches = test_rules_collection.run(ematchtestfile, skip_list=['test1', 'test3'])
assert len(matches) == 0
matches = test_rules_collection.run(ematchtestfile, skip_list=['test2'])
matches = test_rules_collection.run(ematchtestfile, skip_list=['test2', 'test3'])
assert len(matches) == 3
matches = test_rules_collection.run(bracketsmatchtestfile, skip_list=['test1'])
assert len(matches) == 2
Expand All @@ -101,9 +101,13 @@ def test_skip_id(
bracketsmatchtestfile: Lintable,
) -> None:
"""Check that skipping valid IDs excludes their violations."""
matches = test_rules_collection.run(ematchtestfile, skip_list=['TEST0001'])
matches = test_rules_collection.run(
ematchtestfile, skip_list=['TEST0001', 'TEST0003']
)
assert len(matches) == 0
matches = test_rules_collection.run(ematchtestfile, skip_list=['TEST0002'])
matches = test_rules_collection.run(
ematchtestfile, skip_list=['TEST0002', 'TEST0003']
)
assert len(matches) == 3
matches = test_rules_collection.run(bracketsmatchtestfile, skip_list=['TEST0001'])
assert len(matches) == 2
Expand All @@ -116,7 +120,7 @@ def test_skip_non_existent_id(
) -> None:
"""Check that skipping invalid IDs changes nothing."""
matches = test_rules_collection.run(ematchtestfile, skip_list=['DOESNOTEXIST'])
assert len(matches) == 3
assert len(matches) == 4


def test_no_duplicate_rule_ids(test_rules_collection: RulesCollection) -> None:
Expand Down
26 changes: 26 additions & 0 deletions test/rules/RawTaskRule.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
"""Test Rule that needs_raw_task."""
from typing import Any, Dict, Optional, Union

from ansiblelint.file_utils import Lintable
from ansiblelint.rules import AnsibleLintRule


class RawTaskRule(AnsibleLintRule):
"""Test rule that inspects the raw task."""

id = 'TEST0003'
shortdesc = 'Test rule that inspects the raw task'
description = (
'This is a test rule that looks in a raw task to flag raw action params.'
)
tags = ['fake', 'dummy', 'test3']
needs_raw_task = True

def matchtask(
self, task: Dict[str, Any], file: Optional[Lintable] = None
) -> Union[bool, str]:
"""Match a task using __raw_task__ to inspect the module params type."""
raw_task = task["__raw_task__"]
module = task["action"]["__ansible_module_original__"]
found_raw_task_params = not isinstance(raw_task[module], dict)
return found_raw_task_params
2 changes: 1 addition & 1 deletion test/rules/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
"""Test rules resources."""

__all__ = ['UnsetVariableMatcherRule', 'EMatcherRule']
__all__ = ['EMatcherRule', 'RawTaskRule', 'UnsetVariableMatcherRule']

0 comments on commit 2a900ef

Please sign in to comment.