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

Add ForConstraint to IAssertionScope #1918

Closed
ITaluone opened this issue Apr 29, 2022 · 4 comments · Fixed by #2324
Closed

Add ForConstraint to IAssertionScope #1918

ITaluone opened this issue Apr 29, 2022 · 4 comments · Fixed by #2324

Comments

@ITaluone
Copy link
Contributor

ITaluone commented Apr 29, 2022

Before you file a bug, have you:

  • Tried upgrading to newest version of Fluent Assertions, to see if your issue has already been resolved and released?
  • Checked existing open and closed issues, to see if the issue has already been reported?
  • Tried reproducing your problem in a new isolated project?
  • Read the documentation?
  • Considered if this is a general question and not a bug?. For general questions please use StackOverflow.

Description

Is there a good reason why ForConstraint is not available in an IAssertionScope, but only in the AssertionScope implementation?
Reason for this change is, that you can not chain an ForConstraint after a Then, resulting in quite longish sets of Execute.Assertion...

Complete minimal example reproducing the issue

Complete means the code snippet can be copied into a unit test method in a fresh C# project and run.
Minimal means it is stripped from code not related to reproducing the issue.

E.g.

Execute.Assertion
    .BecauseOf(because, becauseArgs)
    .WithExpectation($"Some expectation {0}}{{reason}}, ", parameter)
    .ForCondition(somethingShouldBeTrue)
    .FailWith("but cool error.")
    .Then
    .ForConstraint(occurrenceConstraint, actualCount) // this line here
    .FailWith($"Other cool error {{0}}{{reason}}, but found {{1}}.",
        coolParameter, bla);

Expected behavior:

Chaining a ForConstraint should also work after Then

Actual behavior:

Does not :)

Versions

  • Which version of Fluent Assertions are you using? 6.6.0
@jnyrup
Copy link
Member

jnyrup commented Apr 29, 2022

I can't remember at the top of my head why we didn't also include it on IAssertionScope in v6.
Adding it now is technically a breaking change.

@IT-VBFK
Copy link
Contributor

IT-VBFK commented Apr 30, 2022

Ok, so if we add it at v7 take the following also into consideration: for now you have to include .WithExpectation() twice in the statement. First place: as shown in the example and second before the .ForConstraint(). The only difference is: the second .WithExpectation() includes the reportable for the occurrence {expectedOccurrence}.

To handle this, I have two possible solutions:

  1. Add a new method e.g. .WithReportable() or something like that, where you can pass down the reportable WithExpectation() should care of
  2. remove the {expectedOccurrence} only at .FailWith() after a .ForConstraint()

Hope this is clear :)

Execute.Assertion
    .BecauseOf(because, becauseArgs)
    .WithExpectation($"Some expectation {0}}{{reason}}, ", parameter)
    .WithReportabe("expectedOccurrence")
    .ForCondition(somethingShouldBeTrue)
    .FailWith("but cool error.")
    .Then
    .ForConstraint(occurrenceConstraint, actualCount) // this line here
    .FailWith($"Other cool error {{0}}{{reason}}, but found {{1}}.",
        coolParameter, bla);

@IT-VBFK
Copy link
Contributor

IT-VBFK commented Sep 26, 2023

While implementing this I changed my mind about the second .WithExpectation(), because

  • it affects mostly extensions
  • you want a different expectation message sometimes

@dennisdoomen
Copy link
Member

This is also related to #2253

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

4 participants