Skip to content

Commit a531b81

Browse files
ssbarneassbarnea
authored
Improve testing and code coverage (#4561)
1 parent 771d1db commit a531b81

25 files changed

+148
-118
lines changed

Diff for: ‎examples/playbooks/transform-name.transformed.yml

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
- name: Fixture testing transform name capitalize
3+
hosts: all
4+
tasks:
5+
- name: Missing capital name with notify
6+
ansible.builtin.debug:
7+
msg: "foo"
8+
notify: Missing capital name
9+
- name: Missing capital name with notify list
10+
ansible.builtin.debug:
11+
msg: "foo"
12+
notify:
13+
- Missing capital name
14+
- name: Missing capital name
15+
ansible.builtin.debug:
16+
msg: "bar"

Diff for: ‎examples/playbooks/transform-name.yml

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
- name: Fixture testing transform name capitalize
3+
hosts: all
4+
tasks:
5+
- name: missing capital name with notify
6+
ansible.builtin.debug:
7+
msg: "foo"
8+
notify: missing capital name
9+
- name: missing capital name with notify list
10+
ansible.builtin.debug:
11+
msg: "foo"
12+
notify:
13+
- missing capital name
14+
- name: missing capital name
15+
ansible.builtin.debug:
16+
msg: "bar"

Diff for: ‎pyproject.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ source = ["src", ".tox/*/site-packages"]
6363
[tool.coverage.report]
6464
exclude_also = ["pragma: no cover", "if TYPE_CHECKING:"]
6565
# Increase it just so it would pass on any single-python run
66-
fail_under = 93
66+
fail_under = 96
6767
# During development we might remove code (files) with coverage data, and we dont want to fail:
6868
ignore_errors = true
6969
omit = ["test/*"]

Diff for: ‎src/ansiblelint/__main__.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ def main(argv: list[str] | None = None) -> int:
336336
or options.nodeps
337337
)
338338

339-
if not skip_schema_update:
339+
if not skip_schema_update: # pragma: no cover
340340
# pylint: disable=import-outside-toplevel
341341
from ansiblelint.schemas.__main__ import refresh_schemas
342342

Diff for: ‎src/ansiblelint/_internal/rules.py

+2-6
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,6 @@ def matchdir(self, lintable: Lintable) -> list[MatchError]:
149149
"""Return matches for lintable folders."""
150150
return []
151151

152-
def verbose(self) -> str:
153-
"""Return a verbose representation of the rule."""
154-
return self.id + ": " + self.shortdesc + "\n " + self.description
155-
156152
def match(self, line: str) -> bool | str:
157153
"""Confirm if current rule matches the given string."""
158154
return False
@@ -161,7 +157,7 @@ def __lt__(self, other: BaseRule) -> bool:
161157
"""Enable us to sort rules by their id."""
162158
return (self._order, self.id) < (other._order, other.id)
163159

164-
def __repr__(self) -> str:
160+
def __repr__(self) -> str: # pragma: no cover
165161
"""Return a AnsibleLintRule instance representation."""
166162
return self.id + ": " + self.shortdesc
167163

@@ -179,7 +175,7 @@ def rule_config(self) -> dict[str, Any]:
179175
rule_config = {}
180176
if self.options:
181177
rule_config = self.options.rules.get(self.id, {})
182-
if not isinstance(rule_config, dict): # pragma: no branch
178+
if not isinstance(rule_config, dict): # pragma: no cover
183179
msg = f"Invalid rule config for {self.id}: {rule_config}"
184180
raise RuntimeError(msg) # noqa: TRY004
185181
return rule_config

Diff for: ‎src/ansiblelint/_mockings.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020

2121
def _make_module_stub(module_name: str, options: Options) -> None:
22-
if not options.cache_dir:
22+
if not options.cache_dir: # pragma: no cover
2323
msg = "Cache directory not set"
2424
raise RuntimeError(msg)
2525
# a.b.c is treated a collection
@@ -75,7 +75,7 @@ def _write_module_stub(
7575
def _perform_mockings(options: Options) -> None:
7676
"""Mock modules and roles."""
7777
path: Path
78-
if not options.cache_dir:
78+
if not options.cache_dir: # pragma: no cover
7979
msg = "Cache directory not set"
8080
raise RuntimeError(msg)
8181
for role_name in options.mock_roles:
@@ -105,7 +105,7 @@ def _perform_mockings(options: Options) -> None:
105105

106106
def _perform_mockings_cleanup(options: Options) -> None:
107107
"""Clean up mocked modules and roles."""
108-
if not options.cache_dir:
108+
if not options.cache_dir: # pragma: no cover
109109
msg = "Cache directory not set"
110110
raise RuntimeError(msg)
111111
for role_name in options.mock_roles:

Diff for: ‎src/ansiblelint/cli.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ def merge_config(file_config: dict[Any, Any], cli_config: Options) -> Options:
519519
setattr(cli_config, entry, default)
520520
if cli_config.write_list is None:
521521
cli_config.write_list = []
522-
elif cli_config.write_list == [WriteArgAction._default]: # noqa: SLF001
522+
elif cli_config.write_list == [WriteArgAction._default]: # noqa: SLF001 # pragma: no cover
523523
cli_config.write_list = ["all"]
524524
return cli_config
525525

Diff for: ‎src/ansiblelint/output.py

+7-7
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@
8383
"number",
8484
"path",
8585
)
86+
RE_BB_LINK_PATTERN = re.compile(
87+
r"\[link=([^\]]+)\]((?:[^\[]|\[(?!\/link\]))+)\[/link\]"
88+
)
8689

8790

8891
# Based on Ansible implementation
@@ -258,6 +261,8 @@ class Console:
258261

259262
colored: bool = True
260263
style: type[PlainStyle] = AnsiStyle
264+
# Regex to find opening tags and their content
265+
tag_pattern = re.compile(r"\[([\w\.]+)(?:=(.*?))?\]|\[/\]")
261266

262267
def __init__(self, file: TextIO | None = sys.stdout):
263268
"""Console constructor."""
@@ -298,8 +303,6 @@ def render(self, text: str) -> str:
298303
"failed": (style.failed, style.normal),
299304
"success": (style.success, style.normal),
300305
}
301-
# Regex to find opening tags and their content
302-
tag_pattern = re.compile(r"\[([\w\.]+)(?:=(.*?))?\]|\[/\]")
303306

304307
def replace_bb_links(text: str) -> str:
305308
"""Replaces BBCode-style links ([link=url]title[/link]) with HTML <a> tags.
@@ -310,17 +313,14 @@ def replace_bb_links(text: str) -> str:
310313
Returns:
311314
str: The text with BBCode links replaced by HTML <a> tags.
312315
"""
313-
# Define a safe regex pattern
314-
pattern = r"\[link=([^\]]+)\]((?:[^\[]|\[(?!\/link\]))+)\[/link\]"
315-
316316
# Replace matches with HTML <a> tags
317317

318318
def replacement(match: re.Match[str]) -> str:
319319
url = match.group(1) # The URL part from [link=url]
320320
title = match.group(2)
321321
return style.render_link(url, title)
322322

323-
result = re.sub(pattern, replacement, text)
323+
result = RE_BB_LINK_PATTERN.sub(replacement, text)
324324
return result
325325

326326
def replace_bb_tags(text: str) -> str:
@@ -331,7 +331,7 @@ def replace_bb_tags(text: str) -> str:
331331
result = [] # Result list to build the output HTML
332332
pos = 0 # Current position in the text
333333

334-
for match in tag_pattern.finditer(text):
334+
for match in self.tag_pattern.finditer(text):
335335
start, end = match.span()
336336
tag = match.group(1)
337337
param = match.group(2)

Diff for: ‎src/ansiblelint/rules/__init__.py

+7-14
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@
4646
"matchplay": "play", # called by matchyaml
4747
"matchdir": "dir",
4848
}
49+
RE_JINJA_EXPRESSION = re.compile(r"{{.+?}}")
50+
RE_JINJA_STATEMENT = re.compile(r"{%.+?%}")
51+
RE_JINJA_COMMENT = re.compile(r"{#.+?#}")
4952

5053

5154
class AnsibleLintRule(BaseRule):
@@ -63,9 +66,9 @@ def get_config(self, key: str) -> Any:
6366
@staticmethod
6467
def unjinja(text: str) -> str:
6568
"""Remove jinja2 bits from a string."""
66-
text = re.sub(r"{{.+?}}", "JINJA_EXPRESSION", text)
67-
text = re.sub(r"{%.+?%}", "JINJA_STATEMENT", text)
68-
text = re.sub(r"{#.+?#}", "JINJA_COMMENT", text)
69+
text = RE_JINJA_EXPRESSION.sub("JINJA_EXPRESSION", text)
70+
text = RE_JINJA_STATEMENT.sub("JINJA_STATEMENT", text)
71+
text = RE_JINJA_COMMENT.sub("JINJA_COMMENT", text)
6972
return text
7073

7174
# pylint: disable=too-many-arguments,too-many-positional-arguments
@@ -467,10 +470,6 @@ def __getitem__(self, item: Any) -> BaseRule:
467470
msg = f"Rule {item} is not present inside this collection."
468471
raise ValueError(msg)
469472

470-
def extend(self, more: list[AnsibleLintRule]) -> None:
471-
"""Combine rules."""
472-
self.rules.extend(more)
473-
474473
def run(
475474
self,
476475
file: Lintable,
@@ -525,12 +524,6 @@ def run(
525524

526525
return matches
527526

528-
def __repr__(self) -> str:
529-
"""Return a RulesCollection instance representation."""
530-
return "\n".join(
531-
[rule.verbose() for rule in sorted(self.rules, key=lambda x: x.id)],
532-
)
533-
534527
def known_tags(self) -> list[str]:
535528
"""Return a list of known tags, without returning no sub-tags."""
536529
tags = set()
@@ -561,7 +554,7 @@ def list_tags(self) -> str:
561554
tags = defaultdict(list)
562555
for rule in self.rules:
563556
# Fail early if a rule does not have any of our required tags
564-
if not set(rule.tags).intersection(tag_desc.keys()):
557+
if not set(rule.tags).intersection(tag_desc.keys()): # pragma: no cover
565558
msg = f"Rule {rule} does not have any of the required tags: {', '.join(tag_desc.keys())}"
566559
raise RuntimeError(msg)
567560
for tag in rule.tags:

Diff for: ‎src/ansiblelint/rules/args.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ class ArgsRule(AnsibleLintRule):
9292
_ids = {
9393
"args[module]": description,
9494
}
95+
RE_PATTERN = re.compile(r"(argument|option) '(?P<name>.*)' is of type")
96+
RE_VALUE_OF = re.compile(r"value of (?P<name>.*) must be one of:")
9597

9698
def matchtask(
9799
self,
@@ -232,8 +234,7 @@ def _parse_failed_msg(
232234
except json.decoder.JSONDecodeError: # pragma: no cover
233235
error_message = failed_msg
234236

235-
option_type_check_error = re.search(
236-
r"(argument|option) '(?P<name>.*)' is of type",
237+
option_type_check_error = self.RE_PATTERN.search(
237238
error_message,
238239
)
239240
if option_type_check_error:
@@ -249,8 +250,7 @@ def _parse_failed_msg(
249250
)
250251
return results
251252

252-
value_not_in_choices_error = re.search(
253-
r"value of (?P<name>.*) must be one of:",
253+
value_not_in_choices_error = self.RE_VALUE_OF.search(
254254
error_message,
255255
)
256256
if value_not_in_choices_error:

Diff for: ‎src/ansiblelint/rules/name.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import re
66
import sys
7+
from collections.abc import MutableMapping
78
from typing import TYPE_CHECKING, Any
89

910
import wcmatch.pathlib
@@ -234,7 +235,7 @@ def update_task_name(task_name: str) -> str:
234235
if orig_task_name:
235236
updated_task_name = update_task_name(orig_task_name)
236237
for item in data:
237-
if isinstance(item, dict) and "tasks" in item:
238+
if isinstance(item, MutableMapping) and "tasks" in item:
238239
for task in item["tasks"]:
239240
# We want to rewrite task names in the notify keyword, but
240241
# if there isn't a notify section, there's nothing to do.

Diff for: ‎src/ansiblelint/rules/no_jinja_when.py

+12-8
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import re
66
import sys
7+
from collections.abc import MutableMapping
78
from typing import TYPE_CHECKING, Any
89

910
from ansiblelint.constants import LINE_NUMBER_KEY
@@ -16,6 +17,8 @@
1617
from ansiblelint.file_utils import Lintable
1718
from ansiblelint.utils import Task
1819

20+
RE_JINJA = re.compile(r"{{ (.*?) }}")
21+
1922

2023
class NoFormattingInWhenRule(AnsibleLintRule, TransformMixin):
2124
"""No Jinja2 in when."""
@@ -80,12 +83,13 @@ def transform(
8083
task = self.seek(match.yaml_path, data)
8184
key_to_check = ("when", "changed_when", "failed_when")
8285
for _ in range(len(task)):
83-
k, v = task.popitem(False)
84-
if k == "roles" and isinstance(v, list):
85-
transform_for_roles(v, key_to_check=key_to_check)
86-
elif k in key_to_check:
87-
v = re.sub(r"{{ (.*?) }}", r"\1", v)
88-
task[k] = v
86+
if isinstance(task, MutableMapping):
87+
for k, v in task.items():
88+
if k == "roles" and isinstance(v, list):
89+
transform_for_roles(v, key_to_check=key_to_check)
90+
elif k in key_to_check:
91+
v = RE_JINJA.sub(r"\1", v)
92+
task[k] = v
8993
match.fixed = True
9094

9195

@@ -96,10 +100,10 @@ def transform_for_roles(v: list[Any], key_to_check: tuple[str, ...]) -> None:
96100
if new_key in key_to_check:
97101
if isinstance(new_value, list):
98102
for index, nested_value in enumerate(new_value):
99-
new_value[index] = re.sub(r"{{ (.*?) }}", r"\1", nested_value)
103+
new_value[index] = RE_JINJA.sub(r"\1", nested_value)
100104
v[idx][new_key] = new_value
101105
if isinstance(new_value, str):
102-
v[idx][new_key] = re.sub(r"{{ (.*?) }}", r"\1", new_value)
106+
v[idx][new_key] = RE_JINJA.sub(r"\1", new_value)
103107

104108

105109
if "pytest" in sys.modules:

Diff for: ‎src/ansiblelint/rules/no_same_owner.py

+3-4
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ class NoSameOwnerRule(AnsibleLintRule):
2727
severity = "LOW"
2828
tags = ["opt-in"]
2929
version_changed = "6.4.0"
30+
RE_ARCHIVES = re.compile(r"^.*\.tar(\.(gz|bz2|xz))?$")
3031

3132
def matchtask(
3233
self,
@@ -59,8 +60,7 @@ def handle_synchronize(task: Any, action: dict[str, Any]) -> bool:
5960
return True
6061
return False
6162

62-
@staticmethod
63-
def handle_unarchive(task: Any, action: dict[str, Any]) -> bool:
63+
def handle_unarchive(self, task: Any, action: dict[str, Any]) -> bool:
6464
"""Process unarchive task."""
6565
delegate_to = task.get("delegate_to")
6666
if delegate_to == "localhost" or (
@@ -72,8 +72,7 @@ def handle_unarchive(task: Any, action: dict[str, Any]) -> bool:
7272

7373
if src.endswith("zip") and "-X" in action.get("extra_opts", []):
7474
return True
75-
if re.search(
76-
r".*\.tar(\.(gz|bz2|xz))?$",
75+
if self.RE_ARCHIVES.match(
7776
src,
7877
) and "--no-same-owner" not in action.get("extra_opts", []):
7978
return True

Diff for: ‎src/ansiblelint/rules/schema.py

+4-3
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,12 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
112112

113113
def _get_field_matches(
114114
self,
115-
file: Lintable,
115+
file: Lintable | None,
116116
data: MutableMapping[str, Any],
117117
) -> list[MatchError]:
118118
"""Retrieve all matches related to fields for the given data block."""
119119
results = []
120+
kind = "tasks" if not file else file.kind
120121
for key, values in self.field_checks.items():
121122
if key in data:
122123
plugin_value = data[key]
@@ -128,7 +129,7 @@ def _get_field_matches(
128129
lineno=data.get(LINE_NUMBER_KEY, 1),
129130
filename=file,
130131
details=ValidateSchemaRule.description,
131-
tag=f"schema[{file.kind}]",
132+
tag=f"schema[{kind}]",
132133
),
133134
)
134135
return results
@@ -142,7 +143,7 @@ def matchtask(
142143
if not file:
143144
file = Lintable("", kind="tasks")
144145

145-
if file.failed():
146+
if file and file.failed():
146147
return results
147148

148149
results.extend(self._get_field_matches(file=file, data=task.raw_task))

0 commit comments

Comments
 (0)