Moq4: Regression in .net 4.7.1

[StasPerekrestov]

Hello,

just FYI. There is a potential regression in .net 4.7.1 that significantly affects moq Setup API logic.

[stakx]

@StasPerekrestov - thanks for linking that issue and letting us know!

Regarding Moq's use of StackTrace in the code location you linked to (MethodCall.GetFileInfo): it's a nice to have debug feature, but nonessential, and I feel that it might eventually have to be dropped anyway—it's not supported on .NET Core, and it's already a noticeable performance bottleneck on the .NET Framework (IIRC; I checked a few months ago with PerfView, but don't remember the exact details).

Let's leave this issue open so we can gather some benchmark data on just how badly Moq's performance is affected—by its use of StackTrace in general, and under .NET 4.7.1 specifically—and then figure out if we should do something about it.

[Caleb9]

@stakx I don't have access to machine without latest Windows 10 Creators update, so I can't really give you comparison times, but this issue hit me really hard. For example I have some tests that setup things on Mock<MethodInfo> (need to test something that depends on Castle.Core's IInvocation type). Setting up a single get property takes about 1500 ms right now with pure Moq. Since I am using AutoFixture.AutoMoq package, there is a lot of setup happening behind the scenes. This combination (IInvocation, MethodInfo and AutoFixture.AutoMoq) makes some of my test to take 15 seconds to execute now, while they were perfectly fine and fast unit tests before the update... Not sure how likely it is for Microsoft to fix it on their side, but I'd like to give my vote for removing that feature if you say it's nonessential. :)

[stakx]

@Caleb9: Your vote has been heard. :)

I would say it is very likely that Microsoft will fix this. However, I realise that until this happens, the situations is... problematic.

Moq currently collects source file information for each setup by default. I think this needs to become an opt-in feature: it's there if people want this diagnostic information; and it won't incur any performance cost for people who don't need it.

Given that version 4.8.0 of Moq isn't quite ready yet I'm wondering whether a hotfix release is required...?

[stakx]

@Caleb9, @StasPerekrestov - I have just published a hotfix release of Moq (version 4.7.145). Source file information collection via StackTrace is now disabled by default, meaning the .NET Framework regression should no longer degrade test execution speed.

If you want the old behavior back, i.e. you want better diagnostic messages when doing a mockRepository.Verify[All], you need to opt back in via mock[Repository].Switches |= Switches.CollectDiagnosticFileInfoForSetups.

You shouldn't have any more trouble regarding this issue after you upgrade to 4.7.145 or later. If you do, please post back here!