DictionaryContainsKeyValuePairConstraint doesn't work with `IDictionary<TKey, TValue>`

[pantosha]

DictionaryContainsKeyValuePairConstraint fails when actual object implements only IDictionary<TKey, TValue>, but not IDictionary:

[Test]
void Test()
{
    var dictionary = new TestDictionary();
    Assert.That(dictionary, new DictionaryContainsKeyValuePairConstraint(1, "hello"));
}

class TestDictionary : IDictionary<int, string>
{
    // Dictionary implementation
}

// Test fails with next error:
// 1) Error : NUnit.Framework.Constraints.DictionaryContainsKeyValuePairConstraintTests.ShouldCallContainsKeysMethodOnDictionary
// System.ArgumentException : Expected: IDictionary But was: TestDictionary (Parameter 'actual')

NUnit version: 3.13.2
Runner: NUnit3TestAdapter

It looks like the bug is here:

nunit/src/NUnitFramework/framework/Constraints/DictionaryContainsKeyValuePairConstraint.cs

Line 43 in 2c21bff

var dictionary = ConstraintUtils.RequireActual<IDictionary>(actual, nameof(actual));

[rprouse]

Too much of this code was first written way too long ago before generics, so I expect this isn't the only case of this. Good find.

[mikkelbu]

This code is actually not even one year old :). But I was surprised that System.Collections.IDictionary and System.Collections.Generic.IDictionary<TKey,TValue> are not related in some way.

[louis-z]

I'd be interested in taking a stab at it, if no one else is looking into it.

[mikkelbu]

@louis-z You are most welcome. I've assigned the issue to you, please write if you have questions or problems.

[louis-z]

@louis-z You are most welcome. I've assigned the issue to you, please write if you have questions or problems.

Though I'm not familiar with the codebase much, I just submitted an initial fix. Let me know whether the PR complies with this project's standards. Thanks @mikkelbu.

[ColonelPanic-42]

Is there any chance of backporting this fix to the current release?

[stevenaw]

The change seems fairly low-risk at a glance. @mikkelbu do you have any concerns?

[mikkelbu]

@stevenaw No. I think this should be quite safe.