summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFelipe Monteiro <felipe.monteiro@att.com>2018-10-23 12:06:08 -0400
committerFelipe Monteiro <felipe.monteiro@att.com>2018-10-29 15:13:23 +0000
commitb50619b06df82a426d431e6bbe7a384bf1312146 (patch)
tree78d9a397fde1b7f3794a24ac2abc63fa8cc200b3
parent77201c4f337688d2a40a608de4e7356b2891f4f2 (diff)
refactor: Use temp path fixture to automatically clean up
This patch set refactors uses of tempfile.mkdtemp() used throughout many tests in Pegleg which leaves lingering temporary directories around. Recently a fixture was introduced in [0] which automatically cleans up after itself. This patch set applies the fixture everywhere possible to minimize the testing footprint. [0] https://review.openstack.org/#/c/609818/20/tests/unit/fixtures.py Change-Id: Id4c1195c4f248b974a5396a429d651943a84ee83
Notes
Notes (review): Code-Review+2: Craig Anderson <craig.anderson@att.com> Workflow+1: Craig Anderson <craig.anderson@att.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Mon, 29 Oct 2018 18:32:25 +0000 Reviewed-on: https://review.openstack.org/612738 Project: openstack/airship-pegleg Branch: refs/heads/master
-rw-r--r--tests/unit/conftest.py2
-rw-r--r--tests/unit/engine/test_util_files.py2
-rw-r--r--tests/unit/engine/util/test_git.py7
-rw-r--r--tests/unit/fixtures.py2
-rw-r--r--tests/unit/test_cli.py51
5 files changed, 25 insertions, 39 deletions
diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py
index fc0b171..6dab893 100644
--- a/tests/unit/conftest.py
+++ b/tests/unit/conftest.py
@@ -35,7 +35,7 @@ def restore_config():
35 config.GLOBAL_CONTEXT = original_global_context 35 config.GLOBAL_CONTEXT = original_global_context
36 36
37 37
38@pytest.fixture(scope="class", autouse=True) 38@pytest.fixture(scope="module", autouse=True)
39def clean_temporary_git_repos(): 39def clean_temporary_git_repos():
40 """Iterates through all temporarily created directories and deletes each 40 """Iterates through all temporarily created directories and deletes each
41 one that was created for testing. 41 one that was created for testing.
diff --git a/tests/unit/engine/test_util_files.py b/tests/unit/engine/test_util_files.py
index ea4fffd..bf18a8f 100644
--- a/tests/unit/engine/test_util_files.py
+++ b/tests/unit/engine/test_util_files.py
@@ -12,8 +12,6 @@
12# See the License for the specific language governing permissions and 12# See the License for the specific language governing permissions and
13# limitations under the License. 13# limitations under the License.
14 14
15import tempfile
16
17from pegleg import config 15from pegleg import config
18from pegleg.engine.util import files 16from pegleg.engine.util import files
19from tests.unit.fixtures import create_tmp_deployment_files 17from tests.unit.fixtures import create_tmp_deployment_files
diff --git a/tests/unit/engine/util/test_git.py b/tests/unit/engine/util/test_git.py
index e732814..d86d076 100644
--- a/tests/unit/engine/util/test_git.py
+++ b/tests/unit/engine/util/test_git.py
@@ -14,15 +14,14 @@
14 14
15import os 15import os
16import shutil 16import shutil
17import tempfile
18 17
19import fixtures
20from git import Repo 18from git import Repo
21import mock 19import mock
22import pytest 20import pytest
23 21
24from pegleg.engine import exceptions 22from pegleg.engine import exceptions
25from pegleg.engine.util import git 23from pegleg.engine.util import git
24from tests.unit.fixtures import temp_path
26from tests.unit import test_utils 25from tests.unit import test_utils
27 26
28 27
@@ -509,8 +508,8 @@ def test_is_repository():
509 subpath='deployment_files/site') 508 subpath='deployment_files/site')
510 509
511 510
512def test_is_repository_negative(): 511def test_is_repository_negative(temp_path):
513 assert not git.is_repository(tempfile.mkdtemp()) 512 assert not git.is_repository(temp_path)
514 513
515 514
516@pytest.mark.skipif( 515@pytest.mark.skipif(
diff --git a/tests/unit/fixtures.py b/tests/unit/fixtures.py
index e9b0c60..b47c4e0 100644
--- a/tests/unit/fixtures.py
+++ b/tests/unit/fixtures.py
@@ -158,7 +158,7 @@ schema: pegleg/SiteDefinition/v1
158 158
159 159
160@pytest.fixture() 160@pytest.fixture()
161def temp_clone_path(): 161def temp_path():
162 temp_folder = tempfile.mkdtemp() 162 temp_folder = tempfile.mkdtemp()
163 try: 163 try:
164 yield temp_folder 164 yield temp_folder
diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py
index 79a6ff6..4f73054 100644
--- a/tests/unit/test_cli.py
+++ b/tests/unit/test_cli.py
@@ -14,7 +14,6 @@
14 14
15import os 15import os
16import shutil 16import shutil
17import tempfile
18 17
19from click.testing import CliRunner 18from click.testing import CliRunner
20import mock 19import mock
@@ -24,7 +23,7 @@ from pegleg import cli
24from pegleg.engine import errorcodes 23from pegleg.engine import errorcodes
25from pegleg.engine.util import git 24from pegleg.engine.util import git
26from tests.unit import test_utils 25from tests.unit import test_utils
27from tests.unit.fixtures import temp_clone_path 26from tests.unit.fixtures import temp_path
28 27
29 28
30@pytest.mark.skipif( 29@pytest.mark.skipif(
@@ -65,7 +64,7 @@ class TestSiteCLIOptions(BaseCLIActionTest):
65 ### clone_path tests ### 64 ### clone_path tests ###
66 65
67 def test_list_sites_using_remote_repo_and_clone_path_option( 66 def test_list_sites_using_remote_repo_and_clone_path_option(
68 self, temp_clone_path): 67 self, temp_path):
69 """Validates clone_path (-p) option is working properly with site list 68 """Validates clone_path (-p) option is working properly with site list
70 action when using remote repo. Verify that the repo was cloned in the 69 action when using remote repo. Verify that the repo was cloned in the
71 clone_path 70 clone_path
@@ -80,15 +79,15 @@ class TestSiteCLIOptions(BaseCLIActionTest):
80 79
81 # Note that the -p option is used to specify the clone_folder 80 # Note that the -p option is used to specify the clone_folder
82 site_list = self.runner.invoke( 81 site_list = self.runner.invoke(
83 cli.site, ['-p', temp_clone_path, '-r', repo_url, 'list']) 82 cli.site, ['-p', temp_path, '-r', repo_url, 'list'])
84 83
85 assert site_list.exit_code == 0 84 assert site_list.exit_code == 0
86 # Verify that the repo was cloned into the clone_path 85 # Verify that the repo was cloned into the clone_path
87 assert os.path.exists(os.path.join(temp_clone_path, self.repo_name)) 86 assert os.path.exists(os.path.join(temp_path, self.repo_name))
88 assert git.is_repository(os.path.join(temp_clone_path, self.repo_name)) 87 assert git.is_repository(os.path.join(temp_path, self.repo_name))
89 88
90 def test_list_sites_using_local_repo_and_clone_path_option( 89 def test_list_sites_using_local_repo_and_clone_path_option(
91 self, temp_clone_path): 90 self, temp_path):
92 """Validates clone_path (-p) option is working properly with site list 91 """Validates clone_path (-p) option is working properly with site list
93 action when using a local repo. Verify that the clone_path has NO 92 action when using a local repo. Verify that the clone_path has NO
94 effect when using a local repo 93 effect when using a local repo
@@ -102,12 +101,11 @@ class TestSiteCLIOptions(BaseCLIActionTest):
102 101
103 # Note that the -p option is used to specify the clone_folder 102 # Note that the -p option is used to specify the clone_folder
104 site_list = self.runner.invoke( 103 site_list = self.runner.invoke(
105 cli.site, ['-p', temp_clone_path, '-r', repo_path, 'list']) 104 cli.site, ['-p', temp_path, '-r', repo_path, 'list'])
106 105
107 assert site_list.exit_code == 0 106 assert site_list.exit_code == 0
108 # Verify that passing in clone_path when using local repo has no effect 107 # Verify that passing in clone_path when using local repo has no effect
109 assert not os.path.exists( 108 assert not os.path.exists(os.path.join(temp_path, self.repo_name))
110 os.path.join(temp_clone_path, self.repo_name))
111 109
112 110
113class TestSiteCLIOptionsNegative(BaseCLIActionTest): 111class TestSiteCLIOptionsNegative(BaseCLIActionTest):
@@ -116,7 +114,7 @@ class TestSiteCLIOptionsNegative(BaseCLIActionTest):
116 ### Negative clone_path tests ### 114 ### Negative clone_path tests ###
117 115
118 def test_list_sites_using_remote_repo_and_reuse_clone_path_option( 116 def test_list_sites_using_remote_repo_and_reuse_clone_path_option(
119 self, temp_clone_path): 117 self, temp_path):
120 """Validates clone_path (-p) option is working properly with site list 118 """Validates clone_path (-p) option is working properly with site list
121 action when using remote repo. Verify that the same repo can't be 119 action when using remote repo. Verify that the same repo can't be
122 cloned in the same clone_path if it already exists 120 cloned in the same clone_path if it already exists
@@ -131,14 +129,14 @@ class TestSiteCLIOptionsNegative(BaseCLIActionTest):
131 129
132 # Note that the -p option is used to specify the clone_folder 130 # Note that the -p option is used to specify the clone_folder
133 site_list = self.runner.invoke( 131 site_list = self.runner.invoke(
134 cli.site, ['-p', temp_clone_path, '-r', repo_url, 'list']) 132 cli.site, ['-p', temp_path, '-r', repo_url, 'list'])
135 133
136 assert git.is_repository(os.path.join(temp_clone_path, self.repo_name)) 134 assert git.is_repository(os.path.join(temp_path, self.repo_name))
137 135
138 # Run site list for a second time to validate that the repo can't be 136 # Run site list for a second time to validate that the repo can't be
139 # cloned twice in the same clone_path 137 # cloned twice in the same clone_path
140 site_list = self.runner.invoke( 138 site_list = self.runner.invoke(
141 cli.site, ['-p', temp_clone_path, '-r', repo_url, 'list']) 139 cli.site, ['-p', temp_path, '-r', repo_url, 'list'])
142 140
143 assert site_list.exit_code == 1 141 assert site_list.exit_code == 1
144 msg = "The repository already exists in the given path. Either " \ 142 msg = "The repository already exists in the given path. Either " \
@@ -146,23 +144,13 @@ class TestSiteCLIOptionsNegative(BaseCLIActionTest):
146 "repository as the site repository (-r)." 144 "repository as the site repository (-r)."
147 assert msg in site_list.output 145 assert msg in site_list.output
148 146
149 @classmethod
150 def teardown_class(cls):
151 # Cleanup temporary Git repos.
152 root_tempdir = tempfile.gettempdir()
153 for tempdir in os.listdir(root_tempdir):
154 path = os.path.join(root_tempdir, tempdir)
155 if git.is_repository(path):
156 shutil.rmtree(path, ignore_errors=True)
157
158 147
159class TestSiteCliActions(BaseCLIActionTest): 148class TestSiteCliActions(BaseCLIActionTest):
160 """Tests site-level CLI actions.""" 149 """Tests site-level CLI actions."""
161 150
162 ### Collect tests ### 151 ### Collect tests ###
163 152
164 def _validate_collect_site_action(self, repo_path_or_url): 153 def _validate_collect_site_action(self, repo_path_or_url, save_location):
165 save_location = tempfile.mkdtemp()
166 result = self.runner.invoke(cli.site, [ 154 result = self.runner.invoke(cli.site, [
167 '-r', repo_path_or_url, 'collect', self.site_name, '-s', 155 '-r', repo_path_or_url, 'collect', self.site_name, '-s',
168 save_location 156 save_location
@@ -176,7 +164,7 @@ class TestSiteCliActions(BaseCLIActionTest):
176 # are written out to sensibly named files like airship-treasuremap.yaml 164 # are written out to sensibly named files like airship-treasuremap.yaml
177 assert collected_files[0] == ("%s.yaml" % self.repo_name) 165 assert collected_files[0] == ("%s.yaml" % self.repo_name)
178 166
179 def test_collect_using_remote_repo_url(self): 167 def test_collect_using_remote_repo_url(self, temp_path):
180 """Validates collect action using a remote URL.""" 168 """Validates collect action using a remote URL."""
181 # Scenario: 169 # Scenario:
182 # 170 #
@@ -186,9 +174,10 @@ class TestSiteCliActions(BaseCLIActionTest):
186 174
187 repo_url = 'https://github.com/openstack/%s@%s' % (self.repo_name, 175 repo_url = 'https://github.com/openstack/%s@%s' % (self.repo_name,
188 self.repo_rev) 176 self.repo_rev)
189 self._validate_collect_site_action(repo_url) 177 self._validate_collect_site_action(repo_url, temp_path)
190 178
191 def test_collect_using_remote_repo_url_ending_with_dot_git(self): 179 def test_collect_using_remote_repo_url_ending_with_dot_git(
180 self, temp_path):
192 """Validates collect action using a remote URL ending in .git.""" 181 """Validates collect action using a remote URL ending in .git."""
193 # Scenario: 182 # Scenario:
194 # 183 #
@@ -198,9 +187,9 @@ class TestSiteCliActions(BaseCLIActionTest):
198 187
199 repo_url = 'https://github.com/openstack/%s@%s.git' % (self.repo_name, 188 repo_url = 'https://github.com/openstack/%s@%s.git' % (self.repo_name,
200 self.repo_rev) 189 self.repo_rev)
201 self._validate_collect_site_action(repo_url) 190 self._validate_collect_site_action(repo_url, temp_path)
202 191
203 def test_collect_using_local_path(self): 192 def test_collect_using_local_path(self, temp_path):
204 """Validates collect action using a path to a local repo.""" 193 """Validates collect action using a path to a local repo."""
205 # Scenario: 194 # Scenario:
206 # 195 #
@@ -209,7 +198,7 @@ class TestSiteCliActions(BaseCLIActionTest):
209 # 3) Check that expected file name is there 198 # 3) Check that expected file name is there
210 199
211 repo_path = self.treasuremap_path 200 repo_path = self.treasuremap_path
212 self._validate_collect_site_action(repo_path) 201 self._validate_collect_site_action(repo_path, temp_path)
213 202
214 ### Lint tests ### 203 ### Lint tests ###
215 204