From e166946a0a13e0b2606396b6dd3069c4ddf0582f Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Mon, 19 Mar 2018 10:55:29 -0400 Subject: [PATCH] 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 --- lib/ansible/playbook/task.py | 5 +++++ test/units/playbook/test_task.py | 29 +++++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 47bfd49099..226380c3c0 100644 --- a/lib/ansible/playbook/task.py +++ b/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, diff --git a/test/units/playbook/test_task.py b/test/units/playbook/test_task.py index 3f1f17fd48..c9976428d0 100644 --- a/test/units/playbook/test_task.py +++ b/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):