Skip to content

Commit

Permalink
Changed ExcludeNonBrowsable in IEquivalencyAssertionOptions into sepa…
Browse files Browse the repository at this point in the history
…rate properties IgnoreNonBrowsableOnSubject and ExcludeNonBrowsableOnExpectation.

Updated implementations of the interface accordingly.
Added new selection rule ExcludeNonBrowsableMembersRule.cs and updated SelfReferenceEquivalencyAssertionOptions.SelectionRules to inject an instance of it after AllPropertiesSelectionRule and AllFieldsSelectionRule but before explicit member selection rules.
Updated the ToString implementation in SelfReferenceEquivalencyAssertionOptions.cs to consider ignoreNonBrowsableOnSubject and excludeNonBrowsableOnExpectation independently.
Removed processing of options.ExcludeNonBroswable from StructuralEqualityEquivalencyStep's GetMembersFromExpectation, since this is now handled by selection rules.
Added tests that exercise the situation where a non-browsable field or property is explicitly included after a call to ExcludingNonBrowsableMembers. The current logic actually doesn't provide a way to do a wildcard inclusion of all properties or fields if any Include call targeting a specific member is included, but the tests have been written so that they'll continue to effectively test this function if that changes in the future.
Adjusted existing tests in SelectionRulesSpecs.cs to use IgnoringNonBrowsableMembersOnSubject where appropriate, and to verify that ExcludingNonBrowsableMembers does _not_ cause non-browsable members on the subject to be ignored.
  • Loading branch information
logiclrd committed Mar 16, 2022
1 parent e06dfe1 commit c05137a
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 32 deletions.
Expand Up @@ -61,7 +61,9 @@ public IEnumerable<IEquivalencyStep> UserEquivalencySteps

public MemberVisibility IncludedFields => inner.IncludedFields;

public bool ExcludeNonBrowsable => inner.ExcludeNonBrowsable;
public bool IgnoreNonBrowsableOnSubject => inner.IgnoreNonBrowsableOnSubject;

public bool ExcludeNonBrowsableOnExpectation => inner.ExcludeNonBrowsableOnExpectation;

public bool CompareRecordsByValue => inner.CompareRecordsByValue;

Expand Down
10 changes: 8 additions & 2 deletions Src/FluentAssertions/Equivalency/IEquivalencyAssertionOptions.cs
Expand Up @@ -74,10 +74,16 @@ public interface IEquivalencyAssertionOptions
MemberVisibility IncludedFields { get; }

/// <summary>
/// Gets a value indicating whether members marked with [EditorBrowsable]
/// Gets a value indicating whether members on the subject marked with [EditorBrowsable]
/// and an EditorBrowsableState of Never should be treated as though they don't exist.
/// </summary>
bool IgnoreNonBrowsableOnSubject { get; }

/// <summary>
/// Gets a value indicating whether members on the expectation marked with [EditorBrowsable]
/// and an EditorBrowsableState of Never should be excluded.
/// </summary>
bool ExcludeNonBrowsable { get; }
bool ExcludeNonBrowsableOnExpectation { get; }

/// <summary>
/// Gets a value indicating whether records should be compared by value instead of their members
Expand Down
@@ -0,0 +1,16 @@
using System;
using System.Collections.Generic;
using System.Linq;

namespace FluentAssertions.Equivalency.Selection
{
internal class ExcludeNonBrowsableMembersRule : IMemberSelectionRule
{
public bool IncludesMembers => false;

public IEnumerable<IMember> SelectMembers(INode currentNode, IEnumerable<IMember> selectedMembers, MemberSelectionContext context)
{
return selectedMembers.Where(member => member.IsBrowsable);
}
}
}
Expand Up @@ -56,7 +56,8 @@ public abstract class SelfReferenceEquivalencyAssertionOptions<TSelf> : IEquival

private MemberVisibility includedProperties;
private MemberVisibility includedFields;
private bool excludeNonBrowsable;
private bool ignoreNonBrowsableOnSubject;
private bool excludeNonBrowsableOnExpectation;

private bool compareRecordsByValue;

Expand All @@ -81,7 +82,8 @@ protected SelfReferenceEquivalencyAssertionOptions(IEquivalencyAssertionOptions
useRuntimeTyping = defaults.UseRuntimeTyping;
includedProperties = defaults.IncludedProperties;
includedFields = defaults.IncludedFields;
excludeNonBrowsable = defaults.ExcludeNonBrowsable;
ignoreNonBrowsableOnSubject = defaults.IgnoreNonBrowsableOnSubject;
excludeNonBrowsableOnExpectation = defaults.ExcludeNonBrowsableOnExpectation;
compareRecordsByValue = defaults.CompareRecordsByValue;

ConversionSelector = defaults.ConversionSelector.Clone();
Expand Down Expand Up @@ -117,6 +119,11 @@ IEnumerable<IMemberSelectionRule> IEquivalencyAssertionOptions.SelectionRules
yield return new AllFieldsSelectionRule();
}

if (excludeNonBrowsableOnExpectation)
{
yield return new ExcludeNonBrowsableMembersRule();
}

foreach (IMemberSelectionRule rule in selectionRules)
{
yield return rule;
Expand Down Expand Up @@ -164,7 +171,9 @@ IEnumerable<IMemberSelectionRule> IEquivalencyAssertionOptions.SelectionRules

MemberVisibility IEquivalencyAssertionOptions.IncludedFields => includedFields;

bool IEquivalencyAssertionOptions.ExcludeNonBrowsable => excludeNonBrowsable;
bool IEquivalencyAssertionOptions.IgnoreNonBrowsableOnSubject => ignoreNonBrowsableOnSubject;

bool IEquivalencyAssertionOptions.ExcludeNonBrowsableOnExpectation => excludeNonBrowsableOnExpectation;

public bool CompareRecordsByValue => compareRecordsByValue;

Expand Down Expand Up @@ -322,7 +331,7 @@ public TSelf ExcludingProperties()
/// <returns></returns>
public TSelf IncludingNonBrowsableMembers()
{
excludeNonBrowsable = false;
excludeNonBrowsableOnExpectation = false;
return (TSelf)this;
}

Expand All @@ -334,7 +343,13 @@ public TSelf IncludingNonBrowsableMembers()
/// <returns></returns>
public TSelf ExcludingNonBrowsableMembers()
{
excludeNonBrowsable = true;
excludeNonBrowsableOnExpectation = true;
return (TSelf)this;
}

public TSelf IgnoringNonBrowsableMembersOnSubject()
{
ignoreNonBrowsableOnSubject = true;
return (TSelf)this;
}

Expand Down Expand Up @@ -724,6 +739,11 @@ public override string ToString()
.Append(useRuntimeTyping ? "runtime" : "declared")
.AppendLine(" types and members");

if (ignoreNonBrowsableOnSubject)
{
builder.AppendLine("- Do not consider members marked non-browsable on the subject");
}

if (isRecursive)
{
if (allowInfiniteRecursion)
Expand Down Expand Up @@ -753,15 +773,6 @@ public override string ToString()
builder.AppendLine("- Compare records by their members");
}

if (excludeNonBrowsable)
{
builder.AppendLine("- Exclude non-browsable members");
}
else
{
builder.AppendLine("- Include non-browsable members");
}

foreach (Type valueType in valueTypes)
{
builder.AppendLine($"- Compare {valueType} by value");
Expand All @@ -772,6 +783,15 @@ public override string ToString()
builder.AppendLine($"- Compare {type} by its members");
}

if (excludeNonBrowsableOnExpectation)
{
builder.AppendLine("- Exclude non-browsable members");
}
else
{
builder.AppendLine("- Include non-browsable members");
}

foreach (IMemberSelectionRule rule in selectionRules)
{
builder.Append("- ").AppendLine(rule.ToString());
Expand Down
Expand Up @@ -82,9 +82,7 @@ public class StructuralEqualityEquivalencyStep : IEquivalencyStep
where match is not null
select match;

// This excludes non-browsable members from the subject. If a member is marked non-browsable
// in the expectation, it gets excluded in GetMembersFromExpectation.
if (config.ExcludeNonBrowsable)
if (config.IgnoreNonBrowsableOnSubject)
{
query = query.Where(member => member.IsBrowsable);
}
Expand All @@ -103,13 +101,6 @@ public class StructuralEqualityEquivalencyStep : IEquivalencyStep
new MemberSelectionContext(comparands.CompileTimeType, comparands.RuntimeType, options));
}

// This excludes non-browsable members from the expectation. If a member is marked non-browsable
// in the subject, it gets excluded in FindMatchFor.
if (options.ExcludeNonBrowsable)
{
members = members.Where(member => member.IsBrowsable);
}

return members;
}
}
Expand Down
4 changes: 3 additions & 1 deletion Tests/Benchmarks/UsersOfGetClosedGenericInterfaces.cs
Expand Up @@ -69,7 +69,9 @@ private class Config : IEquivalencyAssertionOptions

public MemberVisibility IncludedFields => throw new NotImplementedException();

public bool ExcludeNonBrowsable => throw new NotImplementedException();
public bool IgnoreNonBrowsableOnSubject => throw new NotImplementedException();

public bool ExcludeNonBrowsableOnExpectation => throw new NotImplementedException();

public bool CompareRecordsByValue => throw new NotImplementedException();

Expand Down
90 changes: 86 additions & 4 deletions Tests/FluentAssertions.Equivalency.Specs/SelectionRulesSpecs.cs
Expand Up @@ -1724,14 +1724,29 @@ public void When_non_browsable_property_differs_excluding_non_browsable_members_
}

[Fact]
public void When_property_is_non_browsable_only_in_subject_excluding_non_browsable_members_should_make_it_succeed()
public void When_property_is_non_browsable_only_in_subject_excluding_non_browsable_members_should_not_make_it_succeed()
{
// Arrange
var subject = new ClassWhereMemberThatCouldBeNonBrowsableIsNonBrowsable() { PropertyThatMightBeNonBrowsable = 0 };
var expectation = new ClassWhereMemberThatCouldBeNonBrowsableIsBrowsable() { PropertyThatMightBeNonBrowsable = 1 };

// Act
Action action =
() => subject.Should().BeEquivalentTo(expectation, config => config.ExcludingNonBrowsableMembers());

// Assert
action.Should().Throw<XunitException>().WithMessage("Expected property subject.PropertyThatMightBeNonBrowsable to be 1, but found 0.*");
}

[Fact]
public void When_property_is_non_browsable_only_in_subject_ignoring_non_browsable_members_on_subject_should_make_it_succeed()
{
// Arrange
var subject = new ClassWhereMemberThatCouldBeNonBrowsableIsNonBrowsable() { PropertyThatMightBeNonBrowsable = 0 };
var expectation = new ClassWhereMemberThatCouldBeNonBrowsableIsBrowsable() { PropertyThatMightBeNonBrowsable = 1 };

// Act & Assert
subject.Should().BeEquivalentTo(expectation, config => config.ExcludingNonBrowsableMembers());
subject.Should().BeEquivalentTo(expectation, config => config.IgnoringNonBrowsableMembersOnSubject());
}

[Fact]
Expand All @@ -1746,14 +1761,29 @@ public void When_property_is_non_browsable_only_in_expectation_excluding_non_bro
}

[Fact]
public void When_field_is_non_browsable_only_in_subject_excluding_non_browsable_members_should_make_it_succeed()
public void When_field_is_non_browsable_only_in_subject_excluding_non_browsable_members_should_not_make_it_succeed()
{
// Arrange
var subject = new ClassWhereMemberThatCouldBeNonBrowsableIsNonBrowsable() { FieldThatMightBeNonBrowsable = 0 };
var expectation = new ClassWhereMemberThatCouldBeNonBrowsableIsBrowsable() { FieldThatMightBeNonBrowsable = 1 };

// Act
Action action =
() => subject.Should().BeEquivalentTo(expectation, config => config.ExcludingNonBrowsableMembers());

// Assert
action.Should().Throw<XunitException>().WithMessage("Expected field subject.FieldThatMightBeNonBrowsable to be 1, but found 0.*");
}

[Fact]
public void When_field_is_non_browsable_only_in_subject_ignoring_non_browsable_members_on_subject_should_make_it_succeed()
{
// Arrange
var subject = new ClassWhereMemberThatCouldBeNonBrowsableIsNonBrowsable() { FieldThatMightBeNonBrowsable = 0 };
var expectation = new ClassWhereMemberThatCouldBeNonBrowsableIsBrowsable() { FieldThatMightBeNonBrowsable = 1 };

// Act & Assert
subject.Should().BeEquivalentTo(expectation, config => config.ExcludingNonBrowsableMembers());
subject.Should().BeEquivalentTo(expectation, config => config.IgnoringNonBrowsableMembersOnSubject());
}

[Fact]
Expand Down Expand Up @@ -1904,6 +1934,58 @@ public void When_field_is_missing_from_expectation_excluding_non_browsable_membe
// Act & Assert
subject.Should().BeEquivalentTo(expected, opt => opt.ExcludingNonBrowsableMembers());
}

[Fact]
public void When_non_browsable_members_are_excluded_it_should_still_be_possible_to_explicitly_include_non_browsable_field()
{
// Arrange
var subject =
new ClassWithNonBrowsableMembers()
{
NonBrowsableField = 1,
};

var expectation =
new ClassWithNonBrowsableMembers()
{
NonBrowsableField = 2,
};

// Act
Action action =
() => subject.Should().BeEquivalentTo(
expectation,
opt => opt.IncludingFields().ExcludingNonBrowsableMembers().Including(e => e.NonBrowsableField));

// Assert
action.Should().Throw<XunitException>().WithMessage("Expected field subject.NonBrowsableField to be 2, but found 1.*");
}

[Fact]
public void When_non_browsable_members_are_excluded_it_should_still_be_possible_to_explicitly_include_non_browsable_property()
{
// Arrange
var subject =
new ClassWithNonBrowsableMembers()
{
NonBrowsableProperty = 1,
};

var expectation =
new ClassWithNonBrowsableMembers()
{
NonBrowsableProperty = 2,
};

// Act
Action action =
() => subject.Should().BeEquivalentTo(
expectation,
opt => opt.IncludingProperties().ExcludingNonBrowsableMembers().Including(e => e.NonBrowsableProperty));

// Assert
action.Should().Throw<XunitException>().WithMessage("Expected property subject.NonBrowsableProperty to be 2, but found 1.*");
}
}

private class ClassWithNonBrowsableMembers
Expand Down

0 comments on commit c05137a

Please sign in to comment.