fix: Do not add the skipped tags to the graph

Makefile

@@ -23,7 +23,10 @@ test_install: build
 	@./test_install.sh $(VIRTUALENV_DIR) $(ANSIBLE_VERSION)
 
 test:
-	cd tests && pytest
+	# Ansible 2.8 CLI sets some global variables causing the tests to fail if the cli tests are run before
+	# the grapher tests. It works in Ansible 2.9. So here we explicitly set the tests order.
+	# TODO: Remove pytest arguments when we drop support for Ansible 2.8
+	cd tests && pytest test_grapher.py test_cli.py test_postprocessor.py
 
 clean:
 	@echo "Cleaning..."

README.md

@@ -40,7 +40,7 @@ $ ansible-playbook-grapher --include-role-tasks  tests/fixtures/with_roles.yml
 
 Some options are available:
 
-```shell script
+```
 $ ansible-playbook-grapher --help
 usage: ansible-playbook-grapher [-h] [-v] [-i INVENTORY]
                                 [--include-role-tasks] [-s]
@@ -82,6 +82,15 @@ optional arguments:
 
 ```
 
+## Configuration: ansible.cfg
+The content of `ansible.cfg` is loaded automatically when running the grapher according to Ansible's behavior. The 
+corresponding environment variables are also loaded. 
+ 
+The values in the config file (and their corresponding environment variables) may affect the behavior of the grapher. 
+For example `TAGS_RUN` and `TAGS_SKIP` or vault configuration.
+
+More information [here](https://docs.ansible.com/ansible/latest/reference_appendices/config.html).
+
 ## Contribution
 Contributions are welcome. Feel free to contribute by creating an issue or submitting a PR :smiley: 
 

ansibleplaybookgrapher/cli.py

@@ -1,7 +1,7 @@
 import ntpath
 import os
-
 import sys
+
 from ansible.cli import CLI
 from ansible.errors import AnsibleOptionsError
 from ansible.release import __version__ as ansible_version
@@ -28,7 +28,7 @@ def get_cli_class():
 
 class PlaybookGrapherCLI28(CLI):
     """
-    Dedicated playbook for Ansible 2.8
+    The dedicated playbook CLI for Ansible 2.8
     """
 
     def __init__(self, args, callback=None):
@@ -105,7 +105,7 @@ class PlaybookGrapherCLI28(CLI):
 
 class PlaybookGrapherCLI29(CLI):
     """
-    Dedicated playbook for Ansible 2.9 and above.
+    The dedicated playbook CLI for Ansible 2.9 and above.
     Note: Use this class as the main CLI when we drop support for ansible < 2.9
     """
 

ansibleplaybookgrapher/data/graph.css

@@ -9,10 +9,3 @@
     stroke-width: 3;
     font-weight: bolder;
 }
-
-/**
-Each element whose id ends with "not_tagged"
- */
-[id$=not_tagged] {
-    opacity: 0.3;
-}

ansibleplaybookgrapher/grapher.py

@@ -14,13 +14,11 @@ from graphviz import Digraph
 from ansibleplaybookgrapher.utils import GraphRepresentation, clean_name, PostProcessor, get_play_colors, \
     handle_include_path, has_role_parent
 
-NOT_TAGGED = "not_tagged"
-
 
 class CustomDigrah(Digraph):
     """
     Custom digraph to avoid quoting issue with node names. Nothing special here except I put some double quotes around
-    the node and edge names and overrided some methods.
+    the node and edge names and override some methods.
     """
     _head = "digraph \"%s\"{"
     _edge = "\t\"%s\" -> \"%s\"%s"
@@ -163,29 +161,27 @@ class Grapher(object):
                 self.display.v("Graphing roles...")
                 role_number = 0
                 for role in play.get_roles():
-                    # Don't insert tasks from ``import/include_role``, preventing
-                    # duplicate graphing
+                    # Don't insert tasks from ``import/include_role``, preventing duplicate graphing
                     if role.from_include:
                         continue
 
+                    # the role object doesn't inherit the tags from the play. So we add it manually.
+                    role.tags = role.tags + play.tags
+                    if not role.evaluate_tags(only_tags=self.options.tags, skip_tags=self.options.skip_tags,
+                                              all_vars=play_vars):
+                        self.display.vv("The role '{}' is skipped due to the tags.".format(role.get_name()))
+                        # Go to the next role
+                        continue
+
                     role_number += 1
                     role_name = "[role] " + clean_name(role.get_name())
 
-                    # the role object doesn't inherit the tags from the play. So we add it manually.
-                    role.tags = role.tags + play.tags
-
-                    role_not_tagged = ""
-                    if not role.evaluate_tags(only_tags=self.options.tags, skip_tags=self.options.skip_tags,
-                                              all_vars=play_vars):
-                        role_not_tagged = NOT_TAGGED
-
                     with self.graph.subgraph(name=role_name, node_attr={}) as role_subgraph:
                         current_counter = role_number + nb_pre_tasks
-                        role_id = "role_" + str(uuid.uuid4()) + role_not_tagged
+                        role_id = "role_" + str(uuid.uuid4())
+                        edge_id = "edge_" + str(uuid.uuid4())
+
                         role_subgraph.node(role_name, id=role_id)
-
-                        edge_id = "edge_" + str(uuid.uuid4()) + role_not_tagged
-
                         # edge from play to role
                         role_subgraph.edge(play_name, role_name, label=str(current_counter), color=color,
                                            fontcolor=color, id=edge_id)
@@ -308,7 +304,7 @@ class Grapher(object):
 
                     self.display.v("An 'include_role' found. Including tasks from '{}'"
                                    .format(task_or_block.args["name"]))
-                    # here we have an include_role. The class IncludeRole is a subclass of TaskInclude.
+                    # Here we have an include_role. The class IncludeRole is a subclass of TaskInclude.
                     # We do this because the management of an include_role is different.
                     # See :func:`~ansible.playbook.included_file.IncludedFile.process_include_results` from line 155
                     my_blocks, _ = task_or_block.get_block_list(play=current_play, loader=self.data_loader,
@@ -350,36 +346,38 @@ class Grapher(object):
                                                                  current_counter=loop_counter, play_vars=task_vars,
                                                                  node_name_prefix=node_name_prefix)
             else:
-                # check if this task comes from a role, and we don't want to include role's task
+                # check if this task comes from a role, and we don't want to include tasks of the role
                 if has_role_parent(task_or_block) and not self.options.include_role_tasks:
                     # skip role's task
                     self.display.vv("The task '{}' has a role as parent and include_role_tasks is false. "
                                     "It will be skipped.".format(task_or_block.get_name()))
+                    # skipping
                     continue
 
-                self._include_task(task_or_block=task_or_block, loop_counter=loop_counter + 1, play_vars=play_vars,
-                                   graph=graph, node_name_prefix=node_name_prefix, color=color,
-                                   parent_node_id=parent_node_id, parent_node_name=parent_node_name)
-
-                loop_counter += 1
+                task_included = self._include_task(task_or_block=task_or_block, loop_counter=loop_counter + 1,
+                                                   play_vars=play_vars,
+                                                   graph=graph, node_name_prefix=node_name_prefix, color=color,
+                                                   parent_node_id=parent_node_id, parent_node_name=parent_node_name)
+                if task_included:
+                    # only increment the counter if task has been successfully included.
+                    loop_counter += 1
 
         return loop_counter
 
     def _include_task(self, task_or_block, loop_counter, play_vars, graph, node_name_prefix, color, parent_node_id,
                       parent_node_name):
         """
-        Include the task in the graph
-        :return:
-        :rtype:
+        Include the task in the graph.
+        :return: True if the task has been included, false otherwise
+        :rtype: bool
         """
+
         self.display.vv("Adding the task '{}' to the graph".format(task_or_block.get_name()))
-        # check if the task should be included
-        tagged = ''
+
         if not task_or_block.evaluate_tags(only_tags=self.options.tags, skip_tags=self.options.skip_tags,
                                            all_vars=play_vars):
-            self.display.vv("The task '{}' should not be executed. It will be marked as NOT_TAGGED"
-                            .format(task_or_block.get_name()))
-            tagged = NOT_TAGGED
+            self.display.vv("The task '{}' is skipped due to the tags.".format(task_or_block.get_name()))
+            return False
 
         task_edge_label = str(loop_counter)
         if len(task_or_block.when) > 0:
@@ -389,11 +387,13 @@ class Grapher(object):
         task_name = clean_name(node_name_prefix + self.template(task_or_block.get_name(), play_vars))
         # get prefix id from node_name
         id_prefix = node_name_prefix.replace("[", "").replace("]", "").replace(" ", "_")
-        task_id = id_prefix + str(uuid.uuid4()) + tagged
-        edge_id = "edge_" + str(uuid.uuid4()) + tagged
+        task_id = id_prefix + str(uuid.uuid4())
+        edge_id = "edge_" + str(uuid.uuid4())
 
         graph.node(task_id, label=task_name, shape="octagon", id=task_id)
         graph.edge(parent_node_name, task_id, label=task_edge_label, color=color, fontcolor=color, style="bold",
                    id=edge_id)
         self.graph_representation.add_link(parent_node_id, edge_id)
         self.graph_representation.add_link(edge_id, task_id)
+
+        return True

tests/fixtures/import_tasks.yml

@@ -6,7 +6,7 @@
         msg: "Pre import"
 
     - name: Import some tasks
-      import_tasks: tasks.yml
+      import_tasks: tasks/tasks.yml
 
     - name: Post import
       debug:

tests/fixtures/include_tasks.yml

@@ -6,7 +6,7 @@
         msg: "Pre import"
 
     - name: Include some tasks
-      include_tasks: tasks2.yml
+      include_tasks: tasks/tasks2.yml
 
     # This should not fail. Some variables are only available during execution
     - name: Task specific to {{ ansible_distribution }}

tests/fixtures/tags.yml

@@ -0,0 +1,16 @@
+---
+- hosts: all
+  pre_tasks:
+    - name: Pretask 1
+      debug: msg="Pretask"
+      tags:
+        - pre_task_tag_1
+    - name: Pretask 2
+      debug: msg="Pretask"
+      tags:
+        - pre_task_tag_2
+
+  roles:
+    - role: fake_role
+      tags:
+        - role_tag

tests/fixtures/tasks.yml

@@ -1,6 +0,0 @@
----
-- name: Imported tasks one
-  command: /bin/foo
-- name: Imported tasks two
-  command: /bin/bar
-  when: ansible_distribution == "Ubuntu"

tests/fixtures/with_roles.yml

@@ -12,7 +12,7 @@
     - name: Pretask 2
       debug: msg="Pretask"
       tags:
-        - post
+        - pre_task_tags
   post_tasks:
     - name: Posttask
       debug: msg="Postask"

tests/test_grapher.py

@@ -52,7 +52,7 @@ def _common_tests(svg_path, playbook_path, plays_number=0, tasks_number=0, post_
     :type plays_number: int
     :param tasks_number: Number of tasks in the playbook
     :type tasks_number: int
-    :param post_tasks_number Number of post tasks in the playbook
+    :param post_tasks_number: Number of post tasks in the playbook
     :type post_tasks_number: int
     :return: dict[str, PyQuery]
     """
@@ -60,7 +60,7 @@ def _common_tests(svg_path, playbook_path, plays_number=0, tasks_number=0, post_
     pq = PyQuery(filename=svg_path)
     pq.remove_namespaces()
 
-    # test if the file exist. It will exist only if we write in it
+    # test if the file exist. It will exist only if we write in it.
     assert os.path.isfile(svg_path), "The svg file should exist"
     assert pq('#root_node text').text() == playbook_path
 
@@ -82,7 +82,7 @@ def _common_tests(svg_path, playbook_path, plays_number=0, tasks_number=0, post_
     assert roles_number == len(roles), "The playbook '{}' should contains {} role(s) but we found {} role(s)".format(
         playbook_path, roles_number, len(roles))
 
-    return {'tasks': tasks, 'plays': plays, 'pq': pq, 'post_tasks': post_tasks, 'pre_tasks': pre_tasks}
+    return {'tasks': tasks, 'plays': plays, 'pq': pq, 'post_tasks': post_tasks, 'pre_tasks': pre_tasks, "roles": roles}
 
 
 def test_simple_playbook(request):
@@ -119,7 +119,7 @@ def test_import_tasks(request):
     """
     svg_path, playbook_path = run_grapher("import_tasks.yml", output_filename=request.node.name)
 
-    _common_tests(svg_path=svg_path, playbook_path=playbook_path, plays_number=1, tasks_number=4)
+    _common_tests(svg_path=svg_path, playbook_path=playbook_path, plays_number=1, tasks_number=5)
 
 
 @pytest.mark.parametrize(["include_role_tasks_option", "expected_tasks_number"],
@@ -174,8 +174,7 @@ def test_nested_include_tasks(request):
 def test_import_role(request, include_role_tasks_option, expected_tasks_number):
     """
     Test import_role.yml, an example with import role.
-    Import role is special because the tasks imported from role are treated as "normal tasks" when the playbook is
-    parsed.
+    Import role is special because the tasks imported from role are treated as "normal tasks" when the playbook is parsed.
     """
     svg_path, playbook_path = run_grapher("import_role.yml", output_filename=request.node.name,
                                           additional_args=[include_role_tasks_option])
@@ -216,3 +215,22 @@ def test_relative_var_files(request):
     # check if the plays title contains the interpolated variables
     assert 'Cristiano Ronaldo' in res['tasks'][0].find('text').text, 'The title should contain player name'
     assert 'Lionel Messi' in res['tasks'][1].find('text').text, 'The title should contain player name'
+
+
+def test_tags(request):
+    """
+    Test a playbook by only graphing a specific tasks based on the given tags
+    """
+    svg_path, playbook_path = run_grapher("tags.yml", output_filename=request.node.name,
+                                          additional_args=["-t", "pre_task_tag_1"])
+    _common_tests(svg_path=svg_path, playbook_path=playbook_path, plays_number=1, pre_tasks_number=1)
+
+
+def test_skip_tags(request):
+    """
+    Test a playbook by only graphing a specific tasks based on the given tags
+    """
+    svg_path, playbook_path = run_grapher("tags.yml", output_filename=request.node.name,
+                                          additional_args=["--skip-tags", "pre_task_tag_1", "--include-role-tasks"])
+    _common_tests(svg_path=svg_path, playbook_path=playbook_path, plays_number=1, pre_tasks_number=1, roles_number=1,
+                  tasks_number=3)