# HG changeset patch # User Georges Racinet # Date 2024-11-30 18:12:02 # Node ID cf5b47b885b164e18a1f1ef946d398e60c06a52b # Parent 544b9d3075f4f6d91187e8cb4b606b4c0d178ffa 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. diff --git a/mercurial/testing/revlog.py b/mercurial/testing/revlog.py --- a/mercurial/testing/revlog.py +++ b/mercurial/testing/revlog.py @@ -23,7 +23,10 @@ data_non_inlined = ( 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 @@ 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( @@ -51,14 +56,38 @@ class RevlogBasedTestBase(unittest.TestC @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)) diff --git a/tests/test-rust-ancestor.py b/tests/test-rust-ancestor.py --- a/tests/test-rust-ancestor.py +++ b/tests/test-rust-ancestor.py @@ -1,5 +1,4 @@ import sys -import unittest from mercurial.node import wdirrev @@ -26,16 +25,6 @@ except ImportError: 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 @@ class rustancestorstest(revlogtesting.Ru 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 @@ class rustancestorstest(revlogtesting.Ru # 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 @@ class rustancestorstest(revlogtesting.Ru 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)