From 728f66c598f55fc552c9de6946f7544f8f106988 Mon Sep 17 00:00:00 2001
From: Volodymyr Shypyguzov <vshypyguzov@mirantis.com>
Date: Thu, 17 Nov 2016 12:54:31 +0200
Subject: [PATCH] Remove cmd from logging exception template

With current approach logging exceptions fails with KeyError if command
itself has curly braces. E.g. awk '{print $2}'.
We need to avoid such situations, since logging exceptions is crucial
for debugging.

+ Do not format template string with command in it, use concatenation instead
+ Add negative unittest for such case

Change-Id: I0570b9fe24e662b0542f6402a0a7a882508abc8d
---
 devops/helpers/exec_result.py            |  8 +++----
 devops/tests/helpers/test_exec_result.py | 28 +++++++++++++++++++++---
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/devops/helpers/exec_result.py b/devops/helpers/exec_result.py
index 9d2d8787..5bce6f00 100644
--- a/devops/helpers/exec_result.py
+++ b/devops/helpers/exec_result.py
@@ -262,12 +262,12 @@ class ExecResult(object):
                 return yaml.safe_load(self.stdout_str)
         except BaseException:
             tmpl = (
-                "'{cmd}' stdout is not valid {fmt}:\n"
+                " stdout is not valid {fmt}:\n"
                 '{{stdout!r}}\n'.format(
-                    cmd=self.cmd,
                     fmt=fmt))
-            logger.exception(tmpl.format(stdout=self.stdout_str))
-            raise error.DevopsError(tmpl.format(stdout=self.stdout_brief))
+            logger.exception(self.cmd + tmpl.format(stdout=self.stdout_str))
+            raise error.DevopsError(
+                self.cmd + tmpl.format(stdout=self.stdout_brief))
         msg = '{fmt} deserialize target is not implemented'.format(fmt=fmt)
         logger.error(msg)
         raise error.DevopsNotImplementedError(msg)
diff --git a/devops/tests/helpers/test_exec_result.py b/devops/tests/helpers/test_exec_result.py
index ee4765c3..c6289bcb 100644
--- a/devops/tests/helpers/test_exec_result.py
+++ b/devops/tests/helpers/test_exec_result.py
@@ -23,9 +23,10 @@ import mock
 from devops import error
 from devops.helpers import exec_result
 from devops.helpers.proc_enums import ExitCodes
+from devops.helpers.subprocess_runner import Subprocess
 
 
-cmd = 'ls -la'
+cmd = "ls -la | awk \'{print $1}\'"
 
 
 # noinspection PyTypeChecker
@@ -90,8 +91,10 @@ class TestExecResult(unittest.TestCase):
             # pylint: enable=pointless-statement
         logger.assert_has_calls((
             mock.call.exception(
-                "'{cmd}' stdout is not valid json:\n"
-                "{stdout_str!r}\n".format(cmd=cmd, stdout_str='')),
+                "{cmd} stdout is not valid json:\n"
+                "{stdout_str!r}\n".format(
+                    cmd=cmd,
+                    stdout_str='')),
         ))
         self.assertIsNone(result['stdout_yaml'])
 
@@ -191,3 +194,22 @@ class TestExecResult(unittest.TestCase):
             ))
             self.assertEqual(result[deprecated], {'test': True})
             logger.reset_mock()
+
+    @mock.patch('devops.helpers.exec_result.logger', autospec=True)
+    def test_wrong_result(self, logger):
+        """Test logging exception if stdout if not a correct json"""
+        cmd = "ls -la | awk \'{print $1\}\'"
+        result = Subprocess.execute(command=cmd)
+        with self.assertRaises(error.DevopsError):
+            # pylint: disable=pointless-statement
+            # noinspection PyStatementEffect
+            result.stdout_json
+            # pylint: enable=pointless-statement
+        logger.assert_has_calls((
+            mock.call.exception(
+                "{cmd} stdout is not valid json:\n"
+                "{stdout_str!r}\n".format(
+                    cmd=cmd,
+                    stdout_str='')),
+        ))
+        self.assertIsNone(result['stdout_yaml'])