proptest-rs / proptest

Hypothesis-like property testing for Rust
Apache License 2.0
1.63k stars 152 forks source link

Panic on empty ranges during strategy creation #449

Closed mirandaconrado closed 2 weeks ago

mirandaconrado commented 1 month ago

I have encountered scenarios in my code where I end up with empty ranges. Most of the time this happens when one of the bounds of the range comes from an arbitrary value. It's kind of hard to track these down because they fail at the moment that values are generated instead of failing when the strategy is created, even though we know by then that it will fail.

Example:

use proptest::prelude::*;

proptest! {
    #[test]
    fn empty_range(s in 0..0) {}
}

causes

---- empty_range stdout ----
thread 'empty_range' panicked at /Users/conrado/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rand-0.8.5/src/distributions/uniform.rs:561:1:
Uniform::new called with `low >= high`
stack backtrace:
   0: rust_begin_unwind
             at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/std/src/panicking.rs:647:5
   1: core::panicking::panic_fmt
             at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/panicking.rs:72:14
   2: core::panicking::panic
             at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/panicking.rs:144:5
   3: <rand::distributions::uniform::UniformInt<i32> as rand::distributions::uniform::UniformSampler>::new
             at /Users/conrado/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rand-0.8.5/src/distributions/uniform.rs:452:17
   4: rand::distributions::uniform::Uniform<X>::new
             at /Users/conrado/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rand-0.8.5/src/distributions/uniform.rs:189:17
   5: proptest::num::sample_uniform
             at /Users/conrado/.cargo/registry/src/index.crates.io-6f17d22bba15001f/proptest-1.4.0/src/num.rs:28:5
   6: proptest::num::i32::<impl proptest::strategy::traits::Strategy for core::ops::range::Range<i32>>::new_tree
             at /Users/conrado/.cargo/registry/src/index.crates.io-6f17d22bba15001f/proptest-1.4.0/src/num.rs:74:21
   7: <proptest::strategy::map::Map<S,F> as proptest::strategy::traits::Strategy>::new_tree
             at /Users/conrado/.cargo/registry/src/index.crates.io-6f17d22bba15001f/proptest-1.4.0/src/strategy/map.rs:53:9
   8: proptest::test_runner::runner::TestRunner::gen_and_run_case
             at /Users/conrado/.cargo/registry/src/index.crates.io-6f17d22bba15001f/proptest-1.4.0/src/test_runner/runner.rs:659:31
   9: proptest::test_runner::runner::TestRunner::run_in_process_with_replay
             at /Users/conrado/.cargo/registry/src/index.crates.io-6f17d22bba15001f/proptest-1.4.0/src/test_runner/runner.rs:598:13
  10: proptest::test_runner::runner::TestRunner::run_in_process
             at /Users/conrado/.cargo/registry/src/index.crates.io-6f17d22bba15001f/proptest-1.4.0/src/test_runner/runner.rs:570:9
  11: proptest::test_runner::runner::TestRunner::run
             at /Users/conrado/.cargo/registry/src/index.crates.io-6f17d22bba15001f/proptest-1.4.0/src/test_runner/runner.rs:413:13
  12: example::empty_range
             at /Users/conrado/.cargo/registry/src/index.crates.io-6f17d22bba15001f/proptest-1.4.0/src/sugar.rs:163:17
  13: example::empty_range::{{closure}}
             at /Users/conrado/.cargo/registry/src/index.crates.io-6f17d22bba15001f/proptest-1.4.0/src/sugar.rs:159:28
  14: core::ops::function::FnOnce::call_once
             at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/ops/function.rs:250:5
  15: core::ops::function::FnOnce::call_once
             at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/ops/function.rs:250:5

which doesn't actually point to the example code anywhere (though it does refer to example::empty_range. It would be more useful to have a panic at the moment that we try to create a strategy for the empty range instead of when we sample from it.

I'm happy to work on a PR for this assuming that the maintainers believe it should be done, and maybe have a recommendation.

rix0rrr commented 4 weeks ago

Would love for this to be done!

matthew-russo commented 3 weeks ago

Hey sorry for the delay.

This seems like a reasonable ask and I can't think of any counterarguments. If you're interested in putting up a PR, I'm in favor of accepting it. I'll drop a note to the other maintainers to make sure they're also onboard.

Thanks

mirandaconrado commented 3 weeks ago

Awesome. I'll look into the codebase on how to implement it and try to follow the existing patterns.