changeset 52436:cf5b47b885b1

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.
author Georges Racinet <georges.racinet@cloudcrane.io>
date Sat, 30 Nov 2024 19:12:02 +0100
parents 544b9d3075f4
children c28ba6fb3fae
files mercurial/testing/revlog.py tests/test-rust-ancestor.py
diffstat 2 files changed, 50 insertions(+), 33 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial/testing/revlog.py	Fri Nov 29 22:55:30 2024 +0100
+++ b/mercurial/testing/revlog.py	Sat Nov 30 19:12:02 2024 +0100
@@ -23,7 +23,10 @@
     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:
@@ -32,11 +35,13 @@
     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(
@@ -51,14 +56,38 @@
 
 
 @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))
--- a/tests/test-rust-ancestor.py	Fri Nov 29 22:55:30 2024 +0100
+++ b/tests/test-rust-ancestor.py	Sat Nov 30 19:12:02 2024 +0100
@@ -1,5 +1,4 @@
 import sys
-import unittest
 
 from mercurial.node import wdirrev
 
@@ -26,16 +25,6 @@
     cparsers = None
 
 
-@unittest.skipIf(
-    rustext is None,
-    'The Rust version of the "ancestor" module is not available. It is needed'
-    ' for this test.',
-)
-@unittest.skipIf(
-    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.
 
@@ -64,16 +53,15 @@
 
     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))
@@ -83,11 +71,11 @@
 
         # 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))
@@ -111,16 +99,16 @@
 
     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)