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

Allow transitive dependencies #86

Merged
merged 6 commits into from Aug 12, 2020
Merged

Allow transitive dependencies #86

merged 6 commits into from Aug 12, 2020

Conversation

tkellogg
Copy link
Collaborator

Moq allows passing constructor args to mock instances via
new Mock<Foo>(arg1, arg2, ...). This pull request enables doing this
for automocker, enhancing the "auto" part of "automocker".

Why do this? Because forcing every class to have an interface seems
unreasonable. Feel free to subscribe to whatever philosophy you prefer,
but the interface-to-class pairing does generate a lot of boilerplate.
Just as Moq allows as many scenarios as possible, Moq.Contrib should
also.

Moq allows passing constructor args to mock instances via
`new Mock<Foo>(arg1, arg2, ...)`. This pull request enables doing this
for automocker, enhancing the "auto" part of "automocker".

Why do this? Because forcing every class to have an interface seems
unreasonable. Feel free to subscribe to whatever philosophy you prefer,
but the interface-to-class pairing does generate a lot of boilerplate.
Just as Moq allows as many scenarios as possible, Moq.Contrib should
also.
@tkellogg tkellogg requested review from adamhewitt627 and Keboo and removed request for adamhewitt627 June 24, 2020 22:21
@Keboo Keboo requested a review from adamhewitt627 June 25, 2020 05:38
Keboo
Keboo previously approved these changes Jun 25, 2020
}
else
{
#pragma warning disable CA1825
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #87 as a proposed cleanup instead of disabling this here.

@@ -1,7 +1,7 @@
namespace Moq.AutoMock.Tests.Util
{
#pragma warning disable CA1801, CA1812 //is an internal class that is apparently never instantiated
internal class With3Parameters
public class With3Parameters
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a bit of a disjoint here. If this should be public then we should remove the CA1812 above and update the comment, or this could go back to internal

@tkellogg
Copy link
Collaborator Author

Hold up on merging this. It doesn't handle recursive dependencies... working on it.

@adamhewitt627
Copy link
Collaborator

LGTM so far - what is the version number impact? Thinking particularly of the .Use<T>(null).

@tkellogg
Copy link
Collaborator Author

Alright, I handled recursive object graphs by throwing an error. We could probably get more robust in that area, but this is a big start.

@adamhewitt627 I believe this doesn't break any behavior. This uses (basically) the public .Get(childType) for dependencies, so it'll use the normal resolution rules to obtain an instance. That means if you did any form of .Use(), it'll use that registered value. It's basically a full DI/IoC container now, except it mocks instead of throwing an exception when it runs into an unknown type.

Moq.AutoMock.Tests/DescribeCreateInstance.cs Outdated Show resolved Hide resolved
Moq.AutoMock/ResolutionContext.cs Outdated Show resolved Hide resolved
Moq.AutoMock/ResolutionContext.cs Outdated Show resolved Hide resolved
Moq.AutoMock/Resolvers/MockResolutionContext.cs Outdated Show resolved Hide resolved
adamhewitt627 and others added 2 commits August 6, 2020 22:52
Co-authored-by: Kevin B <Keboo@users.noreply.github.com>
Standardize test objects as public rather than suppressing errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants