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

boolTask<bool>, PropertyTask<Property> #684

Open
brianrourkeboll opened this issue Oct 7, 2024 · 2 comments
Open

boolTask<bool>, PropertyTask<Property> #684

brianrourkeboll opened this issue Oct 7, 2024 · 2 comments

Comments

@brianrourkeboll
Copy link

brianrourkeboll commented Oct 7, 2024

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.

@kurtschelfthout
Copy link
Member

kurtschelfthout commented Oct 8, 2024 via email

@ploeh
Copy link
Member

ploeh commented Oct 8, 2024

I agree that the behaviour reported here looks undesirable.

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

No branches or pull requests

3 participants