added tests

refactored task selection into method
list-tasks now uses common method of task selection
always is now skippable if specified in --skip-tags

4 files changed, 81 insertions, 67 deletions

diff --git a/bin/ansible-playbook b/bin/ansible-playbook
index a3b2958179..2a692a2e3d 100755
--- a/bin/ansible-playbook
+++ b/bin/ansible-playbook
@@ -227,22 +227,6 @@ def main(args):
                 label = play.name
                 hosts = pb.inventory.list_hosts(play.hosts)
 
-                # Filter all tasks by given tags
-                if pb.only_tags != 'all':
-                    if options.subset and not hosts:
-                        continue
-                    matched_tags, unmatched_tags = play.compare_tags(pb.only_tags)
-
-                    # Remove skipped tasks
-                    matched_tags = matched_tags - set(pb.skip_tags)
-
-                    unmatched_tags.discard('all')
-                    unknown_tags = ((set(pb.only_tags) | set(pb.skip_tags)) -
-                                    (matched_tags | unmatched_tags))
-
-                    if unknown_tags:
-                        continue
-
                 if options.listhosts:
                     print '  play #%d (%s): host count=%d' % (playnum, label, len(hosts))
                     for host in hosts:
@@ -251,12 +235,10 @@ def main(args):
                 if options.listtasks:
                     print '  play #%d (%s):' % (playnum, label)
 
-                    for task in play.tasks():
-                        if (set(task.tags).intersection(pb.only_tags) and not
-                            set(task.tags).intersection(pb.skip_tags)):
-                            if getattr(task, 'name', None) is not None:
-                                # meta tasks have no names
-                                print '    %s' % task.name
+                    for task in pb.tasks_to_run_in_play(play):
+                        if getattr(task, 'name', None) is not None:
+                            # meta tasks have no names
+                            print '    %s' % task.name
                 if options.listhosts or options.listtasks:
                     print ''
             continue
diff --git a/lib/ansible/playbook/__init__.py b/lib/ansible/playbook/__init__.py
index a8ec9b8a45..b4030565b5 100644
--- a/lib/ansible/playbook/__init__.py
+++ b/lib/ansible/playbook/__init__.py
@@ -672,7 +672,53 @@ class PlayBook(object):
         return filename
 
     # *****************************************************
+    def tasks_to_run_in_play(self, play):
 
+        tasks = []
+
+        for task in play.tasks():
+            # only run the task if the requested tags match or has 'always' tag
+            u = set(['untagged'])
+            task_set = set(task.tags)
+
+            if 'always' in task.tags:
+                should_run = True
+            else:
+                if 'all' in self.only_tags:
+                    should_run = True
+                else:
+                    should_run = False
+                    if  'tagged' in self.only_tags:
+                        if task_set != u:
+                            should_run = True
+                    elif 'untagged' in self.only_tags:
+                        if task_set == u:
+                            should_run = True
+                    else:
+                        if task_set.intersection(self.only_tags):
+                            should_run = True
+
+            # Check for tags that we need to skip
+            if 'all' in self.skip_tags:
+                should_run = False
+            else:
+                if 'tagged' in self.skip_tags:
+                    if task_set != u:
+                        should_run = False
+                elif 'untagged' in self.skip_tags:
+                    if task_set == u:
+                        should_run = False
+                else:
+                    if should_run:
+                        if task_set.intersection(self.skip_tags):
+                            should_run = False
+
+            if should_run:
+                tasks.append(task)
+
+        return tasks
+
+    # *****************************************************
     def _run_play(self, play):
         ''' run a list of tasks for a given pattern, in order '''
 
@@ -725,7 +771,7 @@ class PlayBook(object):
             play._play_hosts = self._trim_unavailable_hosts(on_hosts)
             self.inventory.also_restrict_to(on_hosts)
 
-            for task in play.tasks():
+            for task in self.tasks_to_run_in_play(play):
 
                 if task.meta is not None:
                     # meta tasks can force handlers to run mid-play
@@ -735,49 +781,11 @@ class PlayBook(object):
                     # skip calling the handler till the play is finished
                     continue
 
-                # only run the task if the requested tags match or has 'always' tag
-                if 'always' in task.tags:
-                    should_run = True
-                else:
-                    u = set(['untagged'])
-                    task_set = set(task.tags)
-
-                    if 'all' in self.only_tags:
-                        should_run = True
-                    else:
-                        should_run = False
-                        if  'tagged' in self.only_tags:
-                            if task_set != u:
-                                should_run = True
-                        elif 'untagged' in self.only_tags:
-                            if task_set == u:
-                                should_run = True
-                        else:
-                            if task_set.intersection(self.only_tags):
-                                should_run = True
-
-                    # Check for tags that we need to skip
-                    if 'all' in self.skip_tags:
-                        should_run = False
-                    else:
-                        if 'tagged' in self.skip_tags:
-                            if task_set != u:
-                                should_run = False
-                        elif 'untagged' in self.skip_tags:
-                            if task_set == u:
-                                should_run = False
-                        else:
-                            if should_run:
-                                if task_set.intersection(self.skip_tags):
-                                    should_run = False
-
-                if should_run:
-
-                    if not self._run_task(play, task, False):
-                        # whether no hosts matched is fatal or not depends if it was on the initial step.
-                        # if we got exactly no hosts on the first step (setup!) then the host group
-                        # just didn't match anything and that's ok
-                        return False
+                if not self._run_task(play, task, False):
+                    # whether no hosts matched is fatal or not depends if it was on the initial step.
+                    # if we got exactly no hosts on the first step (setup!) then the host group
+                    # just didn't match anything and that's ok
+                    return False
 
                 # Get a new list of what hosts are left as available, the ones that
                 # did not go fail/dark during the task
diff --git a/test/integration/Makefile b/test/integration/Makefile
index c708e08e40..c533e06b15 100644
--- a/test/integration/Makefile
+++ b/test/integration/Makefile
@@ -21,7 +21,7 @@ VAULT_PASSWORD_FILE = vault-password
 
 CONSUL_RUNNING := $(shell python consul_running.py)
 
-all: parsing test_var_precedence unicode non_destructive destructive includes check_mode test_hash test_handlers test_group_by test_vault
+all: parsing test_var_precedence unicode non_destructive destructive includes check_mode test_hash test_handlers test_group_by test_vault test_tags
 
 parsing:
 	ansible-playbook bad_parsing.yml -i $(INVENTORY) -e @$(VARS_FILE) $(CREDENTIALS_ARG) -vvv $(TEST_FLAGS) --tags prepare,common,scenario1; [ $$? -eq 3 ]
@@ -82,6 +82,15 @@ test_delegate_to:
 test_winrm:
 	ansible-playbook test_winrm.yml -i inventory.winrm -e @$(VARS_FILE) $(CREDENTIALS_ARG) -v $(TEST_FLAGS)
 
+test_tags:
+	# Run everything by default
+	[ "$$(ansible-playbook --list-tasks test_tags.yml -i $(INVENTORY) -e @$(VARS_FILE) $(CREDENTIALS_ARG) -v $(TEST_FLAGS) | fgrep Task_with | xargs)" = "Task_with_tag Task_with_always_tag Task_without_tag" ]
+	# Run the exact tags, and always
+	[ "$$(ansible-playbook --list-tasks --tags tag test_tags.yml -i $(INVENTORY) -e @$(VARS_FILE) $(CREDENTIALS_ARG) -v $(TEST_FLAGS) | fgrep Task_with | xargs)" = "Task_with_tag Task_with_always_tag" ]
+	# Skip one tag
+	[ "$$(ansible-playbook --list-tasks --skip-tags tag test_tags.yml -i $(INVENTORY) -e @$(VARS_FILE) $(CREDENTIALS_ARG) -v $(TEST_FLAGS) | fgrep Task_with | xargs)" = "Task_with_always_tag Task_without_tag" ]
+
+
 cloud: amazon rackspace
 
 cloud_cleanup: amazon_cleanup rackspace_cleanup
diff --git a/test/integration/test_tags.yml b/test/integration/test_tags.yml
new file mode 100644
index 0000000000..650c4725ae
--- /dev/null
+++ b/test/integration/test_tags.yml
@@ -0,0 +1,15 @@
+---
+- name: verify tags work as expected
+  hosts: localhost
+  gather_facts: False
+  connection: local
+  tasks:
+    - name: Task_with_tag
+      debug: msg=
+      tags: tag
+    - name: Task_with_always_tag
+      debug: msg=
+      tags: always
+    - name: Task_without_tag
+      debug: msg=
+