search

eranpeer / FakeIt

C++ mocking made easy. A simple yet very expressive, headers only library for c++ mocking.
MIT License
1.23k stars 169 forks source link

Fix access violation that happens when Mock with destructor has more methods. #144

Closed ericlemes closed 5 years ago

ericlemes commented 6 years ago

If the offsets are all initialised to 0, ocasionally the destructors get the wrong offset.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-100.0%) to 0.0% when pulling 5fb5a874126db7a409982e544985f4cb23b97ef0 on ericlemes:master into 33022feda7d2d19a7c15ba786e0a8ff5df075a54 on eranpeer:master.

ericlemes commented 6 years ago

Bad coverage, but seems to be a problem in the tool.

ericlemes commented 6 years ago

I noticed also that the same tests will fail if they are compiled with /Zi instead of /ZI (Debug Information Format under C/C++ options).

I think either needs some more investigation or a documentation changed or a static_assert to avoid compiling with wrong options.

This has been tested on VC++ 15.6.x

eranpeer commented 5 years ago

This was a very important fix. I was looking for it for a long time. Thanks, that was great help.

ericlemes commented 5 years ago

@eranpeer good to know. It was definitely not a simple bug. The other comment about the /Zi on Windows also took me some serious weeks to spot. So, it would be good to know if you are interested in contributions because I have some heavy users and loads of interesting use cases, but since you were a bit unresponsive, we started using a fork.

eranpeer commented 5 years ago

Yes, I was unresponsive for a long time. Are you interested in being an admin in this project? This way you can be a part of the project owners and help me improve the responsiveness.

I'm currently too busy with other things.

On Wed, Jul 25, 2018, 09:01 Eric Lemes notifications@github.com wrote:

@eranpeer https://github.com/eranpeer good to know. It was definitely not a simple bug. The other comment about the /Zi on Windows also took me some serious weeks to spot. So, it would be good to know if you are interested in contributions because I have some heavy users and loads of interesting use cases, but since you were a bit unresponsive, we started using a fork.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eranpeer/FakeIt/pull/144#issuecomment-407806877, or mute the thread https://github.com/notifications/unsubscribe-auth/ACc8go8gYk42mND6tokdK5RhvL_kIUL_ks5uKJZpgaJpZM4UVa63 .

ericlemes commented 5 years ago

That would be really good. I just need to make sure we are aligned in terms of direction, to make sure I keep the right philosophy. But that would be good.

I personally don't like the idea of forking projects. I think making contributions and working together usually gives better results and stronger communities.

eranpeer commented 5 years ago

Great, I will add you as admin, you will be able to approve pull requests and push updates without me being involved.

eranpeer commented 5 years ago

Whenever you want to make functional changes it will be best if we have a discussion first. We want to keep the API and the code as simple and clean as possible. Obviously, everything must be fully tested. Beside that there is no big philosophy.

ericlemes commented 5 years ago

That's cool, thanks!

One thing I was thinking to add in terms of feature is something similar for AlwaysReturn and Return that handles classes without copy constructors. I've noticed you can workaround with AlwaysDo, but it is kind a lot of work.

eranpeer commented 5 years ago

I didn't find a workaround for classes without a copy constructor. If you know how to do it, it can be a great enhancement. Try building a POC first, I may be able to help it it is a lot of work.