Skip to content

Commit

Permalink
Changes for new sanity-ignore-file rule
Browse files Browse the repository at this point in the history
  • Loading branch information
alisonlhart committed Mar 1, 2023
1 parent c5042f1 commit da1828b
Show file tree
Hide file tree
Showing 14 changed files with 195 additions and 4 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ jobs:
WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:GITHUB_STEP_SUMMARY
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 801
PYTEST_REQPASS: 804

steps:
- name: Activate WSL1
if: "contains(matrix.shell, 'wsl')"
Expand Down
2 changes: 1 addition & 1 deletion docs/profiles.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ in
as validated or certified content. It extends [shared](#shared) profile.

- [avoid-dot-notation](https://github.com/ansible/ansible-lint/issues/2174)
- [disallowed-ignore](https://github.com/ansible/ansible-lint/issues/2121)
- [sanity-ignore-file](https://github.com/ansible/ansible-lint/issues/2121)
- [fqcn](rules/fqcn.md)
- [import-task-no-when](https://github.com/ansible/ansible-lint/issues/2219)
- [meta-no-dependencies](https://github.com/ansible/ansible-lint/issues/2159)
Expand Down
1 change: 1 addition & 0 deletions docs/rules/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
- [risky-shell-pipe][]
- [role-name][]
- [run-once][]
- [sanity-ignore-file][]
- [schema][]
- [syntax-check][]
- [var-naming][]
Expand Down
1 change: 1 addition & 0 deletions docs/rules/sanity_ignore_file.md
1 change: 1 addition & 0 deletions examples/sanity_ignores/tests/sanity/ignore-2.13.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
plugins/module_utils/ansible_example_module.py import-3.6!skip # comment
2 changes: 2 additions & 0 deletions examples/sanity_ignores/tests/sanity/ignore-2.14.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
plugins/module_utils/ansible_example_module.py compile-2.6!skip
plugins/module_utils/ansible_example_module.py import-2.6!skip
2 changes: 2 additions & 0 deletions examples/sanity_ignores/tests/sanity/ignore-2.15.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
plugins/module_utils/ansible_example_module.incorrect-3.6!skip
#plugins/module_utils/ansible_example_module.py import-3.6!skip
1 change: 1 addition & 0 deletions src/ansiblelint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
{"changelog": "**/changelogs/changelog.yaml"},
{"yaml": "**/*.{yaml,yml}"},
{"yaml": "**/.*.{yaml,yml}"},
{"sanity-ignore-file": "**/tests/sanity/ignore-*.txt"},
]

BASE_KINDS = [
Expand Down
1 change: 1 addition & 0 deletions src/ansiblelint/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def main():
"yaml", # generic yaml file, previously reported as unknown file type
"ansible-lint-config",
"", # unknown file type
"sanity-ignore-file", # tests/sanity/ignore file
]


Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/data/profiles.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ production:
rules:
avoid-dot-notation:
url: https://github.com/ansible/ansible-lint/issues/2174
disallowed-ignore: # [sanity]
sanity-ignore-file: # [sanity]
url: https://github.com/ansible/ansible-lint/issues/2121
fqcn:
import-task-no-when:
Expand Down
45 changes: 45 additions & 0 deletions src/ansiblelint/rules/sanity_ignore_file.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# sanity-ignore-file

This rule checks the `tests/sanity/ignore-x.x.txt` file for disallowed ignores. This rule is extremely opinionated and enforced by Partner Engineering.
The current allowed ruleset is subject to change, but is starting at a minimal number of allowed ignores for maximum test enforcement. Commented-out ignore entries are not evaluated.

This rule can produce messages such:

- `sanity-ignore-file[cannot-ignore]` - Ignore file contains {test} at line {line_num}, which is not a permitted ignore.
- `sanity-ignore-file[bad-format]` - Ignore file entry at {line_num} is formatted incorrectly. Please review.

Currently allowed ignores are:

- `validate-modules:missing-gplv3-license`
- `import-2.6`
- `import-2.6!skip`
- `import-2.7`
- `import-2.7!skip`
- `import-3.5`
- `import-3.5!skip`
- `compile-2.6`
- `compile-2.6!skip`
- `compile-2.7`
- `compile-2.7!skip`
- `compile-3.5`
- `compile-3.5!skip`


## Problematic code

```
# tests/sanity/ignore-x.x.txt
plugins/module_utils/ansible_example_module.py import-3.6!skip
```

```
# tests/sanity/ignore-x.x.txt
plugins/module_utils/ansible_example_module.oops-3.6!skip
```

## Correct code

```
# tests/sanity/ignore-x.x.txt
plugins/module_utils/ansible_example_module.py import-2.7!skip
```
131 changes: 131 additions & 0 deletions src/ansiblelint/rules/sanity_ignore_file.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
"""Implementation of sanity-ignore-file rule."""
from __future__ import annotations

import sys
from typing import TYPE_CHECKING

from ansiblelint.rules import AnsibleLintRule

# Copyright (c) 2018, Ansible Project


if TYPE_CHECKING:
from typing import Any

from ansiblelint.errors import MatchError
from ansiblelint.file_utils import Lintable


class CheckSanityIgnoreFiles(AnsibleLintRule):
"""Ignore entries in sanity ignore files must match an allow list."""

id = "sanity-ignore-file"
description = "Throws a warning if there are non-allowed entries in the `tests/sanity/ignore*.txt files."
severity = "MEDIUM"
tags = ["experimental"]
version_added = "v6.12.3"

# Partner Engineering defines this list. Please contact PE for changes.
allowed_ignores = [
"validate-modules:missing-gplv3-license",
"import-2.6",
"import-2.6!skip",
"import-2.7",
"import-2.7!skip",
"import-3.5",
"import-3.5!skip",
"compile-2.6",
"compile-2.6!skip",
"compile-2.7",
"compile-2.7!skip",
"compile-3.5",
"compile-3.5!skip",
]

def matchyaml(self, file: Lintable) -> list[MatchError]:
"""Evaluate sanity ignore lists for disallowed ignores.
:param file: Input lintable file that is a match for `sanity-ignore-file`
:returns: List of errors matched to the input file
"""
results: list[MatchError] = []
test = ""

if file.kind != "sanity-ignore-file":
return []

with open(file.abspath, encoding="utf-8") as ignore_file:
entries = ignore_file.read().splitlines()

for line_num, entry in enumerate(entries, 1):
if entry and entry[0] != "#":
try:
if "#" in entry:
entry, _ = entry.split("#")
(_, test) = entry.split()
if test not in self.allowed_ignores:
results.append(
self.create_matcherror(
message=f"Ignore file contains {test} at line {line_num}, which is not a permitted ignore.",
tag="sanity-ignore-file[cannot-ignore]",
linenumber=line_num,
filename=file,
)
)

except ValueError:
results.append(
self.create_matcherror(
message=f"Ignore file entry at {line_num} is formatted incorrectly. Please review.",
tag="sanity-ignore-file[bad-format]",
linenumber=line_num,
filename=file,
)
)

return results


# testing code to be loaded only with pytest or when executed the rule file
if "pytest" in sys.modules:
import pytest

from ansiblelint.rules import RulesCollection # pylint: disable=ungrouped-imports
from ansiblelint.runner import Runner # pylint: disable=ungrouped-imports

@pytest.mark.parametrize(
("test_file", "failures", "tags"),
(
pytest.param(
"examples/sanity_ignores/tests/sanity/ignore-2.14.txt",
0,
"sanity-ignore-file[cannot-ignore]",
id="pass",
),
pytest.param(
"examples/sanity_ignores/tests/sanity/ignore-2.15.txt",
1,
"sanity-ignore-file[bad-format]",
id="fail0",
),
pytest.param(
"examples/sanity_ignores/tests/sanity/ignore-2.13.txt",
1,
"sanity-ignore-file[cannot-ignore]",
id="fail1",
),
),
)
def test_sanity_ignore_files(
default_rules_collection: RulesCollection,
test_file: str,
failures: int,
tags: str,
) -> None:
"""Test rule matches."""
default_rules_collection.register(CheckSanityIgnoreFiles())
results = Runner(test_file, rules=default_rules_collection).run()
for result in results:
assert result.rule.id == CheckSanityIgnoreFiles().id
assert result.tag == tags
assert len(results) == failures
5 changes: 5 additions & 0 deletions test/test_file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,11 @@ def test_discover_lintables_umlaut(monkeypatch: MonkeyPatch) -> None:
pytest.param(
"examples/playbooks/vars/subfolder/settings.yaml", "vars", id="40"
),
pytest.param(
"examples/sanity_ignores/tests/sanity/ignore-2.14.txt",
"sanity-ignore-file",
id="41",
),
),
)
def test_kinds(monkeypatch: MonkeyPatch, path: str, kind: FileType) -> None:
Expand Down
2 changes: 1 addition & 1 deletion test/test_rules_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,5 +166,5 @@ def test_rules_id_format() -> None:
rule.help != "" or rule.description or rule.__doc__
), f"Rule {rule.id} must have at least one of: .help, .description, .__doc__"
assert "yaml" in keys, "yaml rule is missing"
assert len(rules) == 50 # update this number when adding new rules!
assert len(rules) == 51 # update this number when adding new rules!
assert len(keys) == len(rules), "Duplicate rule ids?"

0 comments on commit da1828b

Please sign in to comment.