From a57d6a4206e045ea69b979a93c814eeeaaed3b4d Mon Sep 17 00:00:00 2001
From: Adrian Likins <alikins@redhat.com>
Date: Mon, 11 Dec 2017 17:41:12 -0500
Subject: [PATCH] Use pop in cb clean results (#33779)

* Fix _clean_result for debug callback with 1 char var names

The check in _clean_results was removing any keys that happened
to be one of the chars in ('invocation') instead of the string
'invocation'. This was meant to be a tuple but there was no comma
so the for iterated the string instead of the tuple.

Introduced in 9dba580204fcc7d95d4ca2ea2ae4ce23fe28c2ed

Update unit test to catch this.

Fixes #33723

* Use .pop() to remove invocation from results dict

In base callback _clean_results, simplify the way the
'invocation' item is removed.

Add some more unit tests.
---
 lib/ansible/plugins/callback/__init__.py     |  4 +-
 test/units/plugins/callback/test_callback.py | 44 ++++++++++++++++++--
 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/lib/ansible/plugins/callback/__init__.py b/lib/ansible/plugins/callback/__init__.py
index 6b41e1dd9c8..436e963e042 100644
--- a/lib/ansible/plugins/callback/__init__.py
+++ b/lib/ansible/plugins/callback/__init__.py
@@ -236,9 +236,7 @@ class CallbackBase(AnsiblePlugin):
     def _clean_results(self, result, task_name):
         ''' removes data from results for display '''
         if task_name in ['debug']:
-            for remove_key in ('invocation'):
-                if remove_key in result:
-                    del result[remove_key]
+            result.pop('invocation', None)
 
     def set_play_context(self, play_context):
         pass
diff --git a/test/units/plugins/callback/test_callback.py b/test/units/plugins/callback/test_callback.py
index 0e230472fac..9ccf868753a 100644
--- a/test/units/plugins/callback/test_callback.py
+++ b/test/units/plugins/callback/test_callback.py
@@ -67,15 +67,53 @@ class TestCallbackResults(unittest.TestCase):
         res = cb._get_item(results)
         self.assertEquals(res, "some_item")
 
+    def test_clean_results_debug_task(self):
+        cb = CallbackBase()
+        result = {'item': 'some_item',
+                  'invocation': 'foo --bar whatever [some_json]',
+                  'a': 'a single a in result note letter a is in invocation',
+                  'b': 'a single b in result note letter b is not in invocation',
+                  'changed': True}
+
+        cb._clean_results(result, 'debug')
+
+        # See https://github.com/ansible/ansible/issues/33723
+        self.assertTrue('a' in result)
+        self.assertTrue('b' in result)
+        self.assertFalse('invocation' in result)
+        self.assertTrue('changed' in result)
+
+    def test_clean_results_debug_task_no_invocation(self):
+        cb = CallbackBase()
+        result = {'item': 'some_item',
+                  'a': 'a single a in result note letter a is in invocation',
+                  'b': 'a single b in result note letter b is not in invocation',
+                  'changed': True}
+
+        cb._clean_results(result, 'debug')
+        self.assertTrue('a' in result)
+        self.assertTrue('b' in result)
+        self.assertTrue('changed' in result)
+        self.assertFalse('invocation' in result)
+
+    def test_clean_results_debug_task_empty_results(self):
+        cb = CallbackBase()
+        result = {}
+        cb._clean_results(result, 'debug')
+        self.assertFalse('invocation' in result)
+        self.assertEqual(len(result), 0)
+
     def test_clean_results(self):
         cb = CallbackBase()
         result = {'item': 'some_item',
                   'invocation': 'foo --bar whatever [some_json]',
+                  'a': 'a single a in result note letter a is in invocation',
+                  'b': 'a single b in result note letter b is not in invocation',
                   'changed': True}
 
-        self.assertTrue('changed' in result)
-        self.assertTrue('invocation' in result)
-        cb._clean_results(result, 'debug')
+        expected_result = result.copy()
+        cb._clean_results(result, 'ebug')
+        self.assertEqual(result, expected_result)
 
 
 class TestCallbackDumpResults(unittest.TestCase):