Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
OkkeHendriks opened this issue May 8, 2023 · 3 comments
Open

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

OkkeHendriks opened this issue May 8, 2023 · 3 comments
Labels

Comments

@OkkeHendriks
Copy link

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'.

@kurtschelfthout
Copy link
Member

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)

@OkkeHendriks
Copy link
Author

OkkeHendriks commented Feb 7, 2024

Ran into this one again today.
I think the labels from all Outcomes which 'matter' should be combined.

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?

Do the labels only really matter in case the total combined property is Outcome.Passed?

@kurtschelfthout
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants