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

Optionally include internal members in equivalency assertions #1575

Merged
merged 1 commit into from May 31, 2021

Conversation

dennisdoomen
Copy link
Member

@dennisdoomen dennisdoomen commented May 24, 2021

BeEquivalentTo will no longer include internal properties and fields, unless IncludingInternalProperties or IncludingInternalFields is used

Fixes #1205, #744

  • Release notes

@dennisdoomen dennisdoomen force-pushed the Fix/1205 branch 5 times, most recently from 6b43908 to 9f66ac0 Compare May 27, 2021 19:33
@dennisdoomen dennisdoomen changed the title [WIP] Optionally include internal members in equivalency assertions Optionally include internal members in equivalency assertions May 27, 2021
@dennisdoomen dennisdoomen requested a review from jnyrup May 27, 2021 19:34
Src/FluentAssertions/Common/TypeExtensions.cs Show resolved Hide resolved
return type
.GetProperties(AllInstanceMembersFlag)
.Where(property => property.GetMethod?.IsPrivate == false)
.Where(property => includeInternals || property.GetMethod?.IsAssembly == false)
Copy link
Member

Choose a reason for hiding this comment

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

How does this handle properties marked as private protected or protected internal?

Copy link
Member Author

Choose a reason for hiding this comment

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

AFAIK private protected is not relevant here, but protect internal was. I've added extra tests to cover this scenario.

Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering a bit about what that means to protected members.

We ignore protected members
unless it's protected internal
unless it private protected?

Src/FluentAssertions/Common/TypeExtensions.cs Outdated Show resolved Hide resolved
return type
.GetProperties(AllInstanceMembersFlag)
.Where(property => property.GetMethod?.IsPrivate == false)
.Where(property => includeInternals || property.GetMethod?.IsAssembly == false)
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering a bit about what that means to protected members.

We ignore protected members
unless it's protected internal
unless it private protected?

@dennisdoomen
Copy link
Member Author

I'm wondering a bit about what that means to protected members.
We ignore protected members
unless it's protected internal
unless it private protected?

Why do you care about protected members? They are not public, nor internal.

BeEquivalentTo will no longer include internal properties and fields, unless IncludingInternalProperties or IncludingInternalFields is used
@jnyrup
Copy link
Member

jnyrup commented May 30, 2021

I do not care about protected, I just mixed them up.
It made sense to me after seeing it visualized as a Venn Diagram like https://stackoverflow.com/a/22958035/1087627

@dennisdoomen dennisdoomen merged commit 4fccdd2 into fluentassertions:develop May 31, 2021
@dennisdoomen dennisdoomen deleted the Fix/1205 branch May 31, 2021 05:13
@jnyrup jnyrup linked an issue May 31, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BeEquivalentTo does not exclude internal members Redundant BindingFlags
2 participants