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

Should only add public interfaces #123

Merged
merged 8 commits into from
Aug 6, 2014
Merged

Should only add public interfaces #123

merged 8 commits into from
Aug 6, 2014

Conversation

scott-xu
Copy link
Contributor

@scott-xu scott-xu commented Aug 5, 2014

If the interface is not public, the castle dynamic proxy will throw:
Castle.DynamicProxy.Generators.GeneratorException : Type
System.Data.Entity.Internal.Linq.IInternalQueryAdapter 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 [assembly:
InternalsVisibleTo(InternalsVisible.ToDynamicProxyGenAssembly2)]
attribute.

If the interface is not public, the castle dynamic proxy will throw:
Castle.DynamicProxy.Generators.GeneratorException : Type
System.Data.Entity.Internal.Linq.IInternalQueryAdapter 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 [assembly:
InternalsVisibleTo(InternalsVisible.ToDynamicProxyGenAssembly2)]
attribute.
use tabs instead of spaces
@scott-xu
Copy link
Contributor Author

scott-xu commented Aug 6, 2014

Hi @kzu , sorry to bother you again, the previous pull request #119 caused some breaking issues.

  • If the interface is not public, the castle dynamic proxy will throw Type is not visible to DynamicProxy. This is caused by included non-public interfaces when generate proxies
  • Issue85 FooTest broken. This is caused by method's declaring type changed to interface
  • _326 ShouldSupportMockingWinFormsControl broken. This is caused by included COM interfaces when generate proxies which is not supported by castle dynamic proxy

This pull request fixed the issues listed above and passed all UT and also added 2 UTs. Could you please help take a look?

Thanks in advance!

@kzu
Copy link
Contributor

kzu commented Aug 6, 2014

Looks good. I guess it will be hard to let those that are InternalsVisibleTo go through still?

@@ -33,8 +33,9 @@ internal class InvokeBase : IInterceptStrategy
{
public InterceptionAction HandleIntercept(ICallContext invocation, InterceptorContext ctx, CurrentInterceptContext localctx)
{
if (invocation.Method.DeclaringType == typeof(object) ||
invocation.Method.DeclaringType.IsClass && !invocation.Method.IsAbstract && ctx.Mock.CallBase
if (invocation.Method.DeclaringType == typeof(object) || // interface proxy
Copy link
Contributor

Choose a reason for hiding this comment

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

This condition makes my brain hurt :(. Not sure how to improve it significantly though :(

kzu added a commit that referenced this pull request Aug 6, 2014
Should only add public interfaces
@kzu kzu merged commit 4ade20b into devlooped:master Aug 6, 2014
@AlexArchive AlexArchive mentioned this pull request Aug 6, 2014
@scott-xu
Copy link
Contributor Author

scott-xu commented Aug 7, 2014

Probably it could: check the interface's assembly and see if the assembly has the corresponding attribute.

发自我的 iPhone

在 2014年8月7日,1:16,"Daniel Cazzulino" notifications@github.com 写道:

Looks good. I guess it will be hard to let those that are InternalsVisibleTo go through still?


Reply to this email directly or view it on GitHub.

@kzu
Copy link
Contributor

kzu commented Aug 7, 2014

That would be great :)
On Aug 6, 2014 10:12 PM, "Scott Xu" notifications@github.com wrote:

Probably it could: check the interface's assembly and see if the assembly
has the corresponding attribute.

发自我的 iPhone

在 2014年8月7日,1:16,"Daniel Cazzulino" notifications@github.com 写道:

Looks good. I guess it will be hard to let those that are
InternalsVisibleTo go through still?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub
#123 (comment).

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.

None yet

2 participants