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

Create nested-equals matcher #218

Merged
merged 6 commits into from
Feb 1, 2024
Merged

Conversation

gilvan-reis
Copy link
Member

@gilvan-reis gilvan-reis commented Jan 24, 2024

Create nested-equals function to match nested maps with equals and providing a documentation closer to the code about the nested behavior for matchers.

Closes #217

Comment on lines 38 to 39
This solves a common need of matching nested maps with strict equality.
See also: `match-with`.
Copy link
Member Author

Choose a reason for hiding this comment

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

Feel free to suggest a better explanation here

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe something more like the following which explains it in broader terms and mentions the fact that there are defaults that are being overridden:

A matcher that always uses the equals matcher at every level of nesting. Useful given that matchers usually only change the first level of the data they are applied to, leaving nested data to use the default matcher of that type of data. For instance, this can be used to assert that any nested map has exactly the same keys and matching values as provided in the expected, and no more.

Note: this excludes functions, which continue to be invoked as predicates instead of compared via the equals matcher

Copy link
Member Author

Choose a reason for hiding this comment

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

I have added regex exclusion too. Feel free to correct me if I'm wrong

@philomates
Copy link
Collaborator

other possible names for deep-equals:

  • equals-as-default: has the benefit where default keys the reader to the fact that there is a default that isn't always equals.
  • always-equals
  • strict

As a note, the midje implementation had a similar thing, match-equals, but the name isn't so great because it conflates the match construct with the combinators.

@gilvan-reis gilvan-reis changed the title Create deep-equals matcher Create strictly-equals matcher Jan 25, 2024
@gilvan-reis gilvan-reis marked this pull request as ready for review January 25, 2024 17:02
@gilvan-reis
Copy link
Member Author

I think we should have equals in the function name, so I decided to follow with strictly-equals WDYT?

Copy link
Collaborator

@philomates philomates left a comment

Choose a reason for hiding this comment

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

Nice, thanks for the PR!

Can you add the matcher to the list in the readme?: https://github.com/nubank/matcher-combinators#built-in-matchers

Also the https://github.com/nubank/matcher-combinators#overriding-default-matchers mentions a match-with example that could be replaced with strictly-equals. Could you add a line there that says "For convenience we've also added this as a built-in matcher called strictly-equals?

src/cljc/matcher_combinators/matchers.cljc Outdated Show resolved Hide resolved
Comment on lines 38 to 39
This solves a common need of matching nested maps with strict equality.
See also: `match-with`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe something more like the following which explains it in broader terms and mentions the fact that there are defaults that are being overridden:

A matcher that always uses the equals matcher at every level of nesting. Useful given that matchers usually only change the first level of the data they are applied to, leaving nested data to use the default matcher of that type of data. For instance, this can be used to assert that any nested map has exactly the same keys and matching values as provided in the expected, and no more.

Note: this excludes functions, which continue to be invoked as predicates instead of compared via the equals matcher

test/clj/matcher_combinators/matchers_test.clj Outdated Show resolved Hide resolved
@gilvan-reis
Copy link
Member Author

@philomates I have replaced the match-with example in the README. Feel free to inform me if you want to stay with both instead

@gilvan-reis
Copy link
Member Author

I have also added the versions changes to be validated

Copy link

@scottbale scottbale left a comment

Choose a reason for hiding this comment

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

LGTM but I'll defer to @philomates's review

Copy link
Collaborator

@philomates philomates left a comment

Choose a reason for hiding this comment

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

Looks good!
Was mulling the name over a little bit more and nested-equals came up. I like it because it refers to nesting, which is really where this differs from the default. Like the matcher used for functions and regex stays the same, but for any nested/container datastructure equals is used instead of whatever default.

What do you think? Or should we just stick with this?

@gilvan-reis gilvan-reis changed the title Create strictly-equals matcher Create nested-equals matcher Feb 1, 2024
@gilvan-reis gilvan-reis merged commit 93360be into master Feb 1, 2024
2 checks passed
@gilvan-reis gilvan-reis deleted the create-deep-equals-matcher branch February 1, 2024 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Common misconception about equals not being applied to maps
3 participants