( )⚙ D6345 rust-discovery: optionally don't randomize at all, for tests

This is an archive of the discontinued Mercurial Phabricator instance.

rust-discovery: optionally don't randomize at all, for tests
AbandonedPublic

Authored by gracinet on May 6 2019, 11:30 AM.

Details

Reviewers
kevincox
Group Reviewers
hg-reviewers
Summary

As seen from Python, this is a new randomize kwarg in init of the
discovery object. It replaces random picking by some arbitrary yet
deterministic strategy.

This is the same as what test-setdiscovery.t does, with the added
benefit to be usable both in Python and Rust implementations.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

gracinet created this revision.May 6 2019, 11:30 AM
kevincox accepted this revision.May 10 2019, 9:46 AM

The code and rust style looks fine. I'm not sure that taking the N smallest elements is the best approach for testing though. I can imagine it hiding some issues with larger values.

@kevincox I don't think the potential masking in tests due to this strategy is a big problem. That test is pretty recent, compared to setdiscovery, at least, and doesn't really test that sampling is very relevant. We could do better though by introducing several non random selecting strategies if we need it in the future, but this was good enough for me for the time being.

gracinet abandoned this revision.May 22 2019, 12:38 PM

I'm abandoning this revision because I have to resubmit the whole series after partial incorrect application last week and subsequent droping from the actual repo.