Skip to content

Issue 1404: Escape brackets for ContainSingle(...) #1406

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.

Sign up for GitHub

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

Jump to bottom
Merged
dennisdoomen merged 6 commits into from
Oct 29, 2020
Merged

Issue 1404: Escape brackets for ContainSingle(...) #1406

dennisdoomen merged 6 commits into from
Oct 29, 2020

Conversation

@Serg046
Copy link
Contributor

@Serg046 Serg046 commented Oct 10, 2020 •
edited
Loading

IMPORTANT

  • The code complies with the Coding Guidelines for C#.
  • The changes are covered by a new or existing set of unit tests which follow the Arrange-Act-Assert syntax such as is used in this example.
  • If the contribution adds a feature or fixes a bug, please update the release notes, which are published on the website.
  • If the contribution changes the public API the changes needs to be included by running AcceptApiChanges.ps1/AcceptApiChanges.sh.
  • If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.

@Serg046
Copy link
Contributor Author

Serg046 commented Oct 10, 2020

Fixes #1404

@jnyrup
Copy link
Member

jnyrup commented Oct 25, 2020

@Serg046 Sorry for not getting back to you lately

As described in #1404 (comment)
the proper solution is to avoid the eager formatting of predicate.Body.

Would you be willing to update the PR with that approach?
Please let me know if my description is imprecise.

@Serg046
Copy link
Contributor Author

Serg046 commented Oct 26, 2020

@jnyrup that's done.
I will also look for other places where the issue might occur including your list from the ticket. I will do it in a separate PR to keep this one fixing the original issue. But I will do it a bit later as I already started work on #1363. Unfortunately the issue is more than just ReflectedType vs DeclaredType and seems there is no a simple and elegant solution.

jnyrup
jnyrup requested changes Oct 26, 2020
View reviewed changes
Copy link
Member

@jnyrup jnyrup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates, I just have two small suggestions and then it should be ready to merge.

Serg046 and others added 2 commits October 26, 2020 19:40
@Serg046
Rename the test
753284e
@Serg046 @jnyrup
Apply suggestions from code review
2dd5bc9 
Co-authored-by: Jonas Nyrup <jnyrup@users.noreply.github.com>
@Serg046
Copy link
Contributor Author

Serg046 commented Oct 26, 2020

Renamed the second test (validate it too pls) and applied the notes, thank you 👍

@jnyrup jnyrup requested a review from dennisdoomen October 26, 2020 17:58
@jnyrup jnyrup linked an issue Oct 26, 2020 that may be closed by this pull request
When chaining ContainsSingle, assertion failed because of FormatException inside FluentAssertion instead of actual difference #1404
Closed
@dennisdoomen dennisdoomen merged commit c79b57a into fluentassertions:develop Oct 29, 2020
@jnyrup jnyrup mentioned this pull request Nov 17, 2020
Closed
@jnyrup jnyrup mentioned this pull request Mar 24, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jnyrup jnyrup jnyrup approved these changes

@dennisdoomen dennisdoomen dennisdoomen approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

When chaining ContainsSingle, assertion failed because of FormatException inside FluentAssertion instead of actual difference

3 participants

@Serg046 @jnyrup @dennisdoomen