Skip to content

Add IEnumeratable<KVP> support for ContainsKey(...).WithValue(...)#4854

Merged
manfred-brands merged 6 commits into
nunit:mainfrom
RenderMichael:contains
Oct 8, 2024
Merged

Add IEnumeratable<KVP> support for ContainsKey(...).WithValue(...)#4854
manfred-brands merged 6 commits into
nunit:mainfrom
RenderMichael:contains

Conversation

@RenderMichael

Copy link
Copy Markdown
Contributor

Fixes #4852

@RenderMichael RenderMichael changed the title Add IEnumeratable<KVP> for ContainsKey(...).WithValue(...) Add IEnumeratable<KVP>support for ContainsKey(...).WithValue(...) Oct 7, 2024
@RenderMichael RenderMichael changed the title Add IEnumeratable<KVP>support for ContainsKey(...).WithValue(...) Add IEnumeratable<KVP> support for ContainsKey(...).WithValue(...) Oct 7, 2024

@manfred-brands manfred-brands left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The test that fails (DictionaryContainsKeyValuePairConstraintTests.FailsWhenNotUsedAgainstADictionary) is one that expects the code to fail if it isn't an IDictionary.
The test uses an List<KeyValuePair<,>>` which is the support you added.

Rewrite that test to pass or delete it as you already made a new test.

@RenderMichael

Copy link
Copy Markdown
Contributor Author

I fixed the failing test, having a decent error message is still a good test to have.

Things look a lot neater with the suggestions implemented.

@manfred-brands manfred-brands left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@RenderMichael Most of it looks good now.
Only item I have is the dropping of IDictionary in the exceptions.
Please put that back in.

Comment thread src/NUnitFramework/tests/Constraints/DictionaryContainsKeyValueConstraintTests.cs Outdated
@RenderMichael

Copy link
Copy Markdown
Contributor Author

The error message has been fixed to refer to non-generic IDictionary again.

@manfred-brands manfred-brands left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@RenderMichael Thanks for your contribution. Looks good now.

@manfred-brands manfred-brands merged commit 95b5c7f into nunit:main Oct 8, 2024
@RenderMichael RenderMichael deleted the contains branch October 8, 2024 14:00
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.

Expand the usability of Contains.Key(...).WithValue(...)

2 participants