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

NSubstitute does not overrides private protected virtual methods #612

Closed
vadimart92 opened this issue Mar 10, 2020 · 4 comments
Closed

NSubstitute does not overrides private protected virtual methods #612

vadimart92 opened this issue Mar 10, 2020 · 4 comments
Labels
bug Reported problem with NSubstitute behaviour

Comments

@vadimart92
Copy link

vadimart92 commented Mar 10, 2020

Describe the bug
NSubstitute does not overrides private protected virtual methods.

To Reproduce

using System.Runtime.CompilerServices;
using NSubstitute;
using NUnit.Framework;

[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")]

namespace TestProject1
{
    public class PrivateProtectedSpec
    {
        private protected virtual void PrivateProtectedVirtualMethod() => throw new Exception("I Was Called");
        protected virtual void ProtectedVirtualMethod() => throw new Exception("I Was Called");

        [Test]
        public void TestPrivateProtectedCall() {
            var system = Substitute.For<PrivateProtectedSpec>();
            Assert.DoesNotThrow(() => system.ProtectedVirtualMethod(), "this one passes");
            Assert.DoesNotThrow(() => system.PrivateProtectedVirtualMethod(), "this fails");
        }

    }
}

Expected behaviour
Calls on private protected virtual methods should not result in calls to the base type.

Environment:

  • NSubstitute version: 4.2.1
  • NSubstitute.Analyzers version: CSharp 1.0.12
  • Platform: netcoreapp3.1 win64

Additional context
<PackageReference Include="NUnit" Version="3.8.1" />
<PackageReference Include="NUnit3TestAdapter" Version="3.9.0" />

@dtchepak
Copy link
Member

Thanks for raising this.

The MSDN documentation is not clear on this, but I don't think InternalsVisibleTo works with private protected. In that case NSubstitute will not be able to substitute for these members.

@tpodolak once this is confirmed, i'm not sure if this is worth adding to analyzers as normally the class will be in the assembly being tested (rather than the test assembly), so the members won't be directly callable anyway. WDYT?

@dtchepak dtchepak added the bug Reported problem with NSubstitute behaviour label Mar 10, 2020
@tpodolak
Copy link
Member

@dtchepak I dont see much value in adding this analyzer, this seems to me as a very edge case. Maybe if more issues like that will be reported we could consider writing the analyzer. One way or another, even with analyzer in place, the case shown in the issue wouldnt be reported - as there is no setup done for PrivateProtectedVirtualMethod

@CrispyDrone
Copy link

CrispyDrone commented Sep 12, 2020

@dtchepak

I just created a test solution with 2 projects, one has a class with a private protected member. It is accessible from the other project once it is marked with the InternalsVisibleTo attribute.

Also I went ahead and tested the same code in the Castle.Core project; I think it might be an issue there.

using System.Runtime.CompilerServices;
using Castle.Core.Internal;
using System;
using NUnit.Framework;
[assembly: InternalsVisibleTo(InternalsVisible.ToDynamicProxyGenAssembly2)]

namespace Castle.DynamicProxy.Tests
{
	[TestFixture]
	public class BasicClassProxyTestCase : BasePEVerifyTestCase
	{
		private protected virtual void PrivateProtectedThrowException() => throw new Exception("I was called");
		protected virtual void ProtectedThrowException() => throw new Exception("I was called");
		internal virtual void InternalThrowException() => throw new Exception("I was called");

		[Test]
		public void PrivateProtectedVirtualMethodsAreInterceptedCorrectly()
		{
			var proxy = generator.CreateClassProxy<BasicClassProxyTestCase>(
				new DoNothingAndReturnDefaultInterceptor()
			);

			Assert.DoesNotThrow(() => proxy.InternalThrowException());
			Assert.DoesNotThrow(() => proxy.ProtectedThrowException());
			Assert.DoesNotThrow(() => proxy.PrivateProtectedThrowException());
		}

		public class DoNothingAndReturnDefaultInterceptor : StandardInterceptor
		{
			protected override void PerformProceed(IInvocation invocation)
			{
				object returnValue = invocation.ReturnValue;

				if (returnValue != null)
				{
					var returnValueType = returnValue.GetType();

					if (returnValueType.IsValueType)
					{
						invocation.ReturnValue = Activator.CreateInstance(returnValueType);
					}
					else
					{
						invocation.ReturnValue = null;
					}
				}
			}
		}
	}
}

This test fails only on the method that's private protected.

@dtchepak
Copy link
Member

@CrispyDrone Thanks so much for chasing this up with Castle Project and for all your work on it.

Sorry for the late acknowledgement, I was just going through my inbox and found the notification of your comment. 🤦

I'll close this for now as it should be sorted by Castle Core 5.x release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Reported problem with NSubstitute behaviour
Projects
None yet
Development

No branches or pull requests

4 participants