[playbook/play.py] Increase error test coverage (#74217)

Change:
- Mostly increase error coverage for various conditions in play.py
- Also fix a string in an error, where get_name() was called before
  self.name was read in, so get_name() was always ''.

Test Plan:
- new tests

Signed-off-by: Rick Elrod <rick@elrod.me>

* Fix regex for py2 and py3

Signed-off-by: Rick Elrod <rick@elrod.me>

* py2 hates me

Signed-off-by: Rick Elrod <rick@elrod.me>

lib/ansible/playbook/play.py

@@ -130,8 +130,8 @@ class Play(Base, Taggable, CollectionSearch):
             # this should never happen, but error out with a helpful message
             # to the user if it does...
             if 'remote_user' in ds:
-                raise AnsibleParserError("both 'user' and 'remote_user' are set for %s. "
+                raise AnsibleParserError("both 'user' and 'remote_user' are set for this play. "
-                                         "The use of 'user' is deprecated, and should be removed" % self.get_name(), obj=ds)
+                                         "The use of 'user' is deprecated, and should be removed", obj=ds)
 
             ds['remote_user'] = ds['user']
             del ds['user']

test/integration/targets/playbook/empty_hosts.yml

@@ -0,0 +1,4 @@
+- hosts: []
+  tasks:
+    - debug:
+        msg: does not run

test/integration/targets/playbook/malformed_post_tasks.yml

@@ -0,0 +1,2 @@
+- hosts: localhost
+  post_tasks: 123

test/integration/targets/playbook/malformed_pre_tasks.yml

@@ -0,0 +1,2 @@
+- hosts: localhost
+  pre_tasks: 123

test/integration/targets/playbook/malformed_roles.yml

@@ -0,0 +1,2 @@
+- hosts: localhost
+  roles: 123

test/integration/targets/playbook/malformed_tasks.yml

@@ -0,0 +1,2 @@
+- hosts: localhost
+  tasks: 123

test/integration/targets/playbook/malformed_vars_prompt.yml

@@ -0,0 +1,3 @@
+- hosts: localhost
+  vars_prompt:
+    - foo: bar

test/integration/targets/playbook/old_style_role.yml

@@ -0,0 +1,3 @@
+- hosts: localhost
+  roles:
+    - foo,bar

test/integration/targets/playbook/remote_user_and_user.yml

@@ -0,0 +1,6 @@
+- hosts: localhost
+  remote_user: a
+  user: b
+  tasks:
+    - debug:
+        msg: did not run

test/integration/targets/playbook/roles_null.yml

@@ -0,0 +1,3 @@
+- name: null roles is okay
+  hosts: localhost
+  roles: null

test/integration/targets/playbook/runme.sh

@@ -8,8 +8,85 @@ ansible-playbook -i ../../inventory types.yml -v "$@"
 # test timeout
 ansible-playbook -i ../../inventory timeout.yml -v "$@"
 
+# our Play class allows for 'user' or 'remote_user', but not both.
+# first test that both user and remote_user work individually
+set +e
+result="$(ansible-playbook -i ../../inventory user.yml -v "$@" 2>&1)"
+set -e
+grep -q "worked with user" <<< "$result"
+grep -q "worked with remote_user" <<< "$result"
+
+# then test that the play errors if user and remote_user both exist
+echo "EXPECTED ERROR: Ensure we fail properly if a play has both user and remote_user."
+set +e
+result="$(ansible-playbook -i ../../inventory remote_user_and_user.yml -v "$@" 2>&1)"
+set -e
+grep -q "ERROR! both 'user' and 'remote_user' are set for this play." <<< "$result"
+
+# test that playbook errors if len(plays) == 0
 echo "EXPECTED ERROR: Ensure we fail properly if a playbook is an empty list."
 set +e
 result="$(ansible-playbook -i ../../inventory empty.yml -v "$@" 2>&1)"
 set -e
 grep -q "ERROR! A playbook must contain at least one play" <<< "$result"
+
+# test that play errors if len(hosts) == 0
+echo "EXPECTED ERROR: Ensure we fail properly if a play has 0 hosts."
+set +e
+result="$(ansible-playbook -i ../../inventory empty_hosts.yml -v "$@" 2>&1)"
+set -e
+grep -q "ERROR! Hosts list cannot be empty - please check your playbook" <<< "$result"
+
+# test that play errors if tasks is malformed
+echo "EXPECTED ERROR: Ensure we fail properly if tasks is malformed."
+set +e
+result="$(ansible-playbook -i ../../inventory malformed_tasks.yml -v "$@" 2>&1)"
+set -e
+grep -q "ERROR! A malformed block was encountered while loading tasks: 123 should be a list or None" <<< "$result"
+
+# test that play errors if pre_tasks is malformed
+echo "EXPECTED ERROR: Ensure we fail properly if pre_tasks is malformed."
+set +e
+result="$(ansible-playbook -i ../../inventory malformed_pre_tasks.yml -v "$@" 2>&1)"
+set -e
+grep -q "ERROR! A malformed block was encountered while loading pre_tasks" <<< "$result"
+
+# test that play errors if post_tasks is malformed
+echo "EXPECTED ERROR: Ensure we fail properly if post_tasks is malformed."
+set +e
+result="$(ansible-playbook -i ../../inventory malformed_post_tasks.yml -v "$@" 2>&1)"
+set -e
+grep -q "ERROR! A malformed block was encountered while loading post_tasks" <<< "$result"
+
+# test roles: null -- it gets converted to [] internally
+ansible-playbook -i ../../inventory roles_null.yml -v "$@"
+
+# test roles: 123 -- errors
+echo "EXPECTED ERROR: Ensure we fail properly if roles is malformed."
+set +e
+result="$(ansible-playbook -i ../../inventory malformed_roles.yml -v "$@" 2>&1)"
+set -e
+grep -q "ERROR! A malformed role declaration was encountered." <<< "$result"
+
+# test roles: ["foo,bar"] -- errors about old style
+echo "EXPECTED ERROR: Ensure we fail properly if old style role is given."
+set +e
+result="$(ansible-playbook -i ../../inventory old_style_role.yml -v "$@" 2>&1)"
+set -e
+grep -q "ERROR! Invalid old style role requirement: foo,bar" <<< "$result"
+
+# test vars prompt that has no name
+echo "EXPECTED ERROR: Ensure we fail properly if vars_prompt has no name."
+set +e
+result="$(ansible-playbook -i ../../inventory malformed_vars_prompt.yml -v "$@" 2>&1)"
+set -e
+grep -q "ERROR! Invalid vars_prompt data structure, missing 'name' key" <<< "$result"
+
+# test vars_prompt: null
+ansible-playbook -i ../../inventory vars_prompt_null.yml -v "$@"
+
+# test vars_files: null
+ansible-playbook -i ../../inventory vars_files_null.yml -v "$@"
+
+# test vars_files: filename.yml
+ansible-playbook -i ../../inventory vars_files_string.yml -v "$@"

test/integration/targets/playbook/some_vars.yml

@@ -0,0 +1,2 @@
+a_variable: yep
+another: hi

test/integration/targets/playbook/user.yml

@@ -0,0 +1,23 @@
+- hosts: localhost
+  tasks:
+    - command: whoami
+      register: whoami
+
+    - assert:
+        that:
+          - whoami is successful
+
+    - set_fact:
+        me: "{{ whoami.stdout }}"
+
+- hosts: localhost
+  user: "{{ me }}"
+  tasks:
+    - debug:
+        msg: worked with user ({{ me }})
+
+- hosts: localhost
+  remote_user: "{{ me }}"
+  tasks:
+    - debug:
+        msg: worked with remote_user ({{ me }})

test/integration/targets/playbook/vars_files_null.yml

@@ -0,0 +1,3 @@
+- name: null vars_files is okay
+  hosts: localhost
+  vars_files: null

test/integration/targets/playbook/vars_files_string.yml

@@ -0,0 +1,6 @@
+- hosts: localhost
+  vars_files: some_vars.yml
+  tasks:
+    - assert:
+        that:
+          - 'a_variable == "yep"'

test/integration/targets/playbook/vars_prompt_null.yml

@@ -0,0 +1,3 @@
+- name: null vars prompt is okay
+  hosts: localhost
+  vars_prompt: null

test/units/playbook/test_play.py

@@ -22,7 +22,7 @@ __metaclass__ = type
 from units.compat import unittest
 from units.compat.mock import patch, MagicMock
 
-from ansible.errors import AnsibleParserError
+from ansible.errors import AnsibleAssertionError, AnsibleParserError
 from ansible.playbook.play import Play
 
 from units.mock.loader import DictDataLoader
@@ -64,6 +64,14 @@ class TestPlay(unittest.TestCase):
         )
         self.assertRaises(AnsibleParserError, Play.load, play_data)
 
+    def test_play_with_bad_ds_type(self):
+        play_data = []
+        self.assertRaisesRegexp(
+            AnsibleAssertionError,
+            r"while preprocessing data \(\[\]\), ds should be a dict but was a <(?:class|type) 'list'>",
+            Play.load,
+            play_data)
+
     def test_play_with_tasks(self):
         p = Play.load(dict(
             name="test play",