Skip to content

Improved reporting of properties and field details in the equivalency API #1379

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 1 commit into from
Nov 1, 2020
Merged

Improved reporting of properties and field details in the equivalency API #1379

dennisdoomen merged 1 commit into from
Nov 1, 2020

Conversation

@dennisdoomen
Copy link
Member

@dennisdoomen dennisdoomen commented Aug 16, 2020 •
edited
Loading

The equivalency assertions will now include the type of the member and whether it involves a field or property.

This required significant refactorings:

  • The properties of IEquivalencyValidationContext have been combined into the INode hierarchy and the two Reason and Tracer classes
  • Moved a lot of logic related to the object graph location into the Node, Field and Property classes.
  • The SelectedMemberInfo class has been removed, since it main user, IMemberInfo has been flattened.
  • Several methods that took an IMemberInfo, but could also act on other objects than a property or field now take an IObjectInfo

Because the final design evolved while doing the refactorings, I had to give up on nice and clean commits.

  • Rename the SelectedMemberInfo hierarchy to IMember, Field and Property
  • Move the member path and description into IMember
  • Combine the because and arguments into a reason class
  • Move the tracing parts from the context class to a dedicated class
  • See if we can remove RootIsCollection from IEquivalencyValidationContext
  • Move IsRoot into IMember
  • Check out the weird usage in CollectionMemberAssertionRuleDecorator.
  • Why does IEquivalencyValidationContext inherit from IMemberInfo
  • Ensure INode.Path has no public setter
  • Refactor EquivalencyValidationContext.AsNestedMember
  • Refactor some of the code that gets or sets the IMember properties into more functional methods
  • Does IMember really need GetValue and the access modifier getters?
  • Derive INode.Description from its state instead of setting it externally
  • Figure out why Tracer.WriteLine needs to rewrite the path
  • Is INode.Clone really necessary
  • Why do we need WithoutInitialIndexQualifier?
  • Check that tracing works as expected
  • Does the Node hierarchy need GetHashCode/Equals
  • Check review comments from Jonas
  • Ensure all new members/types are documented
  • Add release notes

Follow-up PR

  • Move depth calculation inside ShouldCompareMembersThisDeep into INode
  • Move the GetExpectationType calculation into IEquivalencyContext.ExpectedType.

jnyrup
jnyrup reviewed Aug 16, 2020
View reviewed changes
Src/FluentAssertions/Equivalency/CollectionMemberAssertionRuleDecorator.cs Outdated
Subject = context.Subject
};

adjustedCopy.SelectedMember.Path = CollectionMemberMemberInfo.GetAdjustedPropertyPath(context.SelectedMember.Path);
Copy link
Member

@jnyrup jnyrup Aug 16, 2020

Choose a reason for hiding this comment

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

Should the Description also still be set?

Copy link
Member Author

@dennisdoomen dennisdoomen Aug 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Ah yes.. No, it's a weird piece of code. It did not update the description at all (which is what Clone does right now). I've added to the bullet list in this PR to see what it really tries to do.

@dennisdoomen dennisdoomen force-pushed the CleanerEquivalencyValidator branch 3 times, most recently from 721a1d1 to b632fc5 Compare August 23, 2020 18:04
@dennisdoomen dennisdoomen force-pushed the CleanerEquivalencyValidator branch 7 times, most recently from bea6ba3 to 87aaed7 Compare August 30, 2020 18:28
@dennisdoomen dennisdoomen force-pushed the CleanerEquivalencyValidator branch 2 times, most recently from 042ae8a to 8594916 Compare September 5, 2020 16:57
@dennisdoomen dennisdoomen force-pushed the CleanerEquivalencyValidator branch 4 times, most recently from 9ca026f to 7776cd3 Compare September 25, 2020 19:48
@dennisdoomen dennisdoomen force-pushed the CleanerEquivalencyValidator branch 8 times, most recently from 63cff70 to 67d6d33 Compare September 30, 2020 19:37
@dennisdoomen dennisdoomen force-pushed the CleanerEquivalencyValidator branch 3 times, most recently from 673552c to 88f9c84 Compare October 17, 2020 19:37
@dennisdoomen dennisdoomen force-pushed the CleanerEquivalencyValidator branch 2 times, most recently from 21e00e5 to 8763de7 Compare October 20, 2020 17:18
@dennisdoomen dennisdoomen mentioned this pull request Oct 27, 2020
Closed
5 tasks
@dennisdoomen dennisdoomen force-pushed the CleanerEquivalencyValidator branch from 202fb5a to be1e444 Compare October 27, 2020 20:04
jnyrup
jnyrup reviewed Oct 27, 2020
View reviewed changes
@dennisdoomen dennisdoomen changed the title [WIP] Cleaning the equivalency validator Improved reporting of properties and field details in the equivalency API Oct 28, 2020
@dennisdoomen dennisdoomen requested a review from jnyrup October 28, 2020 06:15
@dennisdoomen
Copy link
Member Author

dennisdoomen commented Oct 28, 2020

dennisdoomen requested a review from jnyrup 2 hours ago

Sorry, did not notice that you already reviewed it.

@dennisdoomen dennisdoomen force-pushed the CleanerEquivalencyValidator branch 2 times, most recently from 99ee786 to 4e8994d Compare October 28, 2020 18:39
@dennisdoomen dennisdoomen requested review from jnyrup and removed request for jnyrup October 28, 2020 18:55
jnyrup
jnyrup reviewed Oct 31, 2020
View reviewed changes
@jnyrup
Copy link
Member

jnyrup commented Oct 31, 2020

docs/_pages/extensibility.md mentions the removed SelectedMemberInfo

These two now removed class names gave me a good laugh

  • MemberInfoSelectedMemberInfo
  • CollectionMemberMemberInfo

@logiclrd logiclrd mentioned this pull request Oct 31, 2020
Merged
5 tasks
logiclrd
logiclrd reviewed Oct 31, 2020
View reviewed changes
@dennisdoomen dennisdoomen force-pushed the CleanerEquivalencyValidator branch from 4e8994d to 588f089 Compare October 31, 2020 20:55
@dennisdoomen dennisdoomen requested a review from jnyrup October 31, 2020 21:05
jnyrup
jnyrup reviewed Nov 1, 2020
View reviewed changes
@dennisdoomen dennisdoomen requested a review from jnyrup November 1, 2020 13:10
@dennisdoomen dennisdoomen force-pushed the CleanerEquivalencyValidator branch from 588f089 to 5f75642 Compare November 1, 2020 13:44
@dennisdoomen
Improved reporting of properties and field details in the equivalency…
cc10ae7 
… API

The equivalency assertions will now include the type of the member and whether it involves a field or property.

This required significant refactorings:
* The properties of `IEquivalencyValidationContext` have been combined into the `INode` hierarchy and the two `Reason` and `Tracer` classes
* Moved a lot of logic related to the object graph location into the `Node`, `Field` and `Property` classes.
* The `SelectedMemberInfo` class has been removed, since it main user, `IMemberInfo` has been flattened.
* Several methods that took an `IMemberInfo`, but could also act on other objects than a property or field now take an `IObjectInfo`
@dennisdoomen dennisdoomen force-pushed the CleanerEquivalencyValidator branch from 5f75642 to cc10ae7 Compare November 1, 2020 13:45
@dennisdoomen dennisdoomen merged commit c9008fd into fluentassertions:develop Nov 1, 2020
@dennisdoomen dennisdoomen deleted the CleanerEquivalencyValidator branch November 1, 2020 15:32
@jnyrup jnyrup mentioned this pull request Aug 16, 2021
Closed
@JordanWestern JordanWestern mentioned this pull request Sep 16, 2021
Merged
1 task
JordanWestern added a commit to cucumber/common that referenced this pull request Sep 16, 2021
@JordanWestern
Fix issues caused by major version update in fluent assertions library
de1a434 
changes seen here - fluentassertions/fluentassertions#1379
aurelien-reeves added a commit to cucumber/common that referenced this pull request Sep 17, 2021
@renovate @renovate-bot
@JordanWestern @aslakhellesoy @aurelien-reeves
Update dependency FluentAssertions to v6 (#1692)
d16862e 
* Update dependency FluentAssertions to v6

* Fix issues caused by major version update in fluent assertions library

changes seen here - fluentassertions/fluentassertions#1379

Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Jordan Western <j.western@tricentis.com>
Co-authored-by: Aslak Hellesøy <1000+aslakhellesoy@users.noreply.github.com>
Co-authored-by: Aurélien Reeves <aurelien.reeves@smartbear.com>
cukebot pushed a commit to cucumber/gherkin-dotnet that referenced this pull request Sep 17, 2021
@renovate @renovate-bot
@JordanWestern @aslakhellesoy @aurelien-reeves
Update dependency FluentAssertions to v6 (#1692)
0f4199e 
* Update dependency FluentAssertions to v6

* Fix issues caused by major version update in fluent assertions library

changes seen here - fluentassertions/fluentassertions#1379

Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Jordan Western <j.western@tricentis.com>
Co-authored-by: Aslak Hellesøy <1000+aslakhellesoy@users.noreply.github.com>
Co-authored-by: Aurélien Reeves <aurelien.reeves@smartbear.com>
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

+1 more reviewer

@logiclrd logiclrd logiclrd left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

breaking change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dennisdoomen @jnyrup @logiclrd