From 456b88525593003c5fbef549581a5312c1ea7eb7 Mon Sep 17 00:00:00 2001
From: Paul Van Eck <pvaneck@us.ibm.com>
Date: Wed, 7 Jan 2015 10:32:20 -0800
Subject: [PATCH] Change returncode checking to file checking

Previously, after the run_tempest script, was run, all post-processing
was done only if the returncode was 0. If it was non-zero, an error message
was given. This was an oversight, as the run_tempest script will return a code
of 1 when any test case fails. This leaves the unintended side effect of refstack
not parsing the results or producing the output json when Tempest successfully
runs, but a test case fails.

Checking for the existence of the expected subunit file will at least validate
that the Tempest test was at least started successfully, and didn't fail to run
due to some misconfiguration in the environment.

Change-Id: I541b798db41b713525efeb8dedfb9347be780a3f
---
 refstack_client/refstack_client.py        | 8 +++++---
 refstack_client/tests/unit/test_client.py | 7 +++++--
 tox.ini                                   | 2 +-
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/refstack_client/refstack_client.py b/refstack_client/refstack_client.py
index 3ba35e8..6a5ccbb 100755
--- a/refstack_client/refstack_client.py
+++ b/refstack_client/refstack_client.py
@@ -181,7 +181,7 @@ class RefstackClient:
         start_time = time.time()
 
         # Run the tempest script, specifying the conf file, the flag
-        # telling it to not use a virtual environment (-N), and the flag
+        # telling it to use a virtual environment (-V), and the flag
         # telling it to run the tests serially (-t).
         cmd = (self.tempest_script, '-C', self.conf_file, '-V', '-t')
 
@@ -204,7 +204,9 @@ class RefstackClient:
         process = subprocess.Popen(cmd, stderr=stderr)
         process.communicate()
 
-        if process.returncode == 0:
+        # If the subunit file was created, then the Tempest test was at least
+        # started successfully.
+        if os.path.isfile(results_file):
             end_time = time.time()
             elapsed = end_time - start_time
             duration = int(elapsed)
@@ -240,7 +242,7 @@ class RefstackClient:
 
 def parse_cli_args(args=None):
 
-    usage_string = ('refstack-client [-h] {upload,test,setup} ...\n\n'
+    usage_string = ('refstack-client [-h] <ARG> ...\n\n'
                     'To see help on specific argument, do:\n'
                     'refstack-client <ARG> -h')
 
diff --git a/refstack_client/tests/unit/test_client.py b/refstack_client/tests/unit/test_client.py
index 9d77386..a034830 100755
--- a/refstack_client/tests/unit/test_client.py
+++ b/refstack_client/tests/unit/test_client.py
@@ -230,6 +230,7 @@ class TestRefstackClient(unittest.TestCase):
         mock_popen = self.patch(
             'refstack_client.refstack_client.subprocess.Popen',
             return_value=MagicMock(returncode=0))
+        self.patch("os.path.isfile", return_value=True)
         self.mock_keystone()
         client.get_passed_tests = MagicMock(return_value=['test'])
         client.post_results = MagicMock()
@@ -258,6 +259,7 @@ class TestRefstackClient(unittest.TestCase):
         mock_popen = self.patch(
             'refstack_client.refstack_client.subprocess.Popen',
             return_value=MagicMock(returncode=0))
+        self.patch("os.path.isfile", return_value=True)
         self.mock_keystone()
         client.get_passed_tests = MagicMock(return_value=['test'])
         client.post_results = MagicMock()
@@ -283,10 +285,11 @@ class TestRefstackClient(unittest.TestCase):
 
     def test_run_tempest_nonexisting_directory(self):
         """
-        Test when a nonexistent Tempest directory is passed in.
+        Test when the Tempest directory does not exist.
         """
         args = rc.parse_cli_args(self.mock_argv())
         client = rc.RefstackClient(args)
+        client.tempest_dir = "/does/not/exist"
         self.assertRaises(SystemExit, client.test)
 
     def test_failed_run(self):
@@ -323,7 +326,7 @@ class TestRefstackClient(unittest.TestCase):
 
     def test_upload_nonexisting_file(self):
         """
-        Test that the upload file does not exist
+        Test when the file to be uploaded does not exist.
         """
         upload_file_path = self.test_path + "/.testrepository/foo.json"
         args = rc.parse_cli_args(['upload', upload_file_path,
diff --git a/tox.ini b/tox.ini
index 8fdd644..0120bca 100644
--- a/tox.ini
+++ b/tox.ini
@@ -37,4 +37,4 @@ commands = python setup.py build_sphinx
 ignore = E125,H404
 show-source = true
 builtins = _
-exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,tools,build
+exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,tools,build,.tempest