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

Fixing issues #161 and #163 #245

Merged
merged 11 commits into from
Aug 10, 2016
Merged

Fixing issues #161 and #163 #245

merged 11 commits into from
Aug 10, 2016

Conversation

vladonemo
Copy link
Contributor

This contains fix for the two issues causing StackOverflowException when using DefaultValue.Mock.
The idea of the fix is the following:

  • SetupAllProperties avoids loops in the graph of the properties. For example, if a mocked type contains a property of the same type, then this property is mocked with default value (null for reference types) instead. In fact, this was already addressed, but only in the direct child level.
  • The test from issue Stack overflow when setting up mock with default value set to mock. #163 was failing even after the fix due to the fact, that some of the .NET framework types do not implement ISerializable properly (e.g. System.Reflection.Module, System.Reflection.Assembly). So in practice, you are not able to mock IFoo with DefaultValue.Mock if the IFoo contains a property of type Assembly (or Module, or even Type) anywhere in the property graph, because Castle throws ArgumentException. To fix this one I introduced IsSerialiableMockable extension method and use it in the SetupAllProperties (via SerializableTypesValueProvider decorator)

@kzu
Copy link
Contributor

kzu commented May 9, 2016

Could you fix the conflicts by rebasing on master so I can take another look at merging it?

Thanks!

@vladonemo
Copy link
Contributor Author

should be ok now, thanks.

@kzu
Copy link
Contributor

kzu commented May 25, 2016

Hi @vladonemo sorry I broke again the merge-ability :(

I do see a large amount of changes that are just whitespace changes. Please make sure you have the EditorConfig VS extension and auto-format again from VS the files, all whitespace changes should be gone after that since the repo (master) now contains an .editorconfig file with the right settings.

Let me know if that works, thanks!

@vladonemo
Copy link
Contributor Author

I have rebased, but now the AppVeyor build fails. I can't build it locally either. Missing Castle in the references. Any idea why?

@kzu
Copy link
Contributor

kzu commented Jul 11, 2016

Castle reference should be fixed now, but another rebase is needed now :(

@vladonemo
Copy link
Contributor Author

vladonemo commented Jul 14, 2016

done. I had to fix the build error ince the XUnit was updated to v2. And removed one dummy file which I checked in by coincidence. Finally the build is green :)

@kzu
Copy link
Contributor

kzu commented Jul 14, 2016

The whitespaces changes are still there :(.

Please make sure you have the EditorConfig VS extension and auto-format again from VS the files, all whitespace changes should be gone after that since the repo (master) now contains an .editorconfig file with the right settings.

@vladonemo
Copy link
Contributor Author

oh, I'm sorry. I used different PC. I applied the EditorConfig rules now to all 6 files that I've touched. I hope it's fine now. I also have Resharper, so not sure whether it fights with its formatting rules ...

@kzu
Copy link
Contributor

kzu commented Jul 14, 2016

Looks good. I'm sorry you have to use R# ;)

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants