Merge stamps and labels in And/Or property combinators

[Copilot]

When combining properties with .&. (And) or .|. (Or), only the left operand's statistics (collect, classify) and labels were propagated. This made it impossible to see test data distribution for both sides of a combined property.

Example

let prop1 = 1 = 1 |> Prop.collect "Property 1" |> Prop.classify true "P1"
let prop2 = 1 <> 2 |> Prop.collect "Property 2" |> Prop.classify true "P2"

// Before: Ok, passed 100 tests (100% P1, "Property 1").
// After:  Ok, passed 100 tests (100% P1, "Property 1", P2, "Property 2").
prop1 .&. prop2 |> Check.Quick

Changes

• Result.ResAnd/ResOr: Merge Stamp and Labels when both sides have the same outcome (both Passed or both Failed)
• Result.MergeStampsAndLabels: Extract merge logic to avoid duplication
• Tests: Added coverage for stamp/label merging in And/Or combinations

Merging on failure ensures that when multiple sub-properties fail, all their labels are visible in the error output, improving debuggability.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Using Collect and Classify in conjunction with And or Or</issue_title>
<issue_description>Hello,

When I try to collect and /or classify two or more properties only the left hand side of the and or or operator seems to propagate its statistics.
I am not sure if this is a bug or intended behavior?
Using FsCheck 2.16.5.

open FsCheck

let prop1 = 1 = 1 |@ "1 = 1" |> Prop.collect "Property 1" |> Prop.classify true "P1"
let prop2 = 1 <> 2 |@ "1 <> 2" |> Prop.collect "Property 2" |> Prop.classify true "P2"

printfn "\nprop1"
prop1 |> Check.Quick
printfn "\nprop2"
prop2 |> Check.Quick

printfn "\nprop1 .&. prop2"
prop1 .&. prop2 |> Check.Quick
printfn "\nprop2 .&. prop1"
prop2 .&. prop1 |> Check.Quick

printfn "\nprop1 .|. prop2"
prop1 .|. prop2 |> Check.Quick

printfn "\nprop2 .|. prop1"
prop2 .|. prop1 |> Check.Quick

Output:

prop1
Ok, passed 100 tests (100% P1, "Property 1").

prop2
Ok, passed 100 tests (100% P2, "Property 2").

prop1 .&. prop2
Ok, passed 100 tests (100% P1, "Property 1").

prop2 .&. prop1
Ok, passed 100 tests (100% P2, "Property 2").

prop1 .|. prop2
Ok, passed 100 tests (100% P1, "Property 1").

prop2 .|. prop1
Ok, passed 100 tests (100% P2, "Property 2").

If I add a failing prop3 1 = 3, the correct label is shown:

let prop3 = 1 = 3 |@ "1 = 3" |> Prop.collect "Property 3" |> Prop.classify true "P3"

printfn "\nprop1 .&. prop2  .&. prop3"
prop1 .&. prop2 .&. prop3 |> Check.Quick
printfn "\nprop3 .&. prop1  .&. prop2"
prop3 .&. prop1 .&. prop2 |> Check.Quick

Output:

prop1 .&. prop2  .&. prop3
Falsifiable, after 1 test (0 shrinks) (StdGen (674262175, 297181018)):
Label of failing property: 1 = 3
Original:


prop3 .&. prop1  .&. prop2
Falsifiable, after 1 test (0 shrinks) (StdGen (675076347, 297181018)):
Label of failing property: 1 = 3
Original:

But this happens probably because it 'fails fast'.
</issue_description>

<agent_instructions>Read the issue carefully - should be relatively straightforward to fix. Add tests.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@kurtschelfthout Hm yes good point. Probably a bug here:

https://github.com/fscheck/FsCheck/blob/master/src/FsCheck/Testable.fs#L22

Some (all?) of those match arms should merge labels. (labels is where things like collect and classify add a string per test case)</comment_new>
<comment_new>@kurtschelfthout

I.e. for and this would imply combining the labels from both left and right (when Outcome.Passed), but for or we are only interested in the labels from the valid (Outcome.Passed) property or properties?

Yes, that sounds right.</comment_new>
<comment_new>@kurtschelfthout
I think we should merge in all cases, including or. One use of these stats is determining what sort of data coverage generator and tests are getting. since FsCheck is not short circuiting, we shouldn’t pretend it is. On 4 Mar 2025, at 23:35, Calum Sieppert @.***> wrote:
How about this
--- a/src/FsCheck/Testable.fs
+++ b/src/FsCheck/Testable.fs
@@ -21,23 +21,26 @@ open FsCheck.Internals
{ Outcome : Outcome
Stamp : list
Labels : Set
Arguments : list }
///Returns a new result that is Passed if and only if both this
///and the given Result are Passed.
static member internal ResAnd(l, r) =
match (l.Outcome,r.Outcome) with
| (Outcome.Failed ,) -> l //here a potential Failed in r is thrown away...
| (_,Outcome.Failed _) -> r

•    | (_,Outcome.Passed) -> l
  

•    | (Outcome.Passed,_) -> r
  

•    | (Outcome.Rejected,Outcome.Rejected) -> l //or r, whatever
  

•    | (Outcome.Rejected,_) -> l
  

•    | (_,Outcome.Rejected) -> r
  

•    | (Outcome.Passed,Outcome.Passed) ->
  

•        { l with
  

•            Stamp = List.append l.Stamp r.Stamp
  

•            Labels = Set.union l.Labels r.Labels }
  

  static member internal ResOr(l, r) =
  match (l.Outcome, r.Outcome) with
  | (Outcome.Failed ,) -> r
  | (,Outcome.Failed ) -> l
  | (Outcome.Passed,) -> l
  | (,Outcome.Passed) -> r
  | (Outcome.Rejected,Outcome.Rejected) -> l //or r, whatever

I don't think OR needs any change, the current code should keep the stamps from the first passing property, which I think would be the desired behaviour, to mimic short-circuiting?—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

rynoV left a comment (#629)
How about this
--- a/src/FsCheck/Testable.fs
+++ b/src/FsCheck/Testable.fs
...

• Fixes Using Collect and Classify in conjunction with And or Or #629

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.