Repository navigation
Increase test coverage for filecmp
#159663
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
base: main
Are you sure you want to change the base?
Changes from all commits
6441227
78aad17
f781d4c
0062674
70fd4c8
7e44c1f
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 | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,9 +4,12 @@ | |||||||||||||||
| import shutil | ||||||||||||||||
| import tempfile | ||||||||||||||||
| import unittest | ||||||||||||||||
| import stat | ||||||||||||||||
| import io | ||||||||||||||||
|
|
||||||||||||||||
| from test import support | ||||||||||||||||
| from test.support import os_helper | ||||||||||||||||
| from unittest.mock import patch | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def _create_file_shallow_equal(template_path, new_path): | ||||||||||||||||
|
|
@@ -76,6 +79,22 @@ def test_cache_clear(self): | |||||||||||||||
| self.assertTrue(len(filecmp._cache) == 0, | ||||||||||||||||
| "Cache not cleared after calling clear_cache") | ||||||||||||||||
|
|
||||||||||||||||
| def test_cache_overflow_and_hit(self): | ||||||||||||||||
| filecmp.clear_cache() | ||||||||||||||||
| s1 = filecmp._sig(os.stat(self.name)) | ||||||||||||||||
| s2 = filecmp._sig(os.stat(self.name_same)) | ||||||||||||||||
| for i in range(105): | ||||||||||||||||
| fake_path2 = f"fake_path_{i}" | ||||||||||||||||
| filecmp._cache[self.name, fake_path2, s1, s2] = True | ||||||||||||||||
|
|
||||||||||||||||
| self.assertGreater(len(filecmp._cache), 100) | ||||||||||||||||
| res1 = filecmp.cmp(self.name, self.name_same, shallow=False) | ||||||||||||||||
| self.assertTrue(res1) | ||||||||||||||||
| self.assertEqual(len(filecmp._cache), 1) | ||||||||||||||||
|
|
||||||||||||||||
| res2 = filecmp.cmp(self.name, self.name_same, shallow=False) | ||||||||||||||||
| self.assertTrue(res2) | ||||||||||||||||
|
|
||||||||||||||||
| class DirCompareTestCase(unittest.TestCase): | ||||||||||||||||
|
Comment on lines
+96
to
98
Member
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.
Suggested change
|
||||||||||||||||
| def setUp(self): | ||||||||||||||||
| tmpdir = tempfile.gettempdir() | ||||||||||||||||
|
|
@@ -367,5 +386,149 @@ def _assert_report(self, dircmp_report, expected_report_lines): | |||||||||||||||
| self.assertEqual(report_lines, expected_report_lines) | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| class DirCmpImplementationTests(unittest.TestCase): | ||||||||||||||||
| def setUp(self): | ||||||||||||||||
| tmpdir = tempfile.gettempdir() | ||||||||||||||||
|
Member
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. For the the tempdir, I suggest that you use |
||||||||||||||||
| self.base_test_dir = os.path.join(tmpdir, 'test_filecmp_working_dir') | ||||||||||||||||
| os.makedirs(self.base_test_dir, exist_ok=True) | ||||||||||||||||
| self.addCleanup(shutil.rmtree, self.base_test_dir, ignore_errors=True) | ||||||||||||||||
|
|
||||||||||||||||
| self.dir = os.path.join(self.base_test_dir, 'dir') | ||||||||||||||||
| self.dir_same = os.path.join(self.base_test_dir, 'dir-same') | ||||||||||||||||
| self.dir_diff = os.path.join(self.base_test_dir, 'dir-diff') | ||||||||||||||||
| self.dir_diff_file = os.path.join(self.base_test_dir, 'dir-diff-file') | ||||||||||||||||
| self.dir_same_shallow = os.path.join( | ||||||||||||||||
| self.base_test_dir, 'dir-same-shallow' | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| self.dir_ignored = os.path.join(self.dir_same, '.hg') | ||||||||||||||||
| self.caseinsensitive = os.path.normcase('A') == os.path.normcase('a') | ||||||||||||||||
| data = 'Contents of file go here.\n' | ||||||||||||||||
|
|
||||||||||||||||
| os.mkdir(self.dir) | ||||||||||||||||
| subdir_path = os.path.join(self.dir, 'subdir') | ||||||||||||||||
| os.mkdir(subdir_path) | ||||||||||||||||
| dir_file_path = os.path.join(self.dir, "file") | ||||||||||||||||
| with open(dir_file_path, 'w', encoding="utf-8") as output: | ||||||||||||||||
|
Member
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. To reduce all those checks, can you acutlaly use |
||||||||||||||||
| output.write(data) | ||||||||||||||||
|
|
||||||||||||||||
| for d in (self.dir_same, self.dir_same_shallow, | ||||||||||||||||
| self.dir_diff, self.dir_diff_file): | ||||||||||||||||
| os.mkdir(d) | ||||||||||||||||
| subdir_path = os.path.join(d, 'subdir') | ||||||||||||||||
| os.mkdir(subdir_path) | ||||||||||||||||
| fn = ('FiLe' if (self.caseinsensitive and d is self.dir_same) | ||||||||||||||||
| else 'file') | ||||||||||||||||
| file_path = os.path.join(d, fn) | ||||||||||||||||
|
|
||||||||||||||||
| if d is self.dir_same_shallow: | ||||||||||||||||
| _create_file_shallow_equal(dir_file_path, file_path) | ||||||||||||||||
| else: | ||||||||||||||||
| shutil.copy2(dir_file_path, file_path) | ||||||||||||||||
|
|
||||||||||||||||
| with open(os.path.join(self.dir_diff, 'file2'), 'w', | ||||||||||||||||
| encoding="utf-8") as output: | ||||||||||||||||
| output.write('An extra file.\n') | ||||||||||||||||
|
|
||||||||||||||||
| with open(os.path.join(self.dir_diff_file, 'file2'), 'w', | ||||||||||||||||
| encoding="utf-8") as output: | ||||||||||||||||
| output.write('Different contents.\n') | ||||||||||||||||
|
|
||||||||||||||||
| def test_dircmp_os_errors_in_processing(self): | ||||||||||||||||
|
Member
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. For each test, please add a comment (non-docstring) explaing the filesystem layout if possible. It's a bit hard to see what happens exactly. |
||||||||||||||||
| trigger_file = "error_trigger_file.txt" | ||||||||||||||||
| for d in (self.dir, self.dir_same): | ||||||||||||||||
| with open(os.path.join(d, trigger_file), "w", | ||||||||||||||||
| encoding="utf-8") as f: | ||||||||||||||||
| f.write("trigger") | ||||||||||||||||
|
|
||||||||||||||||
| original_stat = os.stat | ||||||||||||||||
|
|
||||||||||||||||
| def fake_stat(path, *args, **kwargs): | ||||||||||||||||
| if trigger_file in str(path): | ||||||||||||||||
| raise OSError | ||||||||||||||||
| return original_stat(path, *args, **kwargs) | ||||||||||||||||
|
|
||||||||||||||||
| with patch("os.stat", side_effect=fake_stat): | ||||||||||||||||
| dc = filecmp.dircmp(self.dir, self.dir_same) | ||||||||||||||||
| self.assertIn(trigger_file, dc.common_funny) | ||||||||||||||||
|
|
||||||||||||||||
| def test_dircmp_different_types_and_funny_types(self): | ||||||||||||||||
| file_diff_type = "diff_type.txt" | ||||||||||||||||
| file_funny_type = "funny_type.txt" | ||||||||||||||||
|
|
||||||||||||||||
| for f_name in (file_diff_type, file_funny_type): | ||||||||||||||||
| for d in (self.dir, self.dir_same): | ||||||||||||||||
| with open(os.path.join(d, f_name), "w", | ||||||||||||||||
| encoding="utf-8") as f: | ||||||||||||||||
| f.write("data") | ||||||||||||||||
|
|
||||||||||||||||
| original_stat = os.stat | ||||||||||||||||
|
|
||||||||||||||||
| def fake_stat_types(path, *args, **kwargs): | ||||||||||||||||
| st = original_stat(path, *args, **kwargs) | ||||||||||||||||
| path_str = str(path) | ||||||||||||||||
|
|
||||||||||||||||
| if file_diff_type in path_str: | ||||||||||||||||
| is_dir_same = "dir-same" in path_str | ||||||||||||||||
| target_type = stat.S_IFREG if is_dir_same else stat.S_IFDIR | ||||||||||||||||
| new_mode = (st.st_mode & ~stat.S_IFREG) | target_type | ||||||||||||||||
| return os.stat_result((new_mode,) + st[1:]) | ||||||||||||||||
|
|
||||||||||||||||
| if file_funny_type in path_str: | ||||||||||||||||
| new_mode = (st.st_mode & ~stat.S_IFREG) | stat.S_IFSOCK | ||||||||||||||||
| return os.stat_result((new_mode,) + st[1:]) | ||||||||||||||||
|
|
||||||||||||||||
| return st | ||||||||||||||||
|
|
||||||||||||||||
| with patch("os.stat", side_effect=fake_stat_types): | ||||||||||||||||
| dc = filecmp.dircmp(self.dir, self.dir_same) | ||||||||||||||||
| self.assertIn(file_diff_type, dc.common_funny) | ||||||||||||||||
| self.assertIn(file_funny_type, dc.common_funny) | ||||||||||||||||
|
|
||||||||||||||||
| def test_dircmp_closure_and_report_funnies(self): | ||||||||||||||||
| deep_dir_1 = os.path.join(self.dir, "subdir", "deep_folder") | ||||||||||||||||
| deep_dir_2 = os.path.join(self.dir_same, "subdir", "deep_folder") | ||||||||||||||||
| os.mkdir(deep_dir_1) | ||||||||||||||||
| os.mkdir(deep_dir_2) | ||||||||||||||||
|
|
||||||||||||||||
| funny_trigger = "funny_report_file.txt" | ||||||||||||||||
| funny_file_trigger = "funny_file_trigger.txt" | ||||||||||||||||
|
|
||||||||||||||||
| for f_name in (funny_trigger, funny_file_trigger): | ||||||||||||||||
| for d in (self.dir, self.dir_same): | ||||||||||||||||
| with open(os.path.join(d, f_name), "w", | ||||||||||||||||
| encoding="utf-8") as f: | ||||||||||||||||
| f.write("data") | ||||||||||||||||
|
|
||||||||||||||||
| original_stat = os.stat | ||||||||||||||||
| original_cmp = filecmp._cmp | ||||||||||||||||
|
|
||||||||||||||||
| def fake_stat_for_report(path, *args, **kwargs): | ||||||||||||||||
| if funny_trigger in str(path): | ||||||||||||||||
| st = original_stat(path, *args, **kwargs) | ||||||||||||||||
| new_mode = (st.st_mode & ~stat.S_IFREG) | stat.S_IFSOCK | ||||||||||||||||
| return os.stat_result((new_mode,) + st[1:]) | ||||||||||||||||
| return original_stat(path, *args, **kwargs) | ||||||||||||||||
|
|
||||||||||||||||
| def fake_private_cmp(a, b, sh): | ||||||||||||||||
| if funny_file_trigger in str(a) or funny_file_trigger in str(b): | ||||||||||||||||
| return 2 | ||||||||||||||||
| return original_cmp(a, b, sh) | ||||||||||||||||
|
|
||||||||||||||||
| with patch("os.stat", side_effect=fake_stat_for_report), \ | ||||||||||||||||
| patch("filecmp._cmp", side_effect=fake_private_cmp): | ||||||||||||||||
|
Comment on lines
+518
to
+519
Member
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.
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
| dc = filecmp.dircmp(self.dir, self.dir_same) | ||||||||||||||||
| _ = dc.subdirs | ||||||||||||||||
| dc.phase4_closure() | ||||||||||||||||
|
|
||||||||||||||||
| with patch("sys.stdout", new=io.StringIO()) as mock_stdout: | ||||||||||||||||
|
Member
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. Use support.captured_stdout() instead here. |
||||||||||||||||
| dc.report() | ||||||||||||||||
| output = mock_stdout.getvalue() | ||||||||||||||||
|
Member
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. And this one as well |
||||||||||||||||
|
|
||||||||||||||||
| self.assertIn("Common funny cases", output) | ||||||||||||||||
| self.assertIn("Trouble with common files", output) | ||||||||||||||||
|
Comment on lines
+529
to
+530
Member
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. Leave those assertions out of the context. |
||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| if __name__ == "__main__": | ||||||||||||||||
| unittest.main() | ||||||||||||||||
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.
Pleae keep the list of imports sorted with the existing ones.