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

[WIP] Add GreedyMockAttribute #1033

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

michalkowal
Copy link

@michalkowal michalkowal commented May 2, 2018

Closes #766

I prepared also abstract MockCustomizeAttribute for any further attributes (like for xUnit, NUnit etc)

@michalkowal
Copy link
Author

michalkowal commented May 2, 2018

I also have proposal to add similar "greedy" functionalities into other mocking projects like Rhino, FakeItEasy etc. Maybe it is not very important functionality, but then most projects will have similar bunch of functionalities.
Of course I can do that

@zvirja
Copy link
Member

zvirja commented May 2, 2018

@michalkowal Thanks a lot for giving your attention here! 🥇

I gave it a quick look and would like to share a few thoughts FWIW:

  • The following customization implementation might not work with the ConfigureMembers = true customization mode:
    return new ConstructorCustomization(parameter.ParameterType, new GreedyMockConstructorQuery());
    If you wrap argument with GreedyMock attribute, the generated object will not be auto-configured.
  • There is the ongoing issue Easy constructor customization for Moq class mocks #1013 to refactor the way we activate mocks. That should be an enabler for this PR (as you want to customize the mock activation without touching the further configuration actions). I have that change in my TODO list and hope to take a look in a week or two.
  • If you are in progress, please ensure to add the [WIP] PR title prefix. This way I know that PR is not ready for review and you are still working on it 😉

I also have proposal to add similar "greedy" functionalities into other mocking projects like Rhino, FakeItEasy etc

I'd say that Rhino is completely dead, but for FakeItEasy and NSubstitute it might make sense to consider the feature. However again, that could happen after the restructure (#1013, #1014) only.

@zvirja zvirja changed the title Add GreedyMockAttribute (#766) Add GreedyMockAttribute May 2, 2018
@michalkowal
Copy link
Author

@zvirja thanks for your review.
Right now, as you suggest, I stop activity here and I will monitor status off this two issues

@zvirja zvirja changed the title Add GreedyMockAttribute [WIP] Add GreedyMockAttribute May 2, 2018
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

2 participants