Some operations on non virtual members should throw an exception

134 views
Skip to first unread message

Simon

unread,
Nov 7, 2010, 9:11:59 PM11/7/10
to NSubstitute
Using the example on your home page I have three unit tests.

Two work and one doesnt.

I understand that it is non virtual methods cannot be proxied with
DP2.
But it would be nice NSubstitute could tell me that it is not
possible. The problem is that syntactically I can write code to assert
something works but even if it fails NSubstitute doesnt notify me. It
makes it very easy to write a unit that doesnt really test anything.


[TestFixture]
public class Class1
{
[Test]
[ExpectedException(typeof(CallNotReceivedException))]
public void WithInterface()
{
var calculator = Substitute.For<ICalculator>();

calculator.Add(1, 2);
calculator.Received().Add(2, 2);

}
[Test]
[ExpectedException(typeof(CallNotReceivedException))]
public void WithNonVirtual()
{
var calculator = Substitute.For<NonVirtualCalculator>();

calculator.Add(1, 2);
calculator.Received().Add(2, 2);

}
[Test]
[ExpectedException(typeof(CallNotReceivedException))]
public void WithVirtual()
{
var calculator = Substitute.For<VirtualCalculator>();

calculator.Add(1, 2);
calculator.Received().Add(2, 2);

}
}

public interface ICalculator
{
int Add(int x, int y);
}
public class NonVirtualCalculator
{
public int Add(int x, int y){return x+y;}
}
public class VirtualCalculator
{
public virtual int Add(int x, int y) {return x+y;}
}

David Tchepak

unread,
Nov 7, 2010, 10:04:53 PM11/7/10
to nsubs...@googlegroups.com
Hi Simon,

Thanks for this. It's a known issue that we're keen to fix. The problem is that because we can't intercept the call it is pretty difficult to work out a non-virtual member has been referenced in this way (via Received() or Returns()). We're looking at some trickery to get this working but haven't found a good solution yet. In the meantime if you've got any suggestions I'd love to hear them! :)

Regards,
David


--
You received this message because you are subscribed to the Google Groups "NSubstitute" group.
To post to this group, send email to nsubs...@googlegroups.com.
To unsubscribe from this group, send email to nsubstitute...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/nsubstitute?hl=en.


Simon Cropp

unread,
Nov 8, 2010, 12:11:41 AM11/8/10
to nsubs...@googlegroups.com
and again cause the previous file was too big.


On Mon, Nov 8, 2010 at 4:09 PM, Simon Cropp <simon...@gmail.com> wrote:
> Cant think of a way to fix DP2
>
> But you could have a helper method that validates peoples unit tests.
> Attached is a proof of concept using Mono Cecil.

NonVirtualNSubstitute.zip

David Tchepak

unread,
Nov 8, 2010, 12:38:25 AM11/8/10
to nsubs...@googlegroups.com
That's a really interesting idea. Thanks a lot for this.

Forgive the naïve question, but would it be possible to do this at runtime within Received() and Returns() methods? If we could find the method body where Received/Returns was called from and then check it was called on a valid member that would be awesome. (Or even if that was possible, would it be crazily slow?)

Simon Cropp

unread,
Nov 8, 2010, 6:46:27 PM11/8/10
to nsubs...@googlegroups.com
David

what you are suggesting would involve the following
1 a stack walk to get the calling method (Easy)
2 somehow map that location in the stack to an IL instruction (Not
sure how difficult)
3 check forward in the IL to see if the next call is on a non virtual
method (Easy)

Off the top of my head I am not sure how to do step 2 but all the
information is there so with a big enough hammer it should be
possible.

As for "crazily slow". I think after the first call it would be
feasible. But the first call would be expensive. The initial parse of
the assemblies IL as well as loading all the required types would mean
the first time u do this check it would add some fairly significant
time. I know I am talking in relative terms so let me try to equate it
to some real numbers. My machine is a 2.8GHz I7. With my initial
sample from the attachment the execution takes 200ms for a first run.
0ms (after rounding down) for a second run. So you suggestion should
be possible in under a ms per call. But are u willing to take the
initial hit?

You could have an API that people should call at startup that
"Initialises" everything. But if u are forcing them to do this u may
as well just do the testing at that point as per my initial
suggestion.

Of course the bigger hammer is to just take the target assembly and
change all instance members to virtual using Cecil. :)

David Tchepak

unread,
Nov 8, 2010, 7:14:50 PM11/8/10
to nsubs...@googlegroups.com
We've considered using Cecil (probably via LinFu AOP) to just rewrite the DLL, but we'd need to manage running the postweaving automagically (might too much to ask people to set that up themselves? For me, I just want to reference a DLL and go) and I'm also unsure how well it works with strong named assemblies. I don't really know how to do that; would need to unload referenced assemblies at runtime and re-reference postweaved DLLs?

I'll definitely have a play around with these ideas, but pretty sure it won't make v1 release (it's on the list for vNext). In the meantime am happy to get all opinions and any contributions that come through. :)

If you do get the validator helper method stuff working let me know and I'll link to it from the docs. 

Thanks for your ideas and feedback. They're very appreciated. :)
Cheers,
David

Simon Cropp

unread,
Nov 8, 2010, 10:59:50 PM11/8/10
to nsubs...@googlegroups.com
What you are suggesting is not possible from the perspective of
nsubstitute. The reason being that by the time u get to the
nsubstitute context the assembly in question is already loaded. Since
u cant unload an assembly u are stuck.

I had a chat to philiplaureano on twitter and it looks like LinFu AOP
wont work for you either. It basically re-writes the target assembly
at post build time. So you would be forcing a hard dependency on Linfu
whenever someone wants to use NSubstitute ie their real production
code needs to be processed by linfu to make it unit testable. Not
something you would want?? And the scenario where you want to mock a
type from a 3rd party assembly is even more complicated.

And I agree with you "I just want to reference a DLL and go". I really
like that I dont need have anything installed or hack proj files to
get nsubstitute work.

I think the first step it would suffice to tell people when they are
mocking something that is not virtual.

Simon

unread,
Jan 4, 2011, 2:28:03 AM1/4/11
to NSubstitute
Dave

So I dont have to worry about this problem anymore I wrote this
http://code.google.com/p/virtuosity/

On Nov 9 2010, 2:59 pm, Simon Cropp <simon.cr...@gmail.com> wrote:
> What you are suggesting is not possible from the perspective of
> nsubstitute. The reason being that by the time u get to the
> nsubstitute context the assembly in question is already loaded. Since
> u cant unload an assembly u are stuck.
>
> I had a chat to philiplaureano on twitter and it looks like LinFu AOP
> wont work for you either. It basically re-writes the target assembly
> at post build time. So you would be forcing a hard dependency on Linfu
> whenever someone wants to use NSubstitute ie their real production
> code needs to be processed by linfu to make it unit testable. Not
> something you would want?? And the scenario where you want to mock a
> type from a 3rd party assembly is even more complicated.
>
> And I agree with you "I just want to reference a DLL and go". I really
> like that I dont need have anything installed or hack proj files to
> get nsubstitute work.
>
> I think the first step it would suffice to tell people when they are
> mocking something that is not virtual.
>
> On Tue, Nov 9, 2010 at 11:14 AM, David Tchepak <tche...@gmail.com> wrote:
> > We've considered using Cecil (probably via LinFu AOP) to just rewrite the
> > DLL, but we'd need to manage running the postweaving automagically (might
> > too much to ask people to set that up themselves? For me, I just want to
> > reference a DLL and go) and I'm also unsure how well it works with strong
> > named assemblies. I don't really know how to do that; would need to unload
> > referenced assemblies at runtime and re-reference postweaved DLLs?
> > I'll definitely have a play around with these ideas, but pretty sure it
> > won't make v1 release (it's on the list for vNext). In the meantime am happy
> > to get all opinions and any contributions that come through. :)
> > If you do get the validator helper method stuff working let me know and I'll
> > link to it from the docs.
> > Thanks for your ideas and feedback. They're very appreciated. :)
> > Cheers,
> > David
>
> >> On Mon, Nov 8, 2010 at 4:38 PM, David Tchepak <tche...@gmail.com> wrote:
> >> > That's a really interesting idea. Thanks a lot for this.
> >> > Forgive the naïve question, but would it be possible to do this at
> >> > runtime
> >> > within Received() and Returns() methods? If we could find the method
> >> > body
> >> > where Received/Returns was called from and then check it was called on a
> >> > valid member that would be awesome. (Or even if that was possible, would
> >> > it
> >> > be crazily slow?)
>
> >> > On Mon, Nov 8, 2010 at 4:11 PM, Simon Cropp <simon.cr...@gmail.com>
> >> > wrote:
>
> >> >> and again cause the previous file was too big.
>
> >> >> On Mon, Nov 8, 2010 at 4:09 PM, Simon Cropp <simon.cr...@gmail.com>
> >> >> wrote:
> >> >> > Cant think of a way to fix DP2
>
> >> >> > But you could have a helper method that validates peoples unit tests.
> >> >> > Attached is a proof of concept using Mono Cecil.
>
> >> >> > On Mon, Nov 8, 2010 at 2:04 PM, David Tchepak <tche...@gmail.com>
> >> >> > wrote:
> >> >> >> Hi Simon,
> >> >> >> Thanks for this. It's a known issue that we're keen to fix. The
> >> >> >> problem
> >> >> >> is
> >> >> >> that because we can't intercept the call it is pretty difficult to
> >> >> >> work
> >> >> >> out
> >> >> >> a non-virtual member has been referenced in this way (via Received()
> >> >> >> or
> >> >> >> Returns()). We're looking at some trickery to get this working but
> >> >> >> haven't
> >> >> >> found a good solution yet. In the meantime if you've got any
> >> >> >> suggestions I'd
> >> >> >> love to hear them! :)
> >> >> >> Regards,
> >> >> >> David
>

David Tchepak

unread,
Jan 4, 2011, 5:11:58 AM1/4/11
to nsubs...@googlegroups.com
Nice work! Thanks for letting us know.

FWIW I'm intending to look at this problem again as soon as my holidays finish, but sounds like Virtuosity will fix the root cause, rather than any basic detection we can do within NSub.

Cheers,
David
Reply all
Reply to author
Forward
0 new messages