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

Mock.Of() and MockRepository.VerifyAll() #617

Closed
robrich opened this issue Apr 22, 2018 · 8 comments
Closed

Mock.Of() and MockRepository.VerifyAll() #617

robrich opened this issue Apr 22, 2018 · 8 comments

Comments

@robrich
Copy link

robrich commented Apr 22, 2018

Problem: Mocks created with Mock.Of() aren't in MockRepository.

var repo = new MockRepository(MockBehavior.Strict);
var mock = Mock.Get(Mock.Of<IService1>());
mock.Setup(m => m.Do());
repo.VerifyAll(); // <-- no exception thrown
mock.VerifyAll(); // <-- exception thrown here

Propose:

MockRepository.Add(Mock<T> mock) used like so:

var repo = new MockRepository(MockBehavior.Strict);
var mock = Mock.Get(Mock.Of<IService1>());
mock.Setup(m => m.Do());
repo.Add(mock);
repo.VerifyAll(); // <-- exception thrown here
@informatorius
Copy link
Contributor

informatorius commented May 2, 2018

Do you know the MockRepository.Create<Interface>() method?
You would call it like this, replace Mock.Of by repo.Create:

var repo = new MockRepository(MockBehavior.Strict);
var mock = Mock.Get(repo.Create<IService1>());
mock.Setup(m => m.Do());
repo.VerifyAll(); // <-- exception thrown here

@robrich
Copy link
Author

robrich commented May 2, 2018

I am familiar with repo.Create<T>(), but I'm having trouble weening people off the elegance of Mock.Of(). As noted in moq/Moq.AutoMocker#33 I'm looking for a way to add mocks to the repository's .VerifyAll() list that aren't created through repo.Create<T>(). Alternatively, I can keep my own list, but then I need a way to throw a single MockException for everything in the list, but there's no public constructor for MockException. https://github.com/moq/moq4/blob/master/Source/Obsolete/MockFactory.cs#L375 keeps everything so private.

@stakx
Copy link
Contributor

stakx commented May 2, 2018

@robrich:

Alternatively, I can keep my own list, but then I need a way to throw a single MockException for everything in the list, but there's no public constructor for MockException.

MockException isn't meant to be instantiable from outside Moq. Also, it isn't designed too well at this time with regard to aggregation of multiple mock errors, so even if you'd find a way into that exception type you'd probably end up pulling your hairs. :)

MockRepository.Add(Mock<T> mock) used like so:

It doesn't appear to make much sense to give a repository the ability to add any (externally created) mock to it. IMHO it would be much preferable to just do the obvious and add a method that corresponds to Mock.Of<T> to MockRepository.

Since the public API of Moq 4 is currently frozen due to Moq 5 being made ready for initial publication, we can't push such an API enhancement forward right now (even though it would be a trivial addition).

I shouldn't even suggest the following, as it intentionally breaks an abstraction... but you could do this:

sealed class MockOfRepository : MockRepository
{
    public MockOfRepository(MockBehavior defaultBehavior) : base(defaultBehavior) { }

    public T Of<T>(Expression<Func<T, bool>> predicate) where T : class
    {
        var mockObject = Mock.Of<T>(predicate);
        var mock = Mock.Get(mockObject);
        var mocks = (ICollection<Mock>)this.Mocks;
        mocks.Add(mock);
        return mockObject;
    }
}

Obviously, assuming that this.Mocks can be successfully cast to ICollection<Mock> isn't exactly clean code (this.Mocks's declared static type is IEnumerable<Mock>), and might break in a future version of Moq... but it does work. Judge yourself whether you want to go down that unsafe route.

@robrich
Copy link
Author

robrich commented May 16, 2018

Would you accept a PR to put mockRepository.Of<T>() into the v5 branch?

Agreed, this solution is perfect except for the obvious fragility of assuming unsafe internals that are already flagged to change.

@stakx
Copy link
Contributor

stakx commented May 16, 2018

/cc @kzu:

Would you accept a PR to put mockRepository.Of<T>() into the v5 branch?

@stakx
Copy link
Contributor

stakx commented May 24, 2018

@robrich - It appears that I've been away too long from Moq's code base, or from this issue... but I'm not exactly sure anymore why we've even been discussing this. Aren't the following existing methods what you're looking for?:

https://github.com/moq/moq4/blob/d4aea9c344330b35487a2780736b04167e5ecb4a/Source/Linq/MockRepository.cs#L18-L38

The only catch here is that these existing methods return an IQueryable<T> so you'll have to call .First() on it. For example:

MockRepository mocks = ...;
var foo = mocks.Of<IFoo>(f => ...).First();

@stakx
Copy link
Contributor

stakx commented Jun 2, 2018

@robrich - Can we close this issue, or is the above solution MockRepository.Of<T>(...).First() not an option for you?

@robrich
Copy link
Author

robrich commented Jun 8, 2018

Yes, we can close this. All else being equal, I still see value in making the .VerifyAll() process more SOLID, but that's definitely jumping the shark here.

@robrich robrich closed this as completed Jun 8, 2018
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

3 participants