FsCheck icon indicating copy to clipboard operation
FsCheck copied to clipboard

`bool` ≈ `Task<bool>`, `Property` ≉ `Task<Property>`

Open brianrourkeboll opened this issue 4 months ago • 2 comments

The behavior added in #634 to address #633 feels confusing to me when contrasted with the behavior of tests that return Task<bool>, especially for xUnit/NUnit tests annotated with PropertyAttribute.

That is:

  1. Tests marked with PropertyAttribute that return bool fail when the bool is false.
  2. Tests marked with PropertyAttribute that return Task<bool> fail when the bool is false.
  3. Tests marked with PropertyAttribute that return Property fail when the Property is falsy.
  4. Tests marked with PropertyAttribute that return Task<Property> pass when the Property is falsy.

I would expect 4 to fail just as 1–3 do.

// Fails as expected.
[Property]
public bool Test(bool b) =>
    b && !b;

// Fails as expected.
[Property]
public Property Test(bool b) =>
    Prop.Label(b && !b, "Some label.");

// Fails as expected.
[Property]
public Task<bool> Test(bool b) =>
    Task.FromResult(b && !b);

// Passes. I would expect this to fail.
[Property]
public Task<Property> Test(bool b) =>
    Task.FromResult(Prop.Label(b && !b, "Some label."));

// Passes. I would expect this to fail.
[Property]
public Task<Property> Test(bool b) =>
    Task.FromResult(Prop.ToProperty(b).And(!b));
// Fails as expected.
[<Property>]
let test1 b =
    b && not b

// Fails as expected.
[<Property>]
let test2 b =
    (b && not b) |> Prop.label "Some label."

// Fails as expected.
[<Property>]
let test3 b =
    Task.FromResult (b && not b)

// Passes. I would expect this to fail.
[<Property>]
let test4 b =
    Task.FromResult ((b && not b) |> Prop.label "Some label.")

// Passes. I would expect this to fail.
[<Property>]
let test5 b =
    Task.FromResult (b .&. not b)

I understand the original desire not to need to explicitly upcast Task<unit> to Task, but the difference in treatment between Task<bool> and Task<Property> is dangerous, because it is far too easy to write a test that returns Task<Property> and have it pass for the wrong reason.

That is, a developer might expect the test to fail if the Property inside the returned Task<Property> was falsy, just as such a test would fail for a false or falsy bool, Task<bool>, or Property — but the test returning Task<Property> would actually always pass as long as no exceptions were thrown.

Is there is any chance we could make Task<Property> behave like Task<bool> and Property here? Or, ideally, Task<'T> when 'T is testable behave like 'T?

I'd be willing to open a PR.

CC @ploeh, @kurtschelfthout.

brianrourkeboll avatar Oct 07 '24 22:10 brianrourkeboll