Refactor remote command execution in vsperf

* Remove unneeded variables
* Do not raise RuntimeError when a remote command fails, instead leave
  it for the yardstick ssh module to handle.
* Prevent CsvReader from raising StopIteration if csv output cannot be
  parsed. The SLA validation will take care of errors in that case.

JIRA: YARDSTICK-1166

Change-Id: I6cf550bd7bc8f511b3c0f25c67f8caab18bccd28
Signed-off-by: Miikka Koistinen <miikka.koistinen@nokia.com>

diff --git a/yardstick/benchmark/scenarios/networking/vsperf.py b/yardstick/benchmark/scenarios/networking/vsperf.py
index 2b34740..8344b15 100644 (file)
--- a/yardstick/benchmark/scenarios/networking/vsperf.py
+++ b/yardstick/benchmark/scenarios/networking/vsperf.py
@@ -193,22 +193,19 @@ class Vsperf(base.Scenario):
             cmd += "--conf-file ~/vsperf.conf "
         cmd += "--test-params=\"%s\"" % (';'.join(test_params))
         LOG.debug("Executing command: %s", cmd)
-        status, stdout, stderr = self.client.execute(cmd)
-
-        if status:
-            raise RuntimeError(stderr)
+        self.client.run(cmd)
 
         # get test results
         cmd = "cat /tmp/results*/result.csv"
         LOG.debug("Executing command: %s", cmd)
-        status, stdout, stderr = self.client.execute(cmd)
-
-        if status:
-            raise RuntimeError(stderr)
+        _, stdout, _ = self.client.execute(cmd, raise_on_error=True)
 
         # convert result.csv to JSON format
-        reader = csv.DictReader(stdout.split('\r\n'))
-        result.update(next(reader))
+        reader = csv.DictReader(stdout.split('\r\n'), strict=True)
+        try:
+            result.update(next(reader))
+        except StopIteration:
+            pass
 
         # sla check; go through all defined SLAs and check if values measured
         # by VSPERF are higher then those defined by SLAs

diff --git a/yardstick/tests/unit/benchmark/scenarios/networking/test_vsperf.py b/yardstick/tests/unit/benchmark/scenarios/networking/test_vsperf.py
index a606543..a1c27f5 100644 (file)
--- a/yardstick/tests/unit/benchmark/scenarios/networking/test_vsperf.py
+++ b/yardstick/tests/unit/benchmark/scenarios/networking/test_vsperf.py
@@ -54,7 +54,8 @@ class VsperfTestCase(unittest.TestCase):
 
         self._mock_SSH = mock.patch.object(ssh, 'SSH')
         self.mock_SSH = self._mock_SSH.start()
-        self.mock_SSH.from_node().execute.return_value = (0, '', '')
+        self.mock_SSH.from_node().execute.return_value = (
+            0, 'throughput_rx_fps\r\n14797660.000\r\n', '')
 
         self._mock_subprocess_call = mock.patch.object(subprocess, 'call')
         self.mock_subprocess_call = self._mock_subprocess_call.start()
@@ -104,40 +105,23 @@ class VsperfTestCase(unittest.TestCase):
     def test_run_ok(self):
         self.scenario.setup()
 
-        self.mock_SSH.from_node().execute.return_value = (
-            0, 'throughput_rx_fps\r\n14797660.000\r\n', '')
-
         result = {}
         self.scenario.run(result)
 
         self.assertEqual(result['throughput_rx_fps'], '14797660.000')
 
     def test_run_ok_setup_not_done(self):
-        self.mock_SSH.from_node().execute.return_value = (
-            0, 'throughput_rx_fps\r\n14797660.000\r\n', '')
-
         result = {}
         self.scenario.run(result)
 
         self.assertTrue(self.scenario.setup_done)
         self.assertEqual(result['throughput_rx_fps'], '14797660.000')
 
-    def test_run_failed_vsperf_execution(self):
-        self.mock_SSH.from_node().execute.side_effect = ((0, '', ''),
-                                                         (1, '', ''))
+    def test_run_ssh_command_call_counts(self):
+        self.scenario.run({})
 
-        with self.assertRaises(RuntimeError):
-            self.scenario.run({})
         self.assertEqual(self.mock_SSH.from_node().execute.call_count, 2)
-
-    def test_run_failed_csv_report(self):
-        self.mock_SSH.from_node().execute.side_effect = ((0, '', ''),
-                                                         (0, '', ''),
-                                                         (1, '', ''))
-
-        with self.assertRaises(RuntimeError):
-            self.scenario.run({})
-        self.assertEqual(self.mock_SSH.from_node().execute.call_count, 3)
+        self.mock_SSH.from_node().run.assert_called_once()
 
     def test_run_sla_fail(self):
         self.mock_SSH.from_node().execute.return_value = (
@@ -160,14 +144,21 @@ class VsperfTestCase(unittest.TestCase):
         self.assertTrue('throughput_rx_fps was not collected by VSPERF'
                         in str(raised.exception))
 
+    def test_run_faulty_result_csv(self):
+        self.mock_SSH.from_node().execute.return_value = (
+            0, 'faulty output not csv', '')
+
+        with self.assertRaises(y_exc.SLAValidationError) as raised:
+            self.scenario.run({})
+
+        self.assertTrue('throughput_rx_fps was not collected by VSPERF'
+                        in str(raised.exception))
+
     def test_run_sla_fail_metric_not_defined_in_sla(self):
         del self.scenario_cfg['sla']['throughput_rx_fps']
         scenario = vsperf.Vsperf(self.scenario_cfg, self.context_cfg)
         scenario.setup()
 
-        self.mock_SSH.from_node().execute.return_value = (
-            0, 'throughput_rx_fps\r\n14797660.000\r\n', '')
-
         with self.assertRaises(y_exc.SLAValidationError) as raised:
             scenario.run({})
         self.assertTrue('throughput_rx_fps is not defined in SLA'