Fix redundant yaml error blurbs on ModArgs parse errors (#36923)

* Fix redundant yaml error blurbs on ModArgs parse errors

Some of the AnsibleParserErrors from parsing.mod_args
are created with the obj=some_yaml_ds options but
some are not.

If they were, we don't want to add another yaml_ds to
it, because that will result in double yaml error blurbs.
And since we dont need to add info, we can just re raise it.

But if there is no ._obj, add it here so we get the extra
detail in the error message (see issue #14790) and raise
a new AnsibleParserError instance.

Fixes #36848

* cleanup existing test_tasks pep8/sanity issues

(cherry picked from commit e166946a0a)

lib/ansible/playbook/task.py

@@ -184,6 +184,11 @@ class Task(Base, Conditional, Taggable, Become):
         try:
             (action, args, delegate_to) = args_parser.parse()
         except AnsibleParserError as e:
+            # if the raises exception was created with obj=ds args, then it includes the detail
+            # so we dont need to add it so we can just re raise.
+            if e._obj:
+                raise
+            # But if it wasn't, we can add the yaml object now to get more detail
             raise AnsibleParserError(to_native(e), obj=ds, orig_exc=e)
 
         # the command/shell/script modules used to support the `cmd` arg,

test/units/playbook/test_task.py

@@ -20,7 +20,10 @@ from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
 from ansible.compat.tests import unittest
+from ansible.compat.tests.mock import patch
 from ansible.playbook.task import Task
+from ansible.parsing.yaml import objects
+from ansible import errors
 
 
 basic_command_task = dict(
@@ -32,6 +35,10 @@ kv_command_task = dict(
     action='command echo hi'
 )
 
+# See #36848
+kv_bad_args_str = '- apk: sdfs sf sdf 37'
+kv_bad_args_ds = {'apk': 'sdfs sf sdf 37'}
+
 
 class TestTask(unittest.TestCase):
 
@@ -42,7 +49,7 @@ class TestTask(unittest.TestCase):
         pass
 
     def test_construct_empty_task(self):
-        t = Task()
+        Task()
 
     def test_construct_task_with_role(self):
         pass
@@ -65,9 +72,27 @@ class TestTask(unittest.TestCase):
         self.assertEqual(t.action, 'command')
         self.assertEqual(t.args, dict(_raw_params='echo hi'))
 
+    @patch.object(errors.AnsibleError, '_get_error_lines_from_file')
+    def test_load_task_kv_form_error_36848(self, mock_get_err_lines):
+        ds = objects.AnsibleMapping(kv_bad_args_ds)
+        ds.ansible_pos = ('test_task_faux_playbook.yml', 1, 1)
+        mock_get_err_lines.return_value = (kv_bad_args_str, '')
+
+        with self.assertRaises(errors.AnsibleParserError) as cm:
+            Task.load(ds)
+
+        self.assertIsInstance(cm.exception, errors.AnsibleParserError)
+        self.assertEqual(cm.exception._obj, ds)
+        self.assertEqual(cm.exception._obj, kv_bad_args_ds)
+        self.assertIn("The error appears to have been in 'test_task_faux_playbook.yml", cm.exception.message)
+        self.assertIn(kv_bad_args_str, cm.exception.message)
+        self.assertIn('apk', cm.exception.message)
+        self.assertEquals(cm.exception.message.count('The offending line'), 1)
+        self.assertEquals(cm.exception.message.count('The error appears to have been in'), 1)
+
     def test_task_auto_name(self):
         assert 'name' not in kv_command_task
-        t = Task.load(kv_command_task)
+        Task.load(kv_command_task)
         # self.assertEqual(t.name, 'shell echo hi')
 
     def test_task_auto_name_with_role(self):