This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6517

rust-discovery: read the index from a repo passed at init
ClosedPublic

Authored by gracinet on Jun 12 2019, 2:13 PM.

Details

Reviewers
kevincox
Group Reviewers
hg-reviewers
Commits
rHGb6f3f704a561: rust-discovery: read the index from a repo passed at init
Summary

This makes the API of the Rust PartialDiscovery object now
the same (or rather a subset) of the Python object, hence
easier to control through module policy down the road.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

gracinet created this revision.Jun 12 2019, 2:13 PM
kevincox accepted this revision.Jun 13 2019, 6:05 AM
Alphare updated this revision to Diff 15932.Jul 17 2019, 12:30 PM
Alphare updated this revision to Diff 16011.Jul 23 2019, 5:14 AM
durin42 added inline comments.Aug 14 2019, 3:27 PM
tests/test-rust-discovery.py
34

I'd rather we not have this one object that mocks both a repo and a changelog - it _will_ become confusing in the future. Make it two objects.

gracinet added a commit: rHGb6f3f704a561: rust-discovery: read the index from a repo passed at init.Aug 14 2019, 4:15 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGb6f3f704a561: rust-discovery: read the index from a repo passed at init (authored by gracinet). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 16011

rust/hg-cpython/src/discovery.rs

py_class!(pub class PartialDiscovery |py| { py_class!(pub class PartialDiscovery |py| {
data inner: RefCell<Box<CorePartialDiscovery<Index>>>; data inner: RefCell<Box<CorePartialDiscovery<Index>>>;
// `_respectsize` is currently only here to replicate the Python API and // `_respectsize` is currently only here to replicate the Python API and
// will be used in future patches inside methods that are yet to be // will be used in future patches inside methods that are yet to be
// implemented. // implemented.
def __new__( def __new__(
_cls, _cls,
index: PyObject, repo: PyObject,
targetheads: PyObject, targetheads: PyObject,
_respectsize: bool _respectsize: bool
) -> PyResult<PartialDiscovery> { ) -> PyResult<PartialDiscovery> {
let index = repo.getattr(py, "changelog")?.getattr(py, "index")?;
Self::create_instance( Self::create_instance(
py, py,
RefCell::new(Box::new(CorePartialDiscovery::new( RefCell::new(Box::new(CorePartialDiscovery::new(
Index::new(py, index)?, Index::new(py, index)?,
rev_pyiter_collect(py, &targetheads)?, rev_pyiter_collect(py, &targetheads)?,
))) )))
) )
} }

tests/test-rust-discovery.py

b'\x0e\xc6&\xa1\x92\xae6\x0b\x02i\xfe-\xe5\xbao\x05\xd1\xe7\x00' 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'\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'\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'\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'\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' b'\x00\x00\x00\x00\x00\x00\x00\x00\x00'
) )
class fakerepo(object):
durin42Unsubmitted
Not Done

I'd rather we not have this one object that mocks both a repo and a changelog - it _will_ become confusing in the future. Make it two objects.

durin42: I'd rather we not have this one object that mocks both a repo and a changelog - it _will_…
def __init__(self, idx):
"""Just make so that self.changelog.index is the given idx."""
self.index = idx
self.changelog = self
@unittest.skipIf(PartialDiscovery is None or cparsers is None, @unittest.skipIf(PartialDiscovery is None or cparsers is None,
"rustext or the C Extension parsers module " "rustext or the C Extension parsers module "
"discovery relies on is not available") "discovery relies on is not available")
class rustdiscoverytest(unittest.TestCase): class rustdiscoverytest(unittest.TestCase):
"""Test the correctness of binding to Rust code. """Test the correctness of binding to Rust code.
This test is merely for the binding to Rust itself: extraction of This test is merely for the binding to Rust itself: extraction of
Python variable, giving back the results etc. Python variable, giving back the results etc.
It is not meant to test the algorithmic correctness of the provided It is not meant to test the algorithmic correctness of the provided
methods. Hence the very simple embedded index data is good enough. methods. Hence the very simple embedded index data is good enough.
Algorithmic correctness is asserted by the Rust unit tests. Algorithmic correctness is asserted by the Rust unit tests.
""" """
def parseindex(self): def parseindex(self):
return cparsers.parse_index2(data_non_inlined, False)[0] return cparsers.parse_index2(data_non_inlined, False)[0]
def repo(self):
return fakerepo(self.parseindex())
def testindex(self): def testindex(self):
idx = self.parseindex() idx = self.parseindex()
# checking our assumptions about the index binary data: # checking our assumptions about the index binary data:
self.assertEqual({i: (r[5], r[6]) for i, r in enumerate(idx)}, self.assertEqual({i: (r[5], r[6]) for i, r in enumerate(idx)},
{0: (-1, -1), {0: (-1, -1),
1: (0, -1), 1: (0, -1),
2: (1, -1), 2: (1, -1),
3: (2, -1)}) 3: (2, -1)})
def testaddcommonsmissings(self): def testaddcommonsmissings(self):
idx = self.parseindex() disco = PartialDiscovery(self.repo(), [3], True)
disco = PartialDiscovery(idx, [3], True)
self.assertFalse(disco.hasinfo()) self.assertFalse(disco.hasinfo())
self.assertFalse(disco.iscomplete()) self.assertFalse(disco.iscomplete())
disco.addcommons([1]) disco.addcommons([1])
self.assertTrue(disco.hasinfo()) self.assertTrue(disco.hasinfo())
self.assertFalse(disco.iscomplete()) self.assertFalse(disco.iscomplete())
disco.addmissings([2]) disco.addmissings([2])
self.assertTrue(disco.hasinfo()) self.assertTrue(disco.hasinfo())
self.assertTrue(disco.iscomplete()) self.assertTrue(disco.iscomplete())
self.assertEqual(disco.commonheads(), {1}) self.assertEqual(disco.commonheads(), {1})
def testaddmissingsstats(self): def testaddmissingsstats(self):
idx = self.parseindex() disco = PartialDiscovery(self.repo(), [3], True)
disco = PartialDiscovery(idx, [3], True)
self.assertIsNone(disco.stats()['undecided'], None) self.assertIsNone(disco.stats()['undecided'], None)
disco.addmissings([2]) disco.addmissings([2])
self.assertEqual(disco.stats()['undecided'], 2) self.assertEqual(disco.stats()['undecided'], 2)
def testaddinfocommonfirst(self): def testaddinfocommonfirst(self):
idx = self.parseindex() disco = PartialDiscovery(self.repo(), [3], True)
disco = PartialDiscovery(idx, [3], True)
disco.addinfo([(1, True), (2, False)]) disco.addinfo([(1, True), (2, False)])
self.assertTrue(disco.hasinfo()) self.assertTrue(disco.hasinfo())
self.assertTrue(disco.iscomplete()) self.assertTrue(disco.iscomplete())
self.assertEqual(disco.commonheads(), {1}) self.assertEqual(disco.commonheads(), {1})
def testaddinfomissingfirst(self): def testaddinfomissingfirst(self):
idx = self.parseindex() disco = PartialDiscovery(self.repo(), [3], True)
disco = PartialDiscovery(idx, [3], True)
disco.addinfo([(2, False), (1, True)]) disco.addinfo([(2, False), (1, True)])
self.assertTrue(disco.hasinfo()) self.assertTrue(disco.hasinfo())
self.assertTrue(disco.iscomplete()) self.assertTrue(disco.iscomplete())
self.assertEqual(disco.commonheads(), {1}) self.assertEqual(disco.commonheads(), {1})
if __name__ == '__main__': if __name__ == '__main__':
import silenttestrunner import silenttestrunner
silenttestrunner.main(__name__) silenttestrunner.main(__name__)