testing: stop skipping all Python tests of Rust revlog

This base class was not adapted for the introduction of `InnerRevlog`,
which also stopped exposing an `Index` class from `rustext`. As a
consequence, `test-rust-ancestor.py` was always skipped (and would
have been slightly broken).

We remove the skipping conditions from `rustancestorstest`, as they
now contradict or repeat those of the base class.
Also, `LazyAncestors` objects apparently hold only one reference to
the inner revlog (they had previously two references on the Rust index).
What matters most of course is the return to `start_count` in these
tests, i.e., that there is no memory leak nor double frees.

In the Python test, we conflate the presence of the `pyo3_rustext` package
with that of `rustext`, as we do not plan to support building one and not
the other (we hope to convert fully to PyO3 soon).
The skipping is actually done by the base test class.

mercurial/testing/revlog.py

@@ -1,64 +1,93
 from __future__ import annotations
 
 import unittest
 
 # picked from test-parse-index2, copied rather than imported
 # so that it stays stable even if test-parse-index2 changes or disappears.
 data_non_inlined = (
     b'\x00\x00\x00\x01\x00\x00\x00\x00\x00\x01D\x19'
     b'\x00\x07e\x12\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\xff\xff'
     b'\xff\xff\xff\xff\xd1\xf4\xbb\xb0\xbe\xfc\x13\xbd\x8c\xd3\x9d'
     b'\x0f\xcd\xd9;\x8c\x07\x8cJ/\x00\x00\x00\x00\x00\x00\x00\x00\x00'
     b'\x00\x00\x00\x00\x00\x00\x01D\x19\x00\x00\x00\x00\x00\xdf\x00'
     b'\x00\x01q\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x00\xff'
     b'\xff\xff\xff\xc1\x12\xb9\x04\x96\xa4Z1t\x91\xdfsJ\x90\xf0\x9bh'
     b'\x07l&\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
     b'\x00\x01D\xf8\x00\x00\x00\x00\x01\x1b\x00\x00\x01\xb8\x00\x00'
     b'\x00\x01\x00\x00\x00\x02\x00\x00\x00\x01\xff\xff\xff\xff\x02\n'
     b'\x0e\xc6&\xa1\x92\xae6\x0b\x02i\xfe-\xe5\xbao\x05\xd1\xe7\x00'
     b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01F'
     b'\x13\x00\x00\x00\x00\x01\xec\x00\x00\x03\x06\x00\x00\x00\x01'
     b'\x00\x00\x00\x03\x00\x00\x00\x02\xff\xff\xff\xff\x12\xcb\xeby1'
     b'\xb6\r\x98B\xcb\x07\xbd`\x8f\x92\xd9\xc4\x84\xbdK\x00\x00\x00'
     b'\x00\x00\x00\x00\x00\x00\x00\x00\x00'
 )
 
-from ..revlogutils.constants import REVLOGV1
+from ..revlogutils.constants import (
+    KIND_CHANGELOG,
+)
+from .. import revlog
 
 
 try:
     from ..cext import parsers as cparsers  # pytype: disable=import-error
 except ImportError:
     cparsers = None
 
 try:
-    from ..rustext.revlog import (  # pytype: disable=import-error
-        Index as RustIndex,
+    from ..rustext import (  # pytype: disable=import-error
+        revlog as rust_revlog,
     )
+
+    rust_revlog.__name__  # force actual import
 except ImportError:
-    RustIndex = None
+    rust_revlog = None
 
 
 @unittest.skipIf(
     cparsers is None,
     'The C version of the "parsers" module is not available. It is needed for this test.',
 )
 class RevlogBasedTestBase(unittest.TestCase):
     def parseindex(self, data=None):
         if data is None:
             data = data_non_inlined
         return cparsers.parse_index2(data, False)[0]
 
 
 @unittest.skipIf(
-    RustIndex is None,
-    'The Rust index is not available. It is needed for this test.',
+    rust_revlog is None,
+    'The Rust revlog module is not available. It is needed for this test.',
 )
 class RustRevlogBasedTestBase(unittest.TestCase):
-    def parserustindex(self, data=None):
+    # defaults
+    revlog_data_config = revlog.DataConfig()
+    revlog_delta_config = revlog.DeltaConfig()
+    revlog_feature_config = revlog.FeatureConfig()
+
+    def make_inner_revlog(
+        self, data=None, vfs_is_readonly=True, kind=KIND_CHANGELOG
+    ):
         if data is None:
             data = data_non_inlined
-        # not inheriting RevlogBasedTestCase to avoid having a
-        # `parseindex` method that would be shadowed by future subclasses
-        # this duplication will soon be removed
-        return RustIndex(data, REVLOGV1)
+
+        return rust_revlog.InnerRevlog(
+            vfs_base=b"Just a path",
+            fncache=None,  # might be enough for now
+            vfs_is_readonly=vfs_is_readonly,
+            index_data=data,
+            index_file=b'test.i',
+            data_file=b'test.d',
+            sidedata_file=None,
+            inline=False,
+            data_config=self.revlog_data_config,
+            delta_config=self.revlog_delta_config,
+            feature_config=self.revlog_feature_config,
+            chunk_cache=None,
+            default_compression_header=None,
+            revlog_type=kind,
+            use_persistent_nodemap=False,  # until we cook one.
+        )
+
+    def parserustindex(self, data=None):
+        return revlog.RustIndexProxy(self.make_inner_revlog(data=data))

tests/test-rust-ancestor.py

@@ -1,167 +1,155
 import sys
-import unittest
 
 from mercurial.node import wdirrev
 
 from mercurial.testing import revlog as revlogtesting
 
 try:
     from mercurial import rustext
 
     rustext.__name__  # trigger immediate actual import
 except ImportError:
     rustext = None
 else:
     # this would fail already without appropriate ancestor.__package__
     from mercurial.rustext.ancestor import (
         AncestorsIterator,
         LazyAncestors,
         MissingAncestors,
     )
     from mercurial.rustext import dagop
 
 try:
     from mercurial.cext import parsers as cparsers
 except ImportError:
     cparsers = None
 
 
[e-mail](
-    rustext is None,
-    'The Rust version of the "ancestor" module is not available. It is needed'
-    ' for this test.',
-)
[e-mail](
-    rustext is None,
-    'The Rust or C version of the "parsers" module, which the "ancestor" module'
-    ' relies on, is not available.',
-)
 class rustancestorstest(revlogtesting.RustRevlogBasedTestBase):
     """Test the correctness of binding to Rust code.
 
     This test is merely for the binding to Rust itself: extraction of
     Python variable, giving back the results etc.
 
     It is not meant to test the algorithmic correctness of the operations
     on ancestors it provides. Hence the very simple embedded index data is
     good enough.
 
     Algorithmic correctness is asserted by the Rust unit tests.
     """
 
     def testiteratorrevlist(self):
         idx = self.parserustindex()
         # checking test assumption about the index binary data:
         self.assertEqual(
             {i: (r[5], r[6]) for i, r in enumerate(idx)},
             {0: (-1, -1), 1: (0, -1), 2: (1, -1), 3: (2, -1)},
         )
         ait = AncestorsIterator(idx, [3], 0, True)
         self.assertEqual([r for r in ait], [3, 2, 1, 0])
 
         ait = AncestorsIterator(idx, [3], 0, False)
         self.assertEqual([r for r in ait], [2, 1, 0])
 
     def testlazyancestors(self):
         idx = self.parserustindex()
-        start_count = sys.getrefcount(idx)  # should be 2 (see Python doc)
+        start_count = sys.getrefcount(idx.inner)  # should be 2 (see Python doc)
         self.assertEqual(
             {i: (r[5], r[6]) for i, r in enumerate(idx)},
             {0: (-1, -1), 1: (0, -1), 2: (1, -1), 3: (2, -1)},
         )
         lazy = LazyAncestors(idx, [3], 0, True)
-        # we have two more references to the index:
-        # - in its inner iterator for __contains__ and __bool__
-        # - in the LazyAncestors instance itself (to spawn new iterators)
-        self.assertEqual(sys.getrefcount(idx), start_count + 2)
+        # the LazyAncestors instance holds just one reference to the
+        # inner revlog.
+        self.assertEqual(sys.getrefcount(idx.inner), start_count + 1)
 
         self.assertTrue(2 in lazy)
         self.assertTrue(bool(lazy))
         self.assertEqual(list(lazy), [3, 2, 1, 0])
         # a second time to validate that we spawn new iterators
         self.assertEqual(list(lazy), [3, 2, 1, 0])
 
         # now let's watch the refcounts closer
         ait = iter(lazy)
-        self.assertEqual(sys.getrefcount(idx), start_count + 3)
+        self.assertEqual(sys.getrefcount(idx.inner), start_count + 2)
         del ait
-        self.assertEqual(sys.getrefcount(idx), start_count + 2)
+        self.assertEqual(sys.getrefcount(idx.inner), start_count + 1)
         del lazy
-        self.assertEqual(sys.getrefcount(idx), start_count)
+        self.assertEqual(sys.getrefcount(idx.inner), start_count)
 
         # let's check bool for an empty one
         self.assertFalse(LazyAncestors(idx, [0], 0, False))
 
     def testmissingancestors(self):
         idx = self.parserustindex()
         missanc = MissingAncestors(idx, [1])
         self.assertTrue(missanc.hasbases())
         self.assertEqual(missanc.missingancestors([3]), [2, 3])
         missanc.addbases({2})
         self.assertEqual(missanc.bases(), {1, 2})
         self.assertEqual(missanc.missingancestors([3]), [3])
         self.assertEqual(missanc.basesheads(), {2})
 
     def testmissingancestorsremove(self):
         idx = self.parserustindex()
         missanc = MissingAncestors(idx, [1])
         revs = {0, 1, 2, 3}
         missanc.removeancestorsfrom(revs)
         self.assertEqual(revs, {2, 3})
 
     def testrefcount(self):
         idx = self.parserustindex()
-        start_count = sys.getrefcount(idx)
+        start_count = sys.getrefcount(idx.inner)
 
         # refcount increases upon iterator init...
         ait = AncestorsIterator(idx, [3], 0, True)
-        self.assertEqual(sys.getrefcount(idx), start_count + 1)
+        self.assertEqual(sys.getrefcount(idx.inner), start_count + 1)
         self.assertEqual(next(ait), 3)
 
         # and decreases once the iterator is removed
         del ait
-        self.assertEqual(sys.getrefcount(idx), start_count)
+        self.assertEqual(sys.getrefcount(idx.inner), start_count)
 
         # and removing ref to the index after iterator init is no issue
         ait = AncestorsIterator(idx, [3], 0, True)
         del idx
         self.assertEqual(list(ait), [3, 2, 1, 0])
 
         # the index is not tracked by the GC, hence there is nothing more
         # we can assert to check that it is properly deleted once its refcount
         # drops to 0
 
     def testgrapherror(self):
         data = (
             revlogtesting.data_non_inlined[: 64 + 27]
             + b'\xf2'
             + revlogtesting.data_non_inlined[64 + 28 :]
         )
         idx = self.parserustindex(data=data)
         with self.assertRaises(rustext.GraphError) as arc:
             AncestorsIterator(idx, [1], -1, False)
         exc = arc.exception
         self.assertIsInstance(exc, ValueError)
         # rust-cpython issues appropriate str instances for Python 2 and 3
         self.assertEqual(exc.args, ('ParentOutOfRange', 1))
 
     def testwdirunsupported(self):
         # trying to access ancestors of the working directory raises
         idx = self.parserustindex()
         with self.assertRaises(rustext.GraphError) as arc:
             list(AncestorsIterator(idx, [wdirrev], -1, False))
 
         exc = arc.exception
         self.assertIsInstance(exc, ValueError)
         # rust-cpython issues appropriate str instances for Python 2 and 3
         self.assertEqual(exc.args, ('InvalidRevision', wdirrev))
 
     def testheadrevs(self):
         idx = self.parserustindex()
         self.assertEqual(dagop.headrevs(idx, [1, 2, 3]), {3})
 
 
 if __name__ == '__main__':
     import silenttestrunner
 
     silenttestrunner.main(__name__)