bpo-15450: Allow subclassing of dircmp (pythonGH-23424) (python#23424)

NickCrews · cjerdonek · web-flow · commit 2f2f9d0b5c96 · 2020-11-23T18:29:37.000+02:00
Co-authored-by: Chris Jerdonek &lt;chris.jerdonek@gmail.com&gt;
diff --git a/Doc/library/filecmp.rst b/Doc/library/filecmp.rst
@@ -173,7 +173,13 @@ The :class:`dircmp` class
    .. attribute:: subdirs
 
       A dictionary mapping names in :attr:`common_dirs` to :class:`dircmp`
-      objects.
+      instances (or MyDirCmp instances if this instance is of type MyDirCmp, a
+      subclass of :class:`dircmp`).
+
+      .. versionchanged:: 3.10
+         Previously entries were always :class:`dircmp` instances. Now entries
+         are the same type as *self*, if *self* is a subclass of
+         :class:`dircmp`.
 
 .. attribute:: DEFAULT_IGNORES
 
diff --git a/Lib/filecmp.py b/Lib/filecmp.py
@@ -115,7 +115,9 @@ class dircmp:
      same_files: list of identical files.
      diff_files: list of filenames which differ.
      funny_files: list of files which could not be compared.
-     subdirs: a dictionary of dircmp objects, keyed by names in common_dirs.
+     subdirs: a dictionary of dircmp instances (or MyDirCmp instances if this
+       object is of type MyDirCmp, a subclass of dircmp), keyed by names
+       in common_dirs.
      """
 
     def __init__(self, a, b, ignore=None, hide=None): # Initialize
@@ -185,14 +187,15 @@ def phase3(self): # Find out differences between common files
         self.same_files, self.diff_files, self.funny_files = xx
 
     def phase4(self): # Find out differences between common subdirectories
-        # A new dircmp object is created for each common subdirectory,
+        # A new dircmp (or MyDirCmp if dircmp was subclassed) object is created
+        # for each common subdirectory,
         # these are stored in a dictionary indexed by filename.
         # The hide and ignore properties are inherited from the parent
         self.subdirs = {}
         for x in self.common_dirs:
             a_x = os.path.join(self.left, x)
             b_x = os.path.join(self.right, x)
-            self.subdirs[x]  = dircmp(a_x, b_x, self.ignore, self.hide)
+            self.subdirs[x]  = self.__class__(a_x, b_x, self.ignore, self.hide)
 
     def phase4_closure(self): # Recursively call phase4() on subdirectories
         self.phase4()
diff --git a/Lib/test/test_filecmp.py b/Lib/test/test_filecmp.py
@@ -66,6 +66,8 @@ def setUp(self):
         for dir in (self.dir, self.dir_same, self.dir_diff, self.dir_ignored):
             shutil.rmtree(dir, True)
             os.mkdir(dir)
+            subdir_path = os.path.join(dir, 'subdir')
+            os.mkdir(subdir_path)
             if self.caseinsensitive and dir is self.dir_same:
                 fn = 'FiLe'     # Verify case-insensitive comparison
             else:
@@ -110,24 +112,33 @@ def test_cmpfiles(self):
                     "Comparing mismatched directories fails")
 
 
+    def _assert_lists(self, actual, expected):
+        """Assert that two lists are equal, up to ordering."""
+        self.assertEqual(sorted(actual), sorted(expected))
+
+
     def test_dircmp(self):
         # Check attributes for comparison of two identical directories
         left_dir, right_dir = self.dir, self.dir_same
         d = filecmp.dircmp(left_dir, right_dir)
         self.assertEqual(d.left, left_dir)
         self.assertEqual(d.right, right_dir)
         if self.caseinsensitive:
-            self.assertEqual([d.left_list, d.right_list],[['file'], ['FiLe']])
+            self._assert_lists(d.left_list, ['file', 'subdir'])
+            self._assert_lists(d.right_list, ['FiLe', 'subdir'])
         else:
-            self.assertEqual([d.left_list, d.right_list],[['file'], ['file']])
-        self.assertEqual(d.common, ['file'])
+            self._assert_lists(d.left_list, ['file', 'subdir'])
+            self._assert_lists(d.right_list, ['file', 'subdir'])
+        self._assert_lists(d.common, ['file', 'subdir'])
+        self._assert_lists(d.common_dirs, ['subdir'])
         self.assertEqual(d.left_only, [])
         self.assertEqual(d.right_only, [])
         self.assertEqual(d.same_files, ['file'])
         self.assertEqual(d.diff_files, [])
         expected_report = [
             "diff {} {}".format(self.dir, self.dir_same),
             "Identical files : ['file']",
+            "Common subdirectories : ['subdir']",
         ]
         self._assert_report(d.report, expected_report)
 
@@ -136,9 +147,10 @@ def test_dircmp(self):
         d = filecmp.dircmp(left_dir, right_dir)
         self.assertEqual(d.left, left_dir)
         self.assertEqual(d.right, right_dir)
-        self.assertEqual(d.left_list, ['file'])
-        self.assertEqual(d.right_list, ['file', 'file2'])
-        self.assertEqual(d.common, ['file'])
+        self._assert_lists(d.left_list, ['file', 'subdir'])
+        self._assert_lists(d.right_list, ['file', 'file2', 'subdir'])
+        self._assert_lists(d.common, ['file', 'subdir'])
+        self._assert_lists(d.common_dirs, ['subdir'])
         self.assertEqual(d.left_only, [])
         self.assertEqual(d.right_only, ['file2'])
         self.assertEqual(d.same_files, ['file'])
@@ -147,6 +159,7 @@ def test_dircmp(self):
             "diff {} {}".format(self.dir, self.dir_diff),
             "Only in {} : ['file2']".format(self.dir_diff),
             "Identical files : ['file']",
+            "Common subdirectories : ['subdir']",
         ]
         self._assert_report(d.report, expected_report)
 
@@ -159,9 +172,9 @@ def test_dircmp(self):
         d = filecmp.dircmp(left_dir, right_dir)
         self.assertEqual(d.left, left_dir)
         self.assertEqual(d.right, right_dir)
-        self.assertEqual(d.left_list, ['file', 'file2'])
-        self.assertEqual(d.right_list, ['file'])
-        self.assertEqual(d.common, ['file'])
+        self._assert_lists(d.left_list, ['file', 'file2', 'subdir'])
+        self._assert_lists(d.right_list, ['file', 'subdir'])
+        self._assert_lists(d.common, ['file', 'subdir'])
         self.assertEqual(d.left_only, ['file2'])
         self.assertEqual(d.right_only, [])
         self.assertEqual(d.same_files, ['file'])
@@ -170,6 +183,7 @@ def test_dircmp(self):
             "diff {} {}".format(self.dir, self.dir_diff),
             "Only in {} : ['file2']".format(self.dir),
             "Identical files : ['file']",
+            "Common subdirectories : ['subdir']",
         ]
         self._assert_report(d.report, expected_report)
 
@@ -183,24 +197,45 @@ def test_dircmp(self):
             "diff {} {}".format(self.dir, self.dir_diff),
             "Identical files : ['file']",
             "Differing files : ['file2']",
+            "Common subdirectories : ['subdir']",
         ]
         self._assert_report(d.report, expected_report)
 
+    def test_dircmp_subdirs_type(self):
+        """Check that dircmp.subdirs respects subclassing."""
+        class MyDirCmp(filecmp.dircmp):
+            pass
+        d = MyDirCmp(self.dir, self.dir_diff)
+        sub_dirs = d.subdirs
+        self.assertEqual(list(sub_dirs.keys()), ['subdir'])
+        sub_dcmp = sub_dirs['subdir']
+        self.assertEqual(type(sub_dcmp), MyDirCmp)
+
     def test_report_partial_closure(self):
         left_dir, right_dir = self.dir, self.dir_same
         d = filecmp.dircmp(left_dir, right_dir)
+        left_subdir = os.path.join(left_dir, 'subdir')
+        right_subdir = os.path.join(right_dir, 'subdir')
         expected_report = [
             "diff {} {}".format(self.dir, self.dir_same),
             "Identical files : ['file']",
+            "Common subdirectories : ['subdir']",
+            '',
+            "diff {} {}".format(left_subdir, right_subdir),
         ]
         self._assert_report(d.report_partial_closure, expected_report)
 
     def test_report_full_closure(self):
         left_dir, right_dir = self.dir, self.dir_same
         d = filecmp.dircmp(left_dir, right_dir)
+        left_subdir = os.path.join(left_dir, 'subdir')
+        right_subdir = os.path.join(right_dir, 'subdir')
         expected_report = [
             "diff {} {}".format(self.dir, self.dir_same),
             "Identical files : ['file']",
+            "Common subdirectories : ['subdir']",
+            '',
+            "diff {} {}".format(left_subdir, right_subdir),
         ]
         self._assert_report(d.report_full_closure, expected_report)
 
diff --git a/Misc/ACKS b/Misc/ACKS
@@ -368,6 +368,7 @@ Ryan Coyner
 Christopher A. Craig
 Jeremy Craven
 Laura Creighton
+Nick Crews
 Tyler Crompton
 Simon Cross
 Felipe Cruz
diff --git a/Misc/NEWS.d/next/Library/2020-11-20-10-38-34.bpo-15450.E-y9PA.rst b/Misc/NEWS.d/next/Library/2020-11-20-10-38-34.bpo-15450.E-y9PA.rst
@@ -0,0 +1,2 @@
+Make :class:`filecmp.dircmp` respect subclassing. Now the
+:attr:`filecmp.dircmp.subdirs` behaves as expected when subclassing dircmp.

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+Make :class:`filecmp.dircmp` respect subclassing. Now the
	`2`	+:attr:`filecmp.dircmp.subdirs` behaves as expected when subclassing dircmp.