diff --git a/armada/tests/unit/utils/test_source.py b/armada/tests/unit/utils/test_source.py index e29b0e41..fd3b36d5 100644 --- a/armada/tests/unit/utils/test_source.py +++ b/armada/tests/unit/utils/test_source.py @@ -13,16 +13,33 @@ # limitations under the License. import os +import socket import shutil +import fixtures import mock -import unittest +import testtools from armada.exceptions import source_exceptions +from armada.tests import test_utils from armada.utils import source -class GitTestCase(unittest.TestCase): +def is_connected(): + """Verifies whether network connectivity is up. + + :returns: True if connected else False. + """ + try: + host = socket.gethostbyname("www.github.com") + socket.create_connection((host, 80), 2) + return True + except: + pass + return False + + +class GitTestCase(testtools.TestCase): def _validate_git_clone(self, repo_dir, expected_ref=None): self.assertTrue(os.path.isdir(repo_dir)) @@ -36,23 +53,32 @@ class GitTestCase(unittest.TestCase): as git_file: self.assertIn(expected_ref, git_file.read()) + @testtools.skipUnless( + is_connected(), 'git clone requires network connectivity.') def test_git_clone_good_url(self): url = 'http://github.com/att-comdev/armada' git_dir = source.git_clone(url) self._validate_git_clone(git_dir) + @testtools.skipUnless( + is_connected(), 'git clone requires network connectivity.') def test_git_clone_commit(self): url = 'http://github.com/att-comdev/armada' commit = 'cba78d1d03e4910f6ab1691bae633c5bddce893d' git_dir = source.git_clone(url, commit) self._validate_git_clone(git_dir) + @testtools.skipUnless( + is_connected(), 'git clone requires network connectivity.') def test_git_clone_ref(self): ref = 'refs/changes/54/457754/73' git_dir = source.git_clone( 'https://github.com/openstack/openstack-helm', ref) self._validate_git_clone(git_dir, ref) + @test_utils.attr(type=['negative']) + @testtools.skipUnless( + is_connected(), 'git clone requires network connectivity.') def test_git_clone_empty_url(self): url = '' error_re = '%s is not a valid git repository.' % url @@ -61,6 +87,9 @@ class GitTestCase(unittest.TestCase): source_exceptions.GitLocationException, error_re): source.git_clone(url) + @test_utils.attr(type=['negative']) + @testtools.skipUnless( + is_connected(), 'git clone requires network connectivity.') def test_git_clone_bad_url(self): url = 'http://github.com/dummy/armada' error_re = '%s is not a valid git repository.' % url @@ -105,36 +134,59 @@ class GitTestCase(unittest.TestCase): mock_tarfile.open.assert_called_once_with(path) mock_opened_file.extractall.assert_called_once_with('/tmp/armada') + @test_utils.attr(type=['negative']) @mock.patch('armada.utils.source.path') @mock.patch('armada.utils.source.tarfile') def test_tarball_extract_bad_path(self, mock_tarfile, mock_path): mock_path.exists.return_value = False path = '/tmp/armada' - with self.assertRaises(source_exceptions.InvalidPathException): - source.extract_tarball(path) + + self.assertRaises(source_exceptions.InvalidPathException, + source.extract_tarball, path) mock_tarfile.open.assert_not_called() mock_tarfile.extractall.assert_not_called() + @testtools.skipUnless( + is_connected(), 'git clone requires network connectivity.') + @mock.patch.object(source, 'LOG') + def test_source_cleanup(self, mock_log): + url = 'http://github.com/att-comdev/armada' + git_path = source.git_clone(url) + source.source_cleanup(git_path) + mock_log.warning.assert_not_called() + + @test_utils.attr(type=['negative']) + @mock.patch.object(source, 'LOG') + @mock.patch('armada.utils.source.shutil') + def test_source_cleanup_fake_git_path(self, mock_shutil, mock_log): + # Verify that passing in a fake path does nothing but log a warning. + # Don't want anyone using the function to delete random directories. + fake_path = self.useFixture(fixtures.TempDir()).path + self.addCleanup(shutil.rmtree, fake_path) + source.source_cleanup(fake_path) + + mock_shutil.rmtree.assert_not_called() + self.assertTrue(mock_log.warning.called) + actual_call = mock_log.warning.mock_calls[0][1][:2] + self.assertEqual( + ('%s is not a valid git repository. Details: %s', fake_path), + actual_call) + + @test_utils.attr(type=['negative']) + @mock.patch.object(source, 'LOG') @mock.patch('armada.utils.source.shutil') @mock.patch('armada.utils.source.path') - def test_source_cleanup(self, mock_path, mock_shutil): - mock_path.exists.return_value = True - path = 'armada' - - try: - source.source_cleanup(path) - except source_exceptions.SourceCleanupException: - pass - - mock_shutil.rmtree.assert_called_with(path) - - @unittest.skip('not handled correctly') - @mock.patch('armada.utils.source.shutil') - @mock.patch('armada.utils.source.path') - def test_source_cleanup_bad_path(self, mock_path, mock_shutil): + def test_source_cleanup_missing_git_path(self, mock_path, mock_shutil, + mock_log): + # Verify that passing in a missing path does nothing but log a warning. mock_path.exists.return_value = False path = 'armada' - with self.assertRaises(source_exceptions.InvalidPathException): - source.source_cleanup(path) + source.source_cleanup(path) + mock_shutil.rmtree.assert_not_called() + self.assertTrue(mock_log.warning.called) + actual_call = mock_log.warning.mock_calls[0][1] + self.assertEqual( + ('Could not delete the path %s. Is it a git repository?', path), + actual_call) diff --git a/armada/utils/source.py b/armada/utils/source.py index d6ea2002..278e1ed6 100644 --- a/armada/utils/source.py +++ b/armada/utils/source.py @@ -18,20 +18,24 @@ import tarfile import tempfile from os import path -import requests +from git import exc as git_exc from git import Git from git import Repo +from oslo_log import log as logging +import requests from requests.packages import urllib3 from armada.exceptions import source_exceptions +LOG = logging.getLogger(__name__) + def git_clone(repo_url, ref='master'): - ''' - :params repo_url - URL of git repo to clone - :params ref - branch, commit or reference in the repo to clone + '''Clone a git repository from ``repo_url`` using the reference ``ref``. - Returns a path to the cloned repo + :params repo_url: URL of git repo to clone. + :params ref: branch, commit or reference in the repo to clone. + :returns: Path to the cloned repo. ''' if repo_url == '': @@ -92,9 +96,23 @@ def extract_tarball(tarball_path): return _tmp_dir -def source_cleanup(target_dir): +def source_cleanup(git_path): + '''Clean up the git repository that was created by ``git_clone`` above. + + Removes the ``git_path`` repository and all associated files if they + exist. + + :param str git_path: The git repository to delete. ''' - Clean up source - ''' - if path.exists(target_dir): - shutil.rmtree(target_dir) + if path.exists(git_path): + try: + # Internally validates whether the path points to an actual repo. + Repo(git_path) + except git_exc.InvalidGitRepositoryError as e: + LOG.warning('%s is not a valid git repository. Details: %s', + git_path, e) + else: + shutil.rmtree(git_path) + else: + LOG.warning('Could not delete the path %s. Is it a git repository?', + git_path) diff --git a/tox.ini b/tox.ini index e1977697..e026c1bc 100644 --- a/tox.ini +++ b/tox.ini @@ -51,6 +51,6 @@ commands = --cov=armada [flake8] -filename= *.py +filename = *.py ignore = E722 -exclude= .git, .idea, .tox, *.egg-info, *.eggs, bin, dist, hapi, docs/*, build/* +exclude = .git,.tox,dist,*lib/python*,*egg,build,releasenotes,docs/*,hapi,venv