Tests and `debug_assert!`

[blinxen]

I am currently packaging ropey for Fedora as it is a dependency for helix. When running tests in release mode, the following tests fails:

• https://github.com/cessen/ropey/blob/master/src/tree/node_children.rs#L788
• https://github.com/cessen/ropey/blob/master/src/tree/node_children.rs#L1000

Looking at the first test, it seems that it fails because the assertion that checks for index bounds is only executed in debug mode. Is this intentional? Out-of-bounds checks look like safety checks to me, so I would expect assert! to be used instead of debug_assert!. However, if they are only sanity checks for tests, then I would assume that adding #[cfg(debug_assertions)] to the failing tests would be appropriate as they only panic in debug mode. What do you think about this?

[pascalkuthe]

That is not an out-of-bounds check/safety check but just a sanity check.

This method will always return the same thing in case char_idx >= (accum.chars + self.info()[idx].chars) as usize because search_by will essentially compute a sum of all tree children as long as pred returns true. If char_idx is too large you simply sum all children (same as if you passed the node length directly).

The reason this debug assert is that you don't actually want to pass a larger number here and the case where it does occur likely indicates a a bug in the calling code. It does not however not represent a correctness/saefty invariant that would need runtime checks outside of tests/debugging.

[blinxen]

Thanks for the clarification! I will create a PR to mark both tests with #[cfg(debug_assertions)] as it only makes sense to execute them in debug mode.