-
Notifications
You must be signed in to change notification settings - Fork 728
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
Addition of DictionaryContainsKeyValuePairConstraint #3486
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks fine. We'll need a second review from the @nunit/framework-team though and if you have nothing you want to add, you'll need to remove the WIP.
@rprouse / @nunit/framework-team |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I think it looks good, but I've some minor suggestions. Also we need to rerun "NUnit Framework CI", but I think a new commit or force push will do the trick
src/NUnitFramework/tests/Constraints/DictionaryContainsKeyValueConstraintTests.cs
Outdated
Show resolved
Hide resolved
src/NUnitFramework/framework/Constraints/DictionaryContainsKeyValuePairConstraint.cs
Outdated
Show resolved
Hide resolved
src/NUnitFramework/tests/Constraints/DictionaryContainsKeyValueConstraintTests.cs
Outdated
Show resolved
Hide resolved
src/NUnitFramework/framework/Constraints/DictionaryContainsKeyConstraint.cs
Outdated
Show resolved
Hide resolved
src/NUnitFramework/framework/Constraints/DictionaryContainsKeyValuePairConstraint.cs
Outdated
Show resolved
Hide resolved
src/NUnitFramework/framework/Constraints/DictionaryContainsKeyConstraint.cs
Outdated
Show resolved
Hide resolved
…ValuePairConstraint.cs Co-Authored-By: Mikkel Nylander Bundgaard <mikkelbu@users.noreply.github.com>
…eConstraintTests.cs Co-Authored-By: Mikkel Nylander Bundgaard <mikkelbu@users.noreply.github.com>
…ValuePairConstraint.cs Co-Authored-By: Mikkel Nylander Bundgaard <mikkelbu@users.noreply.github.com>
…Constraint.cs Co-Authored-By: Mikkel Nylander Bundgaard <mikkelbu@users.noreply.github.com>
…eConstraintTests.cs
Thanks for supplying this PR @Falco20019, and sorry about the slow feedback from our side. |
Thanks for updating the PR. I totally forgot that I had open suggestions. |
Fixes #3470
Just a first idea to resolve it. Not sure if I implemented
WithValue
inDictionaryContainsKeyConstraint
correctly. Also not sure if usingKeyValuePair<TKey, TValue>
is better or using the approach already used for properties (ResolvableConstraintExpression
?) is better. This seemed to have worked forDoes.ContainKey.WithValue
and alsoDoes.Not.ContainKey.WithValue
.Looking forward to feedback :)