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

Upgrade from 4.2.1409.1722 to 4.2.1507.0118 changed VerifyAll behavior #191

Closed
smith-it opened this issue Jul 15, 2015 · 10 comments
Closed
Milestone

Comments

@smith-it
Copy link

VerifyAll() fails for setups that weren't created following the upgrade. In particular, any mock made from an interface containing a get-only property fails on not satisfying a set on that property. All failing tests do not have setups for the methods/properties being reported as not matched.

Moq.MockVerificationException: Moq.MockVerificationException: The following setups were not matched:

IDbContext m => m.Database

where IDbContext is defined:

    public interface IDbContext : IDisposable
    {
        Database Database { get; }
        // Other methods...
    }

Additionally, there seems to be a recursive traversal of properties that are somehow getting automatically wired up as a setup. Here, IConfigurationManager defined as:

    public interface IConfigurationManager
    {
        NameValueCollection AppSettings { get; }
        // Other methods...
    }

Any tests using a mock of type IConfigurationManager produces the following failure message:

Moq.MockVerificationException: Moq.MockVerificationException: The following setups were not matched:

NameValueCollection m => m.AllKeys

NameValueCollection m => m.Count

NameValueCollection m => m.Keys

NameValueCollection m => m.Count

NameValueCollection m => m.SyncRoot

NameValueCollection m => m.IsSynchronized
@smith-it smith-it changed the title Upgrade from 4.2.1409.1722 to 4.2.1507.0118 broke a third of unit tests Upgrade from 4.2.1409.1722 to 4.2.1507.0118 broke unit tests Jul 15, 2015
@smith-it smith-it changed the title Upgrade from 4.2.1409.1722 to 4.2.1507.0118 broke unit tests Upgrade from 4.2.1409.1722 to 4.2.1507.0118 changed VerifyAll behavior Jul 15, 2015
@kzu
Copy link
Contributor

kzu commented Jul 16, 2015

Could you please provide a complete and bare-bones interface definition and accompanying (single) unit test with the relevant setup which fails?

Thanks!

@smith-it
Copy link
Author

Hey @kzu, thanks for the reply. I went through the process of creating a stand alone solution to reproduce the problem and discovered an additional detail. The problem manifests when chaining into a subdependency on setup where that subdependency has a get-only property exposed on the interface. Here's the repo code:

using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;

namespace MoqTest
{
    [TestClass]
    public class Tests
    {
        [TestMethod]
        public void DoSomething_NormalConditions_CallsDependency()
        {
            var mockDependency = new Mock<IDependency>();
            var service = new TestService( mockDependency.Object );
            mockDependency
                .Setup( m => m.CreateDependency().Foo() );

            service.DoSomething();

            mockDependency.VerifyAll();
        }
    }

    public class Payload
    {
    }

    public interface IDependency
    {
        ISubDependency CreateDependency();
    }

    public interface ISubDependency
    {
        void Foo();
        Payload Payload { get; }
    }

    public class TestService
    {
        private readonly IDependency _dependency;

        public TestService( IDependency dependency )
        {
            _dependency = dependency;
        }

        public void DoSomething()
        {
            _dependency.CreateDependency().Foo();
        }
    }
}

Here is the Test output:

Test method MoqTest.Tests.DoSomething_NormalConditions_CallsDependency threw exception: 

Moq.MockVerificationException: The following setups were not matched:

ISubDependency m => m.Payload


    at Moq.Mock.VerifyAll()
   at MoqTest.Tests.DoSomething_NormalConditions_CallsDependency() in TestClass.cs: line 19


This was a new solution in Visual Studio 2013 Update 4 targeting the .Net 4.5.2 framework. Here are the references used by the project:

    <Reference Include="Microsoft.VisualStudio.QualityTools.UnitTestFramework, Version=10.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" />
    <Reference Include="Moq, Version=4.2.1507.118, Culture=neutral, PublicKeyToken=69f491c39445e920, processorArchitecture=MSIL">
      <HintPath>..\packages\Moq.4.2.1507.0118\lib\net40\Moq.dll</HintPath>
      <Private>True</Private>
    </Reference>
    <Reference Include="System" />
    <Reference Include="System.Core" />
    <Reference Include="System.Xml.Linq" />
    <Reference Include="System.Data.DataSetExtensions" />
    <Reference Include="Microsoft.CSharp" />
    <Reference Include="System.Data" />
    <Reference Include="System.Xml" />

@smith-it
Copy link
Author

Any update available on this?

@kzu
Copy link
Contributor

kzu commented Jul 29, 2015

Nop.

It might be worth it to just verify what you really expect to have happened
though. The verify all behavior is much more aligned for strict mocks than
loose mocks (aka stubs).

@smith-it
Copy link
Author

That's disappointing since this works fine with a long range of previous versions. I'm not really interested to rewriting an entire unit test suite to adopt a new version of Moq. I hope this get addressed.

@kzu
Copy link
Contributor

kzu commented Jul 30, 2015

You can always stay with whichever version works for you.

Moq is pretty much a community driven project at this point, and it may be
that some of those contributions ended up affecting your scenario. If it's
really key for you to get the new contributions while still keeping this
behavior, it could be worth it to attempt to fix it and send a PR. We're
more that open to contributions.

Thanks

@kzu
Copy link
Contributor

kzu commented Feb 21, 2017

Marking as up-for-grabs since it doesn't seem to be a high-impact one.

@NecroFilja
Copy link

Having the same issue. About 200 tests started to failing.
It is really ugly that for 2 years there is no workaround.

And just to upgrade all packages to latest version I need to replace 200 VerifyAll on verifiable()/Verify

@stakx
Copy link
Contributor

stakx commented Jul 17, 2017

@ce-smith: I realise this was posted long ago, but I'd like to add my ¢2. Here's your unit test, abbreviated and slightly reordered:

var mockDependency = new Mock<IDependency>();
mockDependency.Setup(m => m.CreateDependency().Foo());
…
mockDependency.VerifyAll();

Two things are crucial for understanding why this test succeeded in version 4.2.1409.1722 but started failing in version 4.2.1507.118:

  • Setup(m => m.CreateDependency().Foo()) does not just set up what you see in that expression; it also sets up all properties in all traversed objects. Imagine that Moq calls subDependency.SetupAllProperties() behind the scenes (because it really does!). This means that subDependency.Payload gets set up, too, and will thus get checked by VerifyAll.

  • Except that it didn't: Your test only succeeded because (perhaps without you even knowing it) it relied on a regression bug present in in version 4.2.1507.118, which caused SetupAllProperties to not set up read-only properties (such as your Payload). Thanks to that bug, you could then call VerifyAll and Payload got ignored.

PR #137 (which was included in Moq 4.2.1502.911) addressed that bug, meaning that all properties now get set up by SetupAllProperties (as it should be) and that's why your test starts failing.

If you're asking to bring back the old behavior, that's essentially the same as saying, "Make SetupAllProperties skip read-only properties." This is not going to happen, because it would mean reintroducing a bug, and also because it's illogical. This is why I am going to close this issue. (But please keep on reading, I'm almost done!)

The only and perhaps most reasonable way forward is to consider a different breaking change, namely that a setup for an expression such as Setup(m => m.CreateDependency().Foo()) should only set up what is contained and visible in that very expression (i.e. no properties). I will open a new issue for this shortly. If you'd like to support that change, please "vote" for it in that new issue (e.g. with a 👍).

@stakx
Copy link
Contributor

stakx commented Sep 7, 2018

Due to the decision made in #681 and the corresponding change made to SetupAllProperties in #684, this issue will be fixed in Moq version >4.9.0.

/cc @ce-smith, @NecroFilja, @Arithmomaniac

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

4 participants