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

@@ -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')
 

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,

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