dascandy / hippomocks

GNU Lesser General Public License v2.1
196 stars 67 forks source link

[feature request] Add support to mock nonvirtual member functions. #24

Closed ghost closed 7 years ago

ghost commented 10 years ago

I have wrote test example to mock member function to prove It's possible. Can You add more comfortable way to do such mocks? Member functions do not differ very much from free functions, so why not to intercept it the same way? They have __thiscall calling conversion, which can be intercepted exactly by the same "jmp" instruction as common function.

#include "hippomocks.h"

#include <iostream>

class Test
{
public:
  int mem_fun()
  {  return 100; }
};

int main()
{
  MockRepository mocks;

  Test t;
  int(Test::* mem_ptr)() = &Test::mem_fun; // get member function address
  void* mem_fun_ptr = (void*&)mem_ptr;    // convert it to common address
  int (*mem_st)(Test*) = reinterpret_cast<int(*)(Test*)>(mem_fun_ptr);  // convert it to required address type
  mocks.OnCallFuncOverload(mem_st).Return(111);

  std::cout << "Test::mem_fun() " << t.mem_fun() << std::endl;
  return 0;
}
aswna commented 8 years ago

+1 Would be very useful!

dascandy commented 8 years ago

It's been done once before and I think I can dig up the changelist but it has very big problems and implications, which is why it's not there now:

In short, I see that it could be useful, it would be dangerous to use even on unoptimized debug builds as the compiler may have inlined or omitted something, it would cause major performance issues if you actually were to use it and its interface is confusing. So as far as I'm concerned it's a no.

aswna commented 8 years ago

Thanks for the detailed explanation.

Europar commented 8 years ago

I'm starting to use HippoMocks to help unit testing our product. It would be great if we can have this feature optional. I totally understand the few problems you listed out, however I find our unit test environment has none or less of the problems:

dascandy commented 7 years ago

Will update documentation with rationale why this is not going to be added. It's an often requested feature but it's too likely to cause trouble for most users.

If you disagree, feel free to fork the project though.