[PATCH] fix for BZ 789163 fix for make coverage to include ALL py files in coverage report added tests to bring back up the test coverage to 99% after the missed files were added back into coverage

Dan Radez dradez at redhat.com
Fri Feb 10 21:17:29 UTC 2012


---
 agent/Makefile                       |    2 +-
 agent/src/audrey/__init__.py         |    6 +-
 agent/src/audrey/csclient/tooling.py |   96 +++++++----------------
 agent/tests/__init__.py              |   10 +++
 agent/tests/agent.py                 |    2 +-
 agent/tests/csclient.py              |  140 +++++++++++++++++++++++++---------
 agent/tests/user_data.py             |    6 +-
 7 files changed, 149 insertions(+), 113 deletions(-)

diff --git a/agent/Makefile b/agent/Makefile
index 5a96004..04d8ecf 100644
--- a/agent/Makefile
+++ b/agent/Makefile
@@ -92,6 +92,6 @@ clean:
 coverage: src/audrey/agent.py
 	coverage erase
 	coverage run test_audrey_agent.py
-	coverage report -m test_audrey_agent.py src/audrey/*.py tests/*.py
+	coverage report -m test_audrey_agent.py `find src/audrey/ -name "*.py"` tests/*.py
 
 .PHONY: dist test rpms srpm
diff --git a/agent/src/audrey/__init__.py b/agent/src/audrey/__init__.py
index 545181f..5675487 100644
--- a/agent/src/audrey/__init__.py
+++ b/agent/src/audrey/__init__.py
@@ -72,8 +72,10 @@ class ASError(Exception):
     '''
     Some sort of error occurred. The exact cause of the error should
     have been logged. So, this just indicates that something is wrong.
-    when invoked from rc.local we redirect stderr to the log so
+    when invoked without args we redirect stderr to the log so
     logging here is not really nessesary
-    TODO: Should really have more of these that are more specific to the error.
     '''
     pass
+
+class ASErrorInvalidTar(ASError):
+    pass
diff --git a/agent/src/audrey/csclient/tooling.py b/agent/src/audrey/csclient/tooling.py
index 0878372..d3b82d8 100644
--- a/agent/src/audrey/csclient/tooling.py
+++ b/agent/src/audrey/csclient/tooling.py
@@ -20,7 +20,8 @@ import os
 import logging
 import tarfile
 
-from audrey import ASError
+from audrey import ASError, ASErrorInvalidTar
+from audrey.shell import run_cmd
 
 TOOLING_DIR = '/var/audrey/tooling/'
 
@@ -46,12 +47,12 @@ class ConfigTooling(object):
     def __init__(self, tool_dir=TOOLING_DIR):
         '''
         Description:
-            Set initial state so it can be tracked. Valuable for
-            testing and debugging.
+            Set initial state so it can be tracked.
         '''
         self.tool_dir = tool_dir
-        self.user_dir = tool_dir + 'user/'
-        self.log = tool_dir + 'log'
+        self.user_dir = os.path.join(tool_dir, 'user')
+        self.audrey_dir = os.path.join(tool_dir, 'AUDREY_TOOLING')
+        self.redhat_dir = os.path.join(tool_dir, 'REDHAT')
         self.tarball = ''
 
         # Create the extraction destination
@@ -64,9 +65,6 @@ class ConfigTooling(object):
                 raise ASError(('Failed to create directory %s. ' + \
                     'Error: %s') % (self.user_dir, strerror))
 
-        self.ct_logger = logging.getLogger('ConfigTooling')
-        self.ct_logger.addHandler(logging.FileHandler(self.log))
-
     def __str__(self):
         '''
         Description:
@@ -76,36 +74,14 @@ class ConfigTooling(object):
         return('\n<Instance of: %s\n' \
                '\tTooling Dir: %s\n' \
                '\tUnpack User Tooling Tarball Dir: %s\n' \
-               '\tLog File: %s\n' \
                '\ttarball Name: %s\n' \
                'eot>' %
             (self.__class__.__name__,
             str(self.tool_dir),
             str(self.user_dir),
-            str(self.log),
             str(self.tarball),
             ))
 
-    def log_info(self, log_str):
-        '''
-        Description:
-            Used for logging the commands that have been executed
-            along with their output and return codes.
-
-            Simply logs the provided input string.
-        '''
-        self.ct_logger.info(log_str)
-
-    def log_error(self, log_str):
-        '''
-        Description:
-            Used for logging errors encountered when attempting to
-            execute the service command.
-
-            Simply logs the provided input string.
-        '''
-        self.ct_logger.error(log_str)
-
     def invoke_tooling(self, services):
         '''
         Description:
@@ -131,24 +107,24 @@ class ConfigTooling(object):
             cmd = [tooling_path]
             cmd_dir = os.path.dirname(tooling_path)
             ret = run_cmd(cmd, cmd_dir)
-            self.log_info('Execute Tooling command: ' + ' '.join(cmd))
+            logger.info('Execute Tooling command: ' + ' '.join(cmd))
 
             retcode = ret['subproc'].returncode
             if retcode == 0:
                 # Command successed, log the output.
-                self.log_info('return code: ' + str(retcode))
-                self.log_info('\n\tStart Output of: ' + ' '.join(cmd) + \
+                logger.info('return code: ' + str(retcode))
+                logger.info('\n\tStart Output of: ' + ' '.join(cmd) + \
                     ' >>>\n' +  \
                     str(ret['out']) + \
                     '\n\t<<< End Output')
             else:
                 # Command failed, log the errors.
-                self.log_info('\n\tStart Output of: ' + ' '.join(cmd) + \
+                logger.info('\n\tStart Output of: ' + ' '.join(cmd) + \
                     ' >>>\n' +  \
                     str(ret['out']) + \
                     '\n\t<<< End Output')
-                self.log_error('error code: ' + str(retcode))
-                self.log_error('error msg:  ' + str(ret['err']))
+                logger.error('error code: ' + str(retcode))
+                logger.error('error msg:  ' + str(ret['err']))
 
             # If tooling was provided at the top level only run it once
             # for all services listed in the required config params.
@@ -171,28 +147,21 @@ class ConfigTooling(object):
         self.tarball = tarball
 
         # Validate the specified tarfile.
-        try:
-            if not tarfile.is_tarfile(self.tarball):
-                # If file exists but is not a tar file force IOError.
-                raise IOError
-        except IOError, (errno, strerror):
-            raise ASError(('File was not found or is not a tar file: %s ' + \
-                    'Error: %s %s') % (self.tarball, errno, strerror))
+        if not os.path.exists(self.tarball):
+            raise ASError('File does not exist: %s ' % self.tarball)
+        if not tarfile.is_tarfile(self.tarball):
+            raise ASErrorInvalidTar('File is not a tar file: %s ' % self.tarball)
 
         # Attempt to extract the contents from the specified tarfile.
-        #
         # If tarfile access or content is bad report to the user to aid
         # problem resolution.
         try:
             tarf = tarfile.open(self.tarball)
             tarf.extractall(path=self.user_dir)
             tarf.close()
-        except IOError, (errno, strerror):
-            raise ASError(('Failed to access tar file %s. Error: %s') %  \
-                (self.tarball, strerror))
         # Capture and report errors with the tarfile
         except (tarfile.TarError, tarfile.ReadError, tarfile.CompressionError, \
-                tarfile.StreamError, tarfile.ExtractError), (strerror):
+                tarfile.StreamError, tarfile.ExtractError, IOError), (strerror):
             raise ASError(('Failed to access tar file %s. Error: %s') %  \
                 (self.tarball, strerror))
 
@@ -242,25 +211,18 @@ class ConfigTooling(object):
             return 1 - True if top level tooling found, False otherwise.
             return 2 - path to tooling
         '''
-
-        top_path = self.tool_dir + 'user/start'
-        if os.access(top_path, os.X_OK):
-            return True, top_path
-
-        service_user_path = self.tool_dir + 'user/' + \
-            service_name + '/start'
-        if os.access(service_user_path, os.X_OK):
-            return False, service_user_path
-
-        service_redhat_path = self.tool_dir + 'AUDREY_TOOLING/' + \
-            service_name + '/start'
-        if os.access(service_redhat_path, os.X_OK):
-            return False, service_redhat_path
-
-        service_redhat_path = self.tool_dir + 'REDHAT/' + \
-            service_name + '/start'
-        if os.access(service_redhat_path, os.X_OK):
-            return False, service_redhat_path
+        # common join
+        service_start = os.path.join(service_name, 'start')
+        # returns, check the paths and return the tuple
+        tooling_paths = [(True, os.path.join(self.user_dir, 'start')),
+                         (False,os.path.join(self.user_dir, service_start)),
+                         (False, os.path.join(self.audrey_dir, service_start)),
+                         (False, os.path.join(self.redhat_dir, service_start)),
+                        ]
+
+        for p in tooling_paths:
+            if os.access(p[1], os.X_OK):
+                return p
 
         # No tooling found. Raise an error.
         raise ASError(('No configuration tooling found for service: %s') % \
diff --git a/agent/tests/__init__.py b/agent/tests/__init__.py
index e64eb3f..a94b985 100644
--- a/agent/tests/__init__.py
+++ b/agent/tests/__init__.py
@@ -15,3 +15,13 @@
 *  limitations under the License.
 *
 '''
+
+import os
+import stat
+
+def _write_file(filepath, content, mode=0664):
+    f = open(filepath, 'w')
+    f.write(content)
+    f.close()
+    # chmod takes 4 digits!
+    os.chmod(filepath, mode)
diff --git a/agent/tests/agent.py b/agent/tests/agent.py
index 0ca549d..de4587a 100644
--- a/agent/tests/agent.py
+++ b/agent/tests/agent.py
@@ -30,7 +30,7 @@ from audrey.agent import main
 from audrey.csclient import gen_env
 
 from tests.mocks import CLOUD_INFO_FILE
-from tests.user_data import _write_file
+from tests import _write_file
 
 class TestAudreyAgent(unittest.TestCase):
     '''
diff --git a/agent/tests/csclient.py b/agent/tests/csclient.py
index 24b2a13..4538e4a 100644
--- a/agent/tests/csclient.py
+++ b/agent/tests/csclient.py
@@ -16,10 +16,12 @@
 *
 '''
 
+import os
+import shutil
 import unittest
 import base64
 
-from audrey import ASError
+from audrey import ASError, ASErrorInvalidTar
 from audrey.shell import run_cmd
 from audrey.csclient.client import CSClient
 from audrey.csclient.tooling import ConfigTooling
@@ -28,11 +30,33 @@ from audrey.csclient import parse_provides_params
 from audrey.csclient import generate_provides
 
 from tests.mocks import HttpUnitTest
+from tests import _write_file
 
 DUMMY_CS_CONFIG = {'endpoint': 'http://example.com/',
                    'oauth_key': 'oauthConsumer',
                    'oauth_secret': 'oauthSecret',}
 
+DUMMY_NO_SERVICE_CONFIG_DATA = '|service|' + \
+    '|parameters|jon_server_ip&' + base64.b64encode('192.168.1.1') + \
+    '|jon_server_ip_2&' + base64.b64encode('192.168.1.2') + \
+    '|jon_server_ip_3&' + base64.b64encode('192.168.1.3') + \
+    '|service|jon2|'
+
+VALIDATE_NO_SERVICE_CONFIG_DATA = {'AUDREY_VAR_jon_server_ip' : '192.168.1.1',
+    'AUDREY_VAR_jon_server_ip_2' : '192.168.1.2',
+    'AUDREY_VAR_jon_server_ip_3' : '192.168.1.3' }
+
+DUMMY_SERVICE_CONFIG_DATA = '|service|jon1' + \
+    '|parameters|jon_server_ip&' + base64.b64encode('192.168.1.1') + \
+    '|jon_server_ip_2&' + base64.b64encode('192.168.1.2') + \
+    '|jon_server_ip_3&' + base64.b64encode('192.168.1.3') + \
+    '|service|jon2|'
+
+VALIDATE_SERVICE_CONFIG_DATA = {'AUDREY_VAR_jon1_jon_server_ip' : '192.168.1.1',
+    'AUDREY_VAR_jon1_jon_server_ip_2' : '192.168.1.2',
+    'AUDREY_VAR_jon1_jon_server_ip_3' : '192.168.1.3' }
+
+
 class TestAudreyCSClient(unittest.TestCase):
     '''
     Class for exercising the gets and put to and from the CS
@@ -106,23 +130,87 @@ class TestAudreyCSClient(unittest.TestCase):
         '''
         self.cs_client._put('http://hostname/raiseException')
 
-class TestAudreyStartupConfigTooling(unittest.TestCase):
+class TestAudreyAgentConfigTooling(unittest.TestCase):
     '''
     Make sure all the Config tooling is tested
     '''
-    def test_is_user_supplied(self):
-        ConfigTooling('test_tooling').is_user_supplied()
+    def setUp(self):
+        self.tool_dir=os.path.join(os.path.abspath('.'),'test_tooling')
+        self.csclient=ConfigTooling(self.tool_dir)
 
-    def test_is_rh_supplied(self):
-        ConfigTooling('test_tooling').is_rh_supplied()
-
-    def test_empty_find_tooling(self):
-        self.assertRaises(ASError, ConfigTooling('test_tooling').find_tooling, '')
+    def tearDown(self):
+        if os.path.exists(self.tool_dir):
+            shutil.rmtree(self.tool_dir)
 
     def test_fail_to_create_tooling_dir(self):
         self.assertRaises(ASError, ConfigTooling, tool_dir='/not/real/dir')
 
-class TestAudreyStartupRequiredConfig(unittest.TestCase):
+    ###### Note
+    # the first two are essentially no op
+    # can we remove the code from the agent?
+    def test_is_user_supplied(self):
+        self.csclient.is_user_supplied()
+
+    def test_is_rh_supplied(self):
+        self.csclient.is_rh_supplied()
+    ###### end note
+
+    def test_empty_find_tooling(self):
+        self.assertRaises(ASError, self.csclient.find_tooling, '')
+
+    def test_find_user_tooling(self):
+        start_path = os.path.join(self.csclient.user_dir, 'start')
+        _write_file(start_path, '#!/bin/sh\nexit 0', 0744)
+        self.csclient.find_tooling('')
+
+    def test_find_user_service_tooling(self):
+        service_dir = os.path.join(self.csclient.user_dir, 'test_service')
+        os.mkdir(service_dir)
+        _write_file(os.path.join(service_dir, 'start'), '#!/bin/sh\nexit 0', 0744)
+        self.csclient.find_tooling('test_service')
+
+    def test_find_audrey_service_tooling(self):
+        service_dir = os.path.join(self.csclient.audrey_dir, 'test_service')
+        os.mkdir(self.csclient.audrey_dir)
+        os.mkdir(service_dir)
+        _write_file(os.path.join(service_dir, 'start'), '#!/bin/sh\nexit 0', 0744)
+        self.csclient.find_tooling('test_service')
+
+    def test_find_redhat_service_tooling(self):
+        service_dir = os.path.join(self.csclient.redhat_dir, 'test_service')
+        os.mkdir(self.csclient.redhat_dir)
+        os.mkdir(service_dir)
+        _write_file(os.path.join(service_dir, 'start'), '#!/bin/sh\nexit 0', 0744)
+        self.csclient.find_tooling('test_service')
+
+    def test_missing_file_unpack_tooling(self):
+        self.assertRaises(ASError, self.csclient.unpack_tooling, 'test_tarball')
+
+    def test_invalid_tar_unpack_tooling(self):
+        tar_file = os.path.join(self.csclient.user_dir, 'invalid_tar')
+        _write_file(tar_file, '')    
+        self.assertRaises(ASErrorInvalidTar, self.csclient.unpack_tooling, tar_file)
+
+    def test_fail_execution_invoke_tooling(self):
+        start_path = os.path.join(self.csclient.user_dir, 'start')
+        _write_file(start_path, '#!/bin/sh\nexit 1', 0744)
+        services = parse_require_config(DUMMY_NO_SERVICE_CONFIG_DATA)
+        self.csclient.invoke_tooling(services)
+
+    def test_user_invoke_tooling(self):
+        start_path = os.path.join(self.csclient.user_dir, 'start')
+        _write_file(start_path, '#!/bin/sh\nexit 0', 0744)
+        services = parse_require_config(DUMMY_NO_SERVICE_CONFIG_DATA)
+        self.csclient.invoke_tooling(services)
+
+    def test_user_service_invoke_tooling(self):
+        service_dir = os.path.join(self.csclient.user_dir, 'jon1')
+        os.mkdir(service_dir)
+        _write_file(os.path.join(service_dir, 'start'), '#!/bin/sh\necho', 0744)
+        services = parse_require_config(DUMMY_SERVICE_CONFIG_DATA)
+        self.csclient.invoke_tooling(services)
+
+class TestAudreyAgentRequiredConfig(unittest.TestCase):
     '''
     Class for exercising the parsing of the Required Configs from the CS.
     '''
@@ -132,20 +220,8 @@ class TestAudreyStartupRequiredConfig(unittest.TestCase):
         Success case:
         - Exercise parse_require_config() with valid input
         '''
-        # Establish valid test data:
-
-        src = '|service|jon1' + \
-            '|parameters|jon_server_ip&' + base64.b64encode('192.168.1.1') + \
-            '|jon_server_ip_2&' + base64.b64encode('192.168.1.2') + \
-            '|jon_server_ip_3&' + base64.b64encode('192.168.1.3') + \
-            '|service|jon2|'
-
-        validation_dict = {'AUDREY_VAR_jon1_jon_server_ip' : '192.168.1.1',
-            'AUDREY_VAR_jon1_jon_server_ip_2' : '192.168.1.2',
-            'AUDREY_VAR_jon1_jon_server_ip_3' : '192.168.1.3' }
-
         # Exersise code segment
-        services = parse_require_config(src)
+        services = parse_require_config(DUMMY_SERVICE_CONFIG_DATA)
 
         # Validate results
         self.assertEqual(services[0].name, 'jon1')
@@ -158,7 +234,7 @@ class TestAudreyStartupRequiredConfig(unittest.TestCase):
                 cmd = ['/usr/bin/printenv', env_var]
                 ret = run_cmd(cmd)
                 self.assertEqual(ret['out'][:-1], \
-                    validation_dict[env_var])
+                    VALIDATE_SERVICE_CONFIG_DATA[env_var])
 
     def test_success_empty_source(self):
         '''
@@ -176,17 +252,7 @@ class TestAudreyStartupRequiredConfig(unittest.TestCase):
         - Exercise parse_require_config() with valid input
         '''
 
-        src = '|service|' + \
-            '|parameters|jon_server_ip&' + base64.b64encode('192.168.1.1') + \
-            '|jon_server_ip_2&' + base64.b64encode('192.168.1.2') + \
-            '|jon_server_ip_3&' + base64.b64encode('192.168.1.3') + \
-            '|service|jon2|'
-
-        validation_dict = {'AUDREY_VAR_jon_server_ip' : '192.168.1.1',
-            'AUDREY_VAR_jon_server_ip_2' : '192.168.1.2',
-            'AUDREY_VAR_jon_server_ip_3' : '192.168.1.3' }
-
-        services = parse_require_config(src)
+        services = parse_require_config(DUMMY_NO_SERVICE_CONFIG_DATA)
         self.assertEqual(services[0].name, '')
         self.assertEqual(services[1].name, 'jon2')
 
@@ -197,7 +263,7 @@ class TestAudreyStartupRequiredConfig(unittest.TestCase):
                 cmd = ['/usr/bin/printenv', env_var]
                 ret = run_cmd(cmd)
                 self.assertEqual(ret['out'][:-1], \
-                    validation_dict[env_var])
+                    VALIDATE_NO_SERVICE_CONFIG_DATA[env_var])
 
     def test_failure_no_services_name(self):
         '''
@@ -252,7 +318,7 @@ class TestAudreyStartupRequiredConfig(unittest.TestCase):
         self.assertRaises(ASError, parse_require_config, src)
 
 
-class TestAudreyStartupProvidesParameters(unittest.TestCase):
+class TestAudreyAgentProvidesParameters(unittest.TestCase):
     '''
     Class for exercising the parsing of the Provides ParametersConfigs
     from the CS.
diff --git a/agent/tests/user_data.py b/agent/tests/user_data.py
index 996f6bd..ea88b16 100644
--- a/agent/tests/user_data.py
+++ b/agent/tests/user_data.py
@@ -24,6 +24,7 @@ import audrey.user_data
 
 from audrey import ASError
 
+from tests import _write_file
 from tests.mocks import mock_run_cmd
 from tests.mocks import mock_run_cmd_modprobe_floppy_fail
 from tests.mocks import mock_run_cmd_mkdir_media_fail
@@ -32,11 +33,6 @@ from tests.mocks import mock_run_cmd_mount_cdrom_fail
 from tests.mocks import DUMMY_USER_DATA
 from tests.mocks import CLOUD_INFO_FILE
 
-def _write_file(filepath, cloud):
-    f = open(filepath, 'w')
-    f.write(cloud)
-    f.close()
-
 class TestAudreyUserData(unittest.TestCase):
     def setUp(self):
         audrey.user_data_ec2.EC2_USER_DATA_URL = 'http://169.254.169.254/latest/user-data'
-- 
1.7.7.6




More information about the aeolus-devel mailing list