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 9dba580204

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

@@ -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'):
+            result.pop('invocation', None)
-                if remove_key in result:
-                    del result[remove_key]
 
     def set_play_context(self, play_context):
         pass

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(self):
+    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)
-        self.assertTrue('invocation' 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}
+
+        expected_result = result.copy()
+        cb._clean_results(result, 'ebug')
+        self.assertEqual(result, expected_result)
 
 
 class TestCallbackDumpResults(unittest.TestCase):