Merge ~pwlars/testflinger-agent:fix-empty-sections-exception into testflinger-agent:master

[Paul Larson]

We already support the idea of skipping a section if it's missing (ex. if you want to skip provision, simply leave out the provisioning section). however I also noticed today that if you leave out the data in the provisioning section, it gives you a pretty confusing error.

Now, if it's present but doesn't have anything in it, it should result in the same sort of non-fatal message that happens when the section is missing completely. However it will still try to do the other parts of the job that it can.

[Sheila Miguez (codersquid)]

lgtm

review: Approve

diff --git a/testflinger_agent/job.py b/testflinger_agent/job.py
index b76dfdc..b717d32 100644
--- a/testflinger_agent/job.py
+++ b/testflinger_agent/job.py
@@ -55,13 +55,13 @@ class TestflingerJob:
         if not cmd:
             logger.info('No %s_command configured, skipping...', phase)
             return 0
-        if phase == 'provision' and 'provision_data' not in self.job_data:
+        if phase == 'provision' and not self.job_data.get('provision_data'):
             logger.info('No provision_data defined in job data, skipping...')
             return 0
-        if phase == 'test' and 'test_data' not in self.job_data:
+        if phase == 'test' and not self.job_data.get('test_data'):
             logger.info('No test_data defined in job data, skipping...')
             return 0
-        if phase == 'reserve' and 'reserve_data' not in self.job_data:
+        if phase == 'reserve' and not self.job_data.get('reserve_data'):
             return 0
         output_log = os.path.join(rundir, phase+'.log')
         serial_log = os.path.join(rundir, phase+'-serial.log')
diff --git a/testflinger_agent/tests/test_agent.py b/testflinger_agent/tests/test_agent.py
index 496dbe3..e9ce10c 100644
--- a/testflinger_agent/tests/test_agent.py
+++ b/testflinger_agent/tests/test_agent.py
@@ -52,7 +52,7 @@ class TestClient:
         self.config['provision_command'] = 'echo provision1'
         fake_job_data = {'job_id': str(uuid.uuid1()),
                          'job_queue': 'test',
-                         'provision_data': ''}
+                         'provision_data': {'url': 'foo'}}
         requests_mock.get(rmock.ANY, [{'text': json.dumps(fake_job_data)},
                                       {'text': '{}'}])
         requests_mock.post(rmock.ANY, status_code=200)
@@ -67,7 +67,7 @@ class TestClient:
         self.config['test_command'] = 'echo test1'
         fake_job_data = {'job_id': str(uuid.uuid1()),
                          'job_queue': 'test',
-                         'test_data': ''}
+                         'test_data': {'test_cmds': 'foo'}}
         requests_mock.get(rmock.ANY, [{'text': json.dumps(fake_job_data)},
                                       {'text': '{}'}])
         requests_mock.post(rmock.ANY, status_code=200)
@@ -82,7 +82,7 @@ class TestClient:
         self.config['test_command'] = 'echo test_string is $test_string'
         fake_job_data = {'job_id': str(uuid.uuid1()),
                          'job_queue': 'test',
-                         'test_data': ''}
+                         'test_data': {'test_cmds': 'foo'}}
         requests_mock.get(rmock.ANY, [{'text': json.dumps(fake_job_data)},
                                       {'text': '{}'}])
         requests_mock.post(rmock.ANY, status_code=200)
@@ -99,8 +99,8 @@ class TestClient:
         self.config['test_command'] = 'echo test1'
         fake_job_data = {'job_id': str(uuid.uuid1()),
                          'job_queue': 'test',
-                         'provision_data': '',
-                         'test_data': ''}
+                         'provision_data': {'url': 'foo'},
+                         'test_data': {'test_cmds': 'foo'}}
         requests_mock.get(rmock.ANY, [{'text': json.dumps(fake_job_data)},
                                       {'text': '{}'}])
         requests_mock.post(rmock.ANY, status_code=200)
@@ -153,8 +153,8 @@ class TestClient:
         job_id = str(uuid.uuid1())
         fake_job_data = {'job_id': job_id,
                          'job_queue': 'test',
-                         'provision_data': '',
-                         'test_data': ''}
+                         'provision_data': {'url': 'foo'},
+                         'test_data': {'test_cmds': 'foo'}}
         # In this case we are making sure that the repost job request
         # gets good status
         with rmock.Mocker() as m:
diff --git a/testflinger_agent/tests/test_job.py b/testflinger_agent/tests/test_job.py
index 481e7b4..bb70c22 100644
--- a/testflinger_agent/tests/test_job.py
+++ b/testflinger_agent/tests/test_job.py
@@ -38,6 +38,20 @@ class TestJob():
             log_output = log.read()
         assert("No provision_data defined in job data" in log_output)
 
+    def test_skip_empty_provision_data(self, client):
+        """
+        Test that provision phase is skipped when provision_data is
+        present but empty
+        """
+        self.config['provision_command'] = '/bin/true'
+        fake_job_data = {'global_timeout': 1, 'provision_data': ''}
+        job = _TestflingerJob(fake_job_data, client)
+        job.run_test_phase('provision', None)
+        logfile = os.path.join(self.tmpdir, 'testflinger-agent.log')
+        with open(logfile) as log:
+            log_output = log.read()
+        assert("No provision_data defined in job data" in log_output)
+
     def test_job_global_timeout(self, client, requests_mock):
         """Test that timeout from job_data is respected"""
         timeout_str = '\nERROR: Global timeout reached! (1s)\n'