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

Weird exception when mocking interface with SignalRHubAttribute applied #1308

Closed
kimbirkelund opened this issue Dec 7, 2022 · 10 comments
Closed

Comments

@kimbirkelund
Copy link

Hi

First of all thanks for a nice framework 👍

I've run into a weird problem. The following tests shows the problem, with the first test exposing the problem:

public class Test
{
    private readonly ITestOutputHelper _outputHelper;
    public Test(ITestOutputHelper outputHelper) { _outputHelper = outputHelper; }

    [Fact]
    public void Should_throw_NullReferenceException_when_mocking_wrapping_interface()
    {
        var mock = new Mock<INest<IThrow>>();
        var setup = mock.Setup(m => m.Get());
        var exception = setup.Invoking(s => s.Returns(new Mock<IThrow>().Object))
                             .Should().Throw<NullReferenceException>()
                             .Which;

        _outputHelper.WriteLine(exception.ToString());
    }

    [Fact]
    public void Should_not_throw_NullReferenceException_when_mocking_wrapping_interface()
    {
        var mock = new Mock<INest<IDoNotThrow>>();
        var setup = mock.Setup(m => m.Get());
        setup.Returns(new Mock<IDoNotThrow>().Object); // no exception occurs
    }

    [Fact]
    public void Should_not_throw_NullReferenceException_when_mocking_wrapping_interface_using_the_other_attribute()
    {
        var mock = new Mock<INest<IDoNotThrowEither>>();
        var setup = mock.Setup(m => m.Get());
        setup.Returns(new Mock<IDoNotThrowEither>().Object); // no exception occurs
    }

    [Fact]
    public void Should_not_throw_when_mocking_directly()
    {
        var mock = new Mock<IThrow>();
        var setup = mock.Setup(m => m.Get());
        setup.Returns("value"); // no exception occurs
    }
}

[CausesNullReferenceException(null)]
public interface IThrow
{
    string Get();
}

[CausesNullReferenceException(new string[0])]
public interface IDoNotThrow
{
    string Get();
}

[DoesNotCauseNullReferenceException(null)]
public interface IDoNotThrowEither
{
    string Get();
}

public interface INest<T>
{
    T Get();
}

[AttributeUsage(AttributeTargets.Interface, Inherited = false)]
public sealed class CausesNullReferenceExceptionAttribute : Attribute
{
    public CausesNullReferenceExceptionAttribute(string[]? p) { }
}

[AttributeUsage(AttributeTargets.Interface)]
public sealed class DoesNotCauseNullReferenceExceptionAttribute : Attribute
{
    public DoesNotCauseNullReferenceExceptionAttribute(string[]? p) { }
}

The only case the exception occurs is if the AttributeUsageAttribute.Inherited property is explicitly set to false (even though that is the default value) and the attribute has an array parameter that is null.

This is the exception (using LINQPad):
image

I'm using Moq version 4.18.3 and .NET 7. Also in use is Xunit and FluentAssertions.

@stakx
Copy link
Contributor

stakx commented Dec 8, 2022

Thanks for the error report, this will likely boil down to a problem in the DynamicProxy library. I'll investigate shortly and, if Moq indeed isn't the culprit, forward a modified report to their repository.

@stakx
Copy link
Contributor

stakx commented Dec 11, 2022

@kimbirkelund, I cannot reproduce any test failures. I had to change the following lines of code in order to get a runnable test:

-        var exception = setup.Invoking(s => s.Returns(new Mock<IThrow>().Object))
-                             .Should().Throw<NullReferenceException>()
-                             .Which;
+        var exception = Assert.Throws<NullReferenceException>(() => s.Returns(new Mock<IThrow>().Object));

Could you please re-post minimally complete repro code (ideally with no package references other than to Xunit and Moq) that has at least one failing test?

@kimbirkelund
Copy link
Author

Interesting. I've attached a project that shows the issue when running dotnet test on my machine.

And just to avoid any misunderstanding: all tests pass if the issue is repro'ed. The test with the assertion demonstrates the problem by catching the exception that shouldn't be thrown. I apologize if that wasn't clear by me original post.

I've further attached the result of running dotnet --info on my machine.

Let me know if you need anything further.

dotnetinfo.txt
BugRepro.zip

@stakx
Copy link
Contributor

stakx commented Dec 11, 2022

The test with the assertion demonstrates the problem by catching the exception that shouldn't be thrown.

Ah, I see. That's a somewhat unusual way to go about it :-) but in that case I can repro it. Will take another look!

@kimbirkelund
Copy link
Author

Sorry that wasn't clear. I thought it was the most precise way to show where the exception was thrown and what kind it was.

@stakx
Copy link
Contributor

stakx commented Dec 11, 2022

No big deal. I'm usually expecting a test failure to signal where something goes wrong. Also, your test name Should_throw_NullReferenceException_when_mocking_wrapping_interface suggested that a NullReferenceException would be the expected outcome, so when the test passed I didn't even look more closely at it.

Never mind though, I know what I need to look at now. :-)

@kimbirkelund
Copy link
Author

Yeah, that wasn't really thought through :-)

@stakx
Copy link
Contributor

stakx commented Dec 12, 2022

@kimbirkelund, this is indeed a bug in DynamicProxy. I've forwarded your bug report with simplified test cases, see castleproject/Core#637.

@stakx stakx removed the needs-repro label Dec 12, 2022
@stakx
Copy link
Contributor

stakx commented Dec 30, 2022

@kimbirkelund, I've just released Moq version 4.18.4, which contains a bugfix that should resolve your issue.

@stakx stakx closed this as completed Dec 30, 2022
@kimbirkelund
Copy link
Author

Good to hear. Looking forward to get it.

Thanks for your hard work 👍

@devlooped devlooped locked and limited conversation to collaborators Sep 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants