New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix issue #285 - Unexpected targets' cleanup order #295
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -374,8 +374,11 @@ class TestTaskClean(object): | |
def tmpdir(self, request): | ||
tmpdir = {} | ||
tmpdir['dir'] = tempfile.mkdtemp(prefix='doit-') | ||
tmpdir['subdir'] = tempfile.mkdtemp(dir=tmpdir['dir']) | ||
files = [os.path.join(tmpdir['dir'], fname) | ||
for fname in ['a.txt', 'b.txt']] | ||
for fname in ['a.txt', | ||
'b.txt', | ||
os.path.join(tmpdir['subdir'], 'c.txt')]] | ||
tmpdir['files'] = files | ||
# create empty files | ||
for filename in tmpdir['files']: | ||
|
@@ -412,14 +415,15 @@ def test_clean_non_existent_targets(self): | |
|
||
def test_clean_empty_dirs(self, tmpdir): | ||
# Remove empty directories listed in targets | ||
targets = tmpdir['files'] + [tmpdir['dir']] | ||
targets = tmpdir['files'] + [tmpdir['subdir']] | ||
t = task.Task("xxx", None, targets=targets, clean=True) | ||
assert True == t._remove_targets | ||
assert 0 == len(t.clean_actions) | ||
t.clean(StringIO(), False) | ||
for filename in tmpdir['files']: | ||
assert not os.path.exists(filename) | ||
assert not os.path.exists(tmpdir['dir']) | ||
assert not os.path.exists(tmpdir['subdir']) | ||
assert os.path.exists(tmpdir['dir']) | ||
|
||
def test_keep_non_empty_dirs(self, tmpdir): | ||
# Keep non empty directories listed in targets | ||
|
@@ -433,6 +437,19 @@ def test_keep_non_empty_dirs(self, tmpdir): | |
assert expected == os.path.exists(filename) | ||
assert os.path.exists(tmpdir['dir']) | ||
|
||
def test_clean_in_reverse_lexical_order(self, tmpdir): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test name is misleading because it actually does not ensure reverse lexical order. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. I'm having a bit of a tough time finding a proper name though. An option I considered is to actually do ensure that the order is that one, by patching both |
||
# Remove targets in reverse lexical order so that subdirectories' order | ||
# in the targets array is irrelevant | ||
targets = tmpdir['files'] + [tmpdir['dir'], tmpdir['subdir']] | ||
t = task.Task("xxx", None, targets=targets, clean=True) | ||
assert True == t._remove_targets | ||
assert 0 == len(t.clean_actions) | ||
t.clean(StringIO(), False) | ||
for filename in tmpdir['files']: | ||
assert not os.path.exists(filename) | ||
assert not os.path.exists(tmpdir['dir']) | ||
assert not os.path.exists(tmpdir['subdir']) | ||
|
||
def test_clean_actions(self, tmpdir): | ||
# a clean action can be anything, it can even not clean anything! | ||
c_path = tmpdir['files'][0] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is DFS? "Distributed File System"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's Depth First Search in this case. I didn't mean to be pedantic.
Thanks for merging!