More thorough testing of BTreeMap::range

[ssomers]

Test more of the paths in the range_search function in map.rs

[rust-highfive]

r? @joshtriplett

(rust_highfive has picked a reviewer for you, use r? to override)

[ssomers]

src/liballoc/collections/btree/map.rs line 1955 is currently the start of loop of the range_search function. There were no tests hitting the match variants at line 1968, 1972 and 1992. With this testing, 1968 is hit but the other two "(true, Excluded(_))" never are. I don't understand the code well so I was unable to hit them, or to figure out that they are perhaps impossible. all are hit.

[ssomers]

It was bound to happen... I had another look and added a test case to cover everything.

Note I also let test_range_large take on a more modest tree, and I don't know if it's useful at all. There's also test_range that tries all positions for the Inclusive search, and test_range_1000 for those who believe that testing on bigger things is better testing.

[RalfJung]

Did you try running this in Miri? I think it'll be too slow, and it's probably best to disable this test in Miri (or maybe use a smaller size in Miri, one that doesn't take forever).

[ssomers]

Yes, I always ask Miri. In fact I was relying on Miri a bit too much lately.

It's not a new test, but renamed from _internal _inclusive, and it was bigger before.

[RalfJung]

Ah, seems like Miri is fast enough then, great. :)

In fact I was relying on Miri a bit too much lately.

How that?

[ssomers]

Well, not Miri itself, actually. Just the fact that I have Miri set up to run only the src/liballoc tests and forget all the rest on Linux.

[ssomers]

Ah, seems like Miri is fast enough then, great. :)

It was dragging through the test at 500, but I really don't see the point in testing something with 500 elements when 144 would give you the same 3-level search. More? Why - to have a filled up node somewhere? Then 150 would be enough. For starters, I wish we could check a tree's height and/or read that node.CAPACITY or node.B value.

[Mark-Simulacrum]

@bors r+
r? @Mark-Simulacrum

Seems fine to add more tests to BTreeMap, since this essentially just checks current behavior rather than changing it.

[bors]

📌 Commit 8314b7f has been approved by Mark-Simulacrum

[bors]

⌛ Testing commit 8314b7f with merge 6250d56...

[bors]

☀️ Test successful - checks-azure
Approved by: Mark-Simulacrum
Pushing 6250d56 to master...