From ef19d91fef1ba61b8a68e3b3799b0c29e17829b7 Mon Sep 17 00:00:00 2001 From: ohbarye Date: Sun, 29 Jun 2025 17:22:21 +0900 Subject: [PATCH 1/3] Add failing tests for IntegerArbitrary#shrink to respect min/max constraints This commit adds test cases that verify the shrink method respects the min/max range constraints. Currently these tests fail because shrink always converges to 0, ignoring the specified range boundaries. Issue: #35 --- spec/pbt/arbitrary/integer_arbitrary_spec.rb | 34 ++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/spec/pbt/arbitrary/integer_arbitrary_spec.rb b/spec/pbt/arbitrary/integer_arbitrary_spec.rb index 90851b4..e703d39 100644 --- a/spec/pbt/arbitrary/integer_arbitrary_spec.rb +++ b/spec/pbt/arbitrary/integer_arbitrary_spec.rb @@ -53,5 +53,39 @@ end end end + + describe "with min/max constraints" do + it "shrinks values within the specified range" do + arb = Pbt::Arbitrary::IntegerArbitrary.new(25, 65) + shrunk_values = arb.shrink(50).to_a + + expect(shrunk_values).to all(be >= 25) + expect(shrunk_values).to all(be <= 65) + end + + it "shrinks from max value respecting the min constraint" do + arb = Pbt::Arbitrary::IntegerArbitrary.new(25, 65) + shrunk_values = arb.shrink(65).to_a + + expect(shrunk_values).to all(be >= 25) + expect(shrunk_values).to all(be <= 65) + expect(shrunk_values.last).to be >= 25 + end + + it "shrinks from min value respecting the min constraint" do + arb = Pbt::Arbitrary::IntegerArbitrary.new(25, 65) + shrunk_values = arb.shrink(25).to_a + + expect(shrunk_values).to be_empty + end + + it "shrinks negative range values within constraints" do + arb = Pbt::Arbitrary::IntegerArbitrary.new(-50, -10) + shrunk_values = arb.shrink(-20).to_a + + expect(shrunk_values).to all(be >= -50) + expect(shrunk_values).to all(be <= -10) + end + end end end From 4706ed1163e076d4d820ae0fcc736791b16f7fd4 Mon Sep 17 00:00:00 2001 From: ohbarye Date: Sun, 29 Jun 2025 17:23:53 +0900 Subject: [PATCH 2/3] fix: ensure IntegerArbitrary#shrink respects min/max bounds Fixed an issue where IntegerArbitrary#shrink could produce values outside the specified min/max range constraints. Changes: - When target is not specified, use the closest value to 0 within bounds - Always clamp target to be within the valid range - Clamp intermediate values during shrinking to stay within bounds - Don't yield target if it equals the current value This ensures that for ranges like min:25, max:65, all shrunk values are guaranteed to be within the specified bounds. Fixes #13 --- lib/pbt/arbitrary/integer_arbitrary.rb | 15 +++++++++++++-- spec/pbt/arbitrary/choose_arbitrary_spec.rb | 2 +- spec/pbt/arbitrary/integer_arbitrary_spec.rb | 8 ++++---- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/pbt/arbitrary/integer_arbitrary.rb b/lib/pbt/arbitrary/integer_arbitrary.rb index b225926..7db5ef6 100644 --- a/lib/pbt/arbitrary/integer_arbitrary.rb +++ b/lib/pbt/arbitrary/integer_arbitrary.rb @@ -20,7 +20,13 @@ def generate(rng) end # @see Arbitrary#shrink - def shrink(current, target: DEFAULT_TARGET) + def shrink(current, target: nil) + # If no target is specified, use the appropriate bound as target + target ||= DEFAULT_TARGET.clamp(@min, @max) + + # Ensure target is within bounds + target = target.clamp(@min, @max) + gap = current - target return Enumerator.new { |_| } if gap == 0 @@ -30,9 +36,14 @@ def shrink(current, target: DEFAULT_TARGET) while (diff = (current - target).abs) > 1 halved = diff / 2 current -= is_positive_gap ? halved : -halved + # Ensure current stays within bounds + current = current.clamp(@min, @max) y.yield current end - y.yield target # no diff here + # Only yield target if it's different from current + if current != target + y.yield target + end end end end diff --git a/spec/pbt/arbitrary/choose_arbitrary_spec.rb b/spec/pbt/arbitrary/choose_arbitrary_spec.rb index a53b52a..7febf52 100644 --- a/spec/pbt/arbitrary/choose_arbitrary_spec.rb +++ b/spec/pbt/arbitrary/choose_arbitrary_spec.rb @@ -27,7 +27,7 @@ it "returns an Enumerator that iterates halved integers towards the min" do arb = Pbt::Arbitrary::ChooseArbitrary.new(-2..10) - expect(arb.shrink(50).to_a).to eq [24, 11, 5, 2, 0, -1, -2] + expect(arb.shrink(50).to_a).to eq [10, 4, 1, 0, -1, -2] end context "when current value and target is same" do diff --git a/spec/pbt/arbitrary/integer_arbitrary_spec.rb b/spec/pbt/arbitrary/integer_arbitrary_spec.rb index e703d39..5d544d3 100644 --- a/spec/pbt/arbitrary/integer_arbitrary_spec.rb +++ b/spec/pbt/arbitrary/integer_arbitrary_spec.rb @@ -58,7 +58,7 @@ it "shrinks values within the specified range" do arb = Pbt::Arbitrary::IntegerArbitrary.new(25, 65) shrunk_values = arb.shrink(50).to_a - + expect(shrunk_values).to all(be >= 25) expect(shrunk_values).to all(be <= 65) end @@ -66,7 +66,7 @@ it "shrinks from max value respecting the min constraint" do arb = Pbt::Arbitrary::IntegerArbitrary.new(25, 65) shrunk_values = arb.shrink(65).to_a - + expect(shrunk_values).to all(be >= 25) expect(shrunk_values).to all(be <= 65) expect(shrunk_values.last).to be >= 25 @@ -75,14 +75,14 @@ it "shrinks from min value respecting the min constraint" do arb = Pbt::Arbitrary::IntegerArbitrary.new(25, 65) shrunk_values = arb.shrink(25).to_a - + expect(shrunk_values).to be_empty end it "shrinks negative range values within constraints" do arb = Pbt::Arbitrary::IntegerArbitrary.new(-50, -10) shrunk_values = arb.shrink(-20).to_a - + expect(shrunk_values).to all(be >= -50) expect(shrunk_values).to all(be <= -10) end From b11e31d2074f5e747af069c4ecee3121627b4cf2 Mon Sep 17 00:00:00 2001 From: ohbarye Date: Sun, 29 Jun 2025 18:46:32 +0900 Subject: [PATCH 3/3] docs: update CHANGELOG for IntegerArbitrary#shrink fix Added entry for PR #36 that fixes the shrink method to respect min/max bounds in all cases. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 929f60f..81542d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## [Unreleased] +- Fix `IntegerArbitrary#shrink` to respect min/max bounds [#36](https://github.com/ohbarye/pbt/pull/36) + ## [0.5.0] - 2024-12-30 - [Breaking change] Drop `:process` and `:thread` workers since there are no concrete use cases.