Skip to content

Commit 4a7e628

Browse files
albu-dikujonasbardino
authored andcommitted
Remove all cleverness from test filesystem cleanup; make it unconditional.
1 parent 9ae2f3a commit 4a7e628

File tree

2 files changed

+40
-27
lines changed

2 files changed

+40
-27
lines changed

tests/support/__init__.py

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,26 @@ def write(self, message):
126126
logging.captureWarnings(True)
127127

128128

129+
def _empty_dir(absolute_dir):
130+
assert os.path.isabs(absolute_dir)
131+
132+
for item in os.listdir(absolute_dir):
133+
path = os.path.join(absolute_dir, item)
134+
135+
if os.path.islink(path):
136+
os.remove(path)
137+
elif os.path.isdir(path):
138+
try:
139+
shutil.rmtree(path)
140+
except Exception as exc:
141+
print(path)
142+
raise
143+
elif os.path.exists(path):
144+
os.remove(path)
145+
else:
146+
pass
147+
148+
129149
class MigTestCase(TestCase):
130150
"""Embellished base class for MiG test cases. Provides additional commonly
131151
used assertions as well as some basics for the standardised and idiomatic
@@ -139,7 +159,6 @@ class MigTestCase(TestCase):
139159
def __init__(self, *args):
140160
super(MigTestCase, self).__init__(*args)
141161
self._cleanup_checks = list()
142-
self._cleanup_paths = set()
143162
self._configuration = None
144163
self._logger = None
145164
self._skip_logging = False
@@ -148,6 +167,12 @@ def setUp(self):
148167
"""Init before tests"""
149168
if not self._skip_logging:
150169
self._reset_logging(stream=self.logger)
170+
171+
# clear the output directory uncondtionally to enforce tests always
172+
# run in isolation - if a file must be there for a test to exercise
173+
# a condition it is the responbility of the test case to arrange it
174+
_empty_dir(TEST_OUTPUT_DIR)
175+
151176
self.before_each()
152177

153178
def tearDown(self):
@@ -164,19 +189,14 @@ def tearDown(self):
164189
if self._logger is not None:
165190
self._reset_logging(stream=BLACKHOLE_STREAM)
166191

167-
for path in self._cleanup_paths:
168-
if os.path.islink(path):
169-
os.remove(path)
170-
elif os.path.isdir(path):
171-
try:
172-
shutil.rmtree(path)
173-
except Exception as exc:
174-
print(path)
175-
raise
176-
elif os.path.exists(path):
177-
os.remove(path)
178-
else:
179-
continue
192+
@classmethod
193+
def tearDownClass(cls):
194+
if MIG_ENV == 'docker':
195+
# the permissions story wrt running inside docker containers is
196+
# such that we can end up with files from previous test runs left
197+
# around that might subsequently cause spurious permissions errors
198+
# once running locally again, so opt to simply avoid this
199+
_empty_dir(TEST_OUTPUT_DIR)
180200

181201
# hooks
182202
def after_each(self):
@@ -190,11 +210,6 @@ def before_each(self):
190210
def _register_check(self, check_callable):
191211
self._cleanup_checks.append(check_callable)
192212

193-
def _register_path(self, cleanup_path):
194-
assert os.path.isabs(cleanup_path)
195-
self._cleanup_paths.add(cleanup_path)
196-
return cleanup_path
197-
198213
def _reset_logging(self, stream):
199214
root_logger = logging.getLogger()
200215
root_handler = root_logger.handlers[0]
@@ -230,10 +245,10 @@ def configuration(self):
230245
if configuration_to_make == 'testconfig':
231246
# use the paths defined by the loaded configuration to create
232247
# the directories which are expected to be present by the code
233-
os.mkdir(self._register_path(configuration_instance.certs_path))
234-
os.mkdir(self._register_path(configuration_instance.state_path))
248+
os.mkdir(configuration_instance.certs_path)
249+
os.mkdir(configuration_instance.state_path)
235250
log_path = os.path.join(configuration_instance.state_path, "log")
236-
os.mkdir(self._register_path(log_path))
251+
os.mkdir(log_path)
237252

238253
self._configuration = configuration_instance
239254

@@ -525,7 +540,7 @@ def fixturepath(relative_path):
525540
return tmp_path
526541

527542

528-
def temppath(relative_path, test_case, ensure_dir=False, skip_clean=False):
543+
def temppath(relative_path, test_case, ensure_dir=False):
529544
"""Register relative_path as a temp path and schedule automatic clean up
530545
after unit tests unless skip_clean is set. Anchors the temp path in
531546
internal test output dir unless skip_output_anchor is set. Returns
@@ -557,8 +572,6 @@ def temppath(relative_path, test_case, ensure_dir=False, skip_clean=False):
557572
if oserr.errno == errno.EEXIST:
558573
raise AssertionError(
559574
"ABORT: use of unclean output path: %s" % tmp_path)
560-
if not skip_clean:
561-
test_case._cleanup_paths.add(tmp_path)
562575
return tmp_path
563576

564577

tests/test_mig_shared_fileio.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class MigSharedFileio__write_chunk(MigTestCase):
5454
# TODO: Add docstrings to this class and its methods
5555
def setUp(self):
5656
super(MigSharedFileio__write_chunk, self).setUp()
57-
self.tmp_path = temppath(DUMMY_FILE_WRITECHUNK, self, skip_clean=True)
57+
self.tmp_path = temppath(DUMMY_FILE_WRITECHUNK, self)
5858
cleanpath(os.path.dirname(DUMMY_FILE_WRITECHUNK), self)
5959

6060
def test_return_false_on_invalid_data(self):
@@ -140,7 +140,7 @@ def test_store_unicode_in_binary_mode(self):
140140
class MigSharedFileio__write_file(MigTestCase):
141141
def setUp(self):
142142
super(MigSharedFileio__write_file, self).setUp()
143-
self.tmp_path = temppath(DUMMY_FILE_WRITEFILE, self, skip_clean=True)
143+
self.tmp_path = temppath(DUMMY_FILE_WRITEFILE, self)
144144
cleanpath(os.path.dirname(DUMMY_FILE_WRITEFILE), self)
145145

146146
def test_return_false_on_invalid_data(self):

0 commit comments

Comments
 (0)