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

Misleading exception when type parameter of mocked class is internal #192

Closed
bannmann opened this issue Jul 24, 2015 · 4 comments
Closed

Comments

@bannmann
Copy link

For new Mock<MockedClass>(), MockedClass needs to be accessible to DynamicProxy. If it isn't, Moq produces a helpful exception message.

When mocking a generic class, say new Mock<Container<Whatever>>, both Container and Whatever need to be accessible to DynamicProxy. However, if the second type parameter ("Whatever") is not accessible, the exception message is very much misleading - it says

Type to mock must be an interface or an abstract or non-sealed class.

I lost an hour isolating the real reason for this exception. Please change Moq to return an exception message similar to the first case, mentioning the correct type ("Whatever").

The following NUnit test class demonstrates the issue (provided the InternalsVisibleTo attribute was not set on the assembly):

class InternalClass
{ }

public class PublicClass
{ }

public class Container<T>
{
    public T Contents { get; set; }
}

[TestFixture]
public class MoqBug
{
    [Test]
    public void TestInternalClass()
    {
        var mock = new Mock<InternalClass>().Object;
        /* Exception message (good):
         * Type Your.Namespace.InternalClass is not visible to
         * DynamicProxy. Can not create proxy for types that are not
         * accessible. Make the type public, or internal and mark your
         * assembly with (...)
         */
    }

    [Test]
    public void TestInternalClassAsTypeParam()
    {
        var mock = new Mock<Container<InternalClass>>().Object;
        /* Exception message (bad):
         * Type to mock must be an interface or an abstract or non-sealed class.
         */
    }

    [Test]
    public void TestPublicClass()
    {
        var mock = new Mock<PublicClass>().Object;
        // Works fine.
    }

    [Test]
    public void TestPublicClassAsTypeParam()
    {
        var mock = new Mock<Container<PublicClass>>().Object;
        // Works fine.
    }
}
@stakx
Copy link
Contributor

stakx commented Jun 4, 2017

With the current version of Moq, the exception messages in your tests come directly from DynamicProxy, not from Moq.

The exception message for the TestInternalClass test reads:

Message: Castle.DynamicProxy.Generators.GeneratorException : Can not create proxy for type InternalClass because it is not accessible. Make it public, or internal and […].

The exception message for the TestInternalClassAsTypeParam test reads:

Castle.DynamicProxy.Generators.GeneratorException : Can not create proxy for type Container`1[[InternalClass, …]] because type InternalClass is not accessible. Make it public, or internal and […].

So what you're asking for has already been fixed in DynamicProxy.

@stakx stakx closed this as completed Jun 4, 2017
@wbalbal
Copy link

wbalbal commented Mar 20, 2018

@stakx nigga! u have a record on closing issue. you should wait for responses first.

@stakx
Copy link
Contributor

stakx commented Mar 20, 2018

@RipPer56:

u have a record on closing issue.

Yes, for the simple reason that I have been the most active collaborator here during the last year or so.

Closing issues isn't a bad thing. It doesn't by itself imply any kind of disagreement with the issue, nor that the OP should shut up, nor any other form of hostility. It simply means that nothing more can be done at the moment. It also isn't necessarily a permanent act: People can still comment, and if it turns out that we're not done yet, issues can be reopened. On the whole, closing issues makes it easier for us to know what's currently happening, and what we need to deal with that people are still actively interested in.

Regarding the present issue, what purpose would've been served by keeping it open for longer when it has demonstrably become obsolete in the meantime?

you should wait for responses first.

Regarding the present issue, exactly what kind of response should I have waited for?

If you feel that you can do a better job, why not prove it by hopping on board and get to work? There are many open source projects, Moq included, that would love to have more active collaborators! Let us know if you're interested, e.g. over in Moq's Gitter chat! 🎆

@wbalbal
Copy link

wbalbal commented Mar 23, 2018

@Staks

I've seen you everywhere responding to issues and i respect that. But don't be such a crybaby about a small remarque. Closing issue is a big deal because other collaborators and members filter only open issues. And in many cases, especially in roslyn repository, people are closing issues that are really helpful for the community! So for me i think closing an issue should be the responsability of its creator or in the case that he forgets about it ☺

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

No branches or pull requests

3 participants