Skip to content

Commit

Permalink
Refactored no-handler rule (#1402)
Browse files Browse the repository at this point in the history
- moved tests alongside rule definition
- extend tests to include 2 more cases
- included the newer "is changed" in the list of matching strings
- made the description more verbose
- included link to handlers documentation
  • Loading branch information
ssbarnea authored Feb 22, 2021
1 parent 483005e commit 0be3f38
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 32 deletions.
4 changes: 0 additions & 4 deletions examples/playbooks/role-with-handler.yml

This file was deleted.

20 changes: 20 additions & 0 deletions examples/playbooks/rule-no-handler.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# should trigger no-handler rule 3 times
- hosts: localhost
name: foo
roles:
- {role: a-role} # 1st from within included role

tasks:

- name: execute something
command: echo 123
register: result
changed_when: true

- name: this should trigger no-handler rule # 2nd
command: echo could be done better
when: result is changed

- name: this should trigger no-handler rule # 3rd
command: echo could be done better too
when: result is changed
5 changes: 0 additions & 5 deletions examples/roles/a-role/handlers/main.yml

This file was deleted.

6 changes: 6 additions & 0 deletions examples/roles/a-role/tasks/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
# should trigger no-handler at line 3
- name: do anything
command: echo 123
when:
- something.changed
2 changes: 1 addition & 1 deletion src/ansiblelint/generate_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def rules_as_rst(rules: RulesCollection) -> str:

description = d.description
if d.link:
description += " `(more) <%s>`_" % d.link
description += " `(more) <%s>`__" % d.link

r += f"\n\n.. _{d.id}:\n\n{title}\n{'*' * len(title)}\n\n{d.shortdesc}\n\n{description}"

Expand Down
36 changes: 34 additions & 2 deletions src/ansiblelint/rules/UseHandlerRatherThanWhenChangedRule.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.

import sys
from typing import Any, Dict, Union

from ansiblelint.rules import AnsibleLintRule
Expand All @@ -28,7 +29,13 @@ def _changed_in_when(item: str) -> bool:
return False
return any(
changed in item
for changed in ['.changed', '|changed', '["changed"]', "['changed']"]
for changed in [
'.changed',
'|changed',
'["changed"]',
"['changed']",
"is changed",
]
)


Expand All @@ -37,8 +44,10 @@ class UseHandlerRatherThanWhenChangedRule(AnsibleLintRule):
shortdesc = 'Tasks that run when changed should likely be handlers'
description = (
'If a task has a ``when: result.changed`` setting, it is effectively '
'acting as a handler'
'acting as a handler. You could use notify and move that task to '
'handlers.'
)
link = "https://docs.ansible.com/ansible/latest/user_guide/playbooks_handlers.html"
severity = 'MEDIUM'
tags = ['idiom']
version_added = 'historic'
Expand All @@ -55,3 +64,26 @@ def matchtask(self, task: Dict[str, Any]) -> Union[bool, str]:
if isinstance(when, str):
return _changed_in_when(when)
return False


if 'pytest' in sys.modules:

from ansiblelint.file_utils import Lintable
from ansiblelint.rules import RulesCollection
from ansiblelint.runner import Runner

def test_rule_no_handler() -> None:
"""Verify rule."""
collection = RulesCollection()
collection.register(UseHandlerRatherThanWhenChangedRule())

lintable = Lintable('examples/playbooks/rule-no-handler.yml')
results = Runner(lintable, rules=collection).run()

assert len(results) == 3
assert results[0].filename == 'examples/playbooks/roles/a-role/tasks/main.yml'
assert results[0].linenumber == 3
assert results[1].filename == 'examples/playbooks/rule-no-handler.yml'
assert results[1].linenumber == 14
assert results[2].filename == 'examples/playbooks/rule-no-handler.yml'
assert results[2].linenumber == 18
20 changes: 0 additions & 20 deletions test/TestRoleHandlers.py

This file was deleted.

0 comments on commit 0be3f38

Please sign in to comment.