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

Unwrap TargetInvocationException #4515

Conversation

manfred-brands
Copy link
Member

Fixes #4481

Code for

Assert.That(new Dictionary<string, string>(), Does.ContainKey(null!));

Now results in:

  Message: 
    System.ArgumentNullException : Value cannot be null. (Parameter 'key')

  Stack Trace: 
    Dictionary`2.FindValue(TKey key)
    Dictionary`2.ContainsKey(TKey key)

There are two methods in Reflect.cs ending in WithTransparentExceptions.
These are used in Assert.Throws and equivalents and were not modified to unwrap.
Unwrapping all InnerExceptions would make it impossible to catch a TargetInvocationException thrown by user code.

This means that to validate that the Does.ContainKey(null!) throws an ArgumentNullException we
first have to test for the TargetInvocationException.
Beside testing NUnit, not many people will Assert an Assert.

Assert.That(() => Assert.That(new Dictionary<string, string>(), Does.ContainKey(null!)),
            Throws.TargetInvocationException.With.InnerException.InstanceOf<ArgumentNullException>());

Copy link
Member

@OsirisTerje OsirisTerje left a comment

Choose a reason for hiding this comment

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

I believe this looks good now.

@OsirisTerje
Copy link
Member

Feel free to merge

@manfred-brands manfred-brands merged commit 158d21f into nunit:master Oct 29, 2023
5 checks passed
@manfred-brands manfred-brands deleted the Issue4481_UnwrapTargetInvocationException branch October 29, 2023 00:47
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.

Does.ContainKey() throws an unintuitive exception for a null key
2 participants