tfhq / googletest

Automatically exported from code.google.com/p/googletest
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Add support for move only parameters in mocked methods #395

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
The new C++11 standard allows for rvalue references in method parameters. Using 
those for move semantics is very useful. We need to suppor them, e.g.:

MOCK_METHOD1(mock, foo(int&& i));

Original issue reported on code.google.com by vladlosev on 19 Dec 2011 at 12:45

GoogleCodeExporter commented 9 years ago
There are also techniques for doing this in C++98/03 *without* using rvalue 
references.

In particular, look at Howard Hinnant's unique_ptr<> emulation: 
http://home.roadrunner.com/~hinnant/unique_ptr03.html

What this means it that people can write APIs in C++98 of this style:

// This is pass-by-value not rvalue-reference.
void TakesOwnership(unique_ptr<Foo> f);

You can only call this with an rvalue argument since unique_ptr is move-only.  
With gmock, this presents a problem that is separate from supporting APIs that 
take rvalue-references.

Original comment by ajw...@google.com on 19 Dec 2011 at 7:44

GoogleCodeExporter commented 9 years ago
Is there a status on this? Google mocks seems to be the most promising C++ 
mocking framework, and it's suggested by my company. The first time I tried 
using gmock, though, my interface uses std::unique_ptr and my gmock'ed class is 
not compiling correctly.

Original comment by bku...@gmail.com on 1 Mar 2013 at 2:31

GoogleCodeExporter commented 9 years ago
I have similar problems. I would dearly love to use gmock but being unable to 
return move-only types is a non-starter in today's C++.

Original comment by rdil...@gmail.com on 16 Aug 2013 at 2:29

GoogleCodeExporter commented 9 years ago
It's reasonably straightforward to add a wrapping function whenever you require 
a move-supporting mock:

struct mock {
   void insert( unique_ptr<int> p ) {
      insert_(p);
   }

   MOCK_METHOD1( insert_, void(unique_ptr<int>&) );
};

When the unit under test calls e.g. insert( std::move( some_ptr )) the wrapper 
accepts the move-only parameter, then passes it by reference to the mock itself.
If your mock needs to move the value further then it can, as you can 
std::move() from a non-const lvalue reference type.

It may be worth questioning whether enforcing the move-only interface on your 
mock is beneficial. If instead of adding a move-enabled wrapper around the mock 
method you were to just make your mock method accept lvalue references, you can 
still verify the value of the parameter, move it, or whatever, without loss of 
test completeness.

The only downside I see is that by dropping the requirement that the parameter 
be moved in, your unit under test might forget to do that in some place. Since 
this would generate a compilation error when you build it with the real 
instance of whatever you are mocking, this doesn't really lose you anything 
though. Hope that makes sense...

@vladlosev - this is the googletest project, but you're requesting a change to 
googlemock. It may be better to post your request to the googlemock project 
page...

Original comment by rob.desb...@gmail.com on 21 Aug 2013 at 10:29

GoogleCodeExporter commented 9 years ago
What about returning types that dont have copy constructor/operator but have 
move constructor/operator? E.g.: std::unique_ptr.

Original comment by dariusz....@gmail.com on 8 Jul 2014 at 9:01

GoogleCodeExporter commented 9 years ago
Returning move-only types in mock methods is supported.
The 'Return()' action does not have the support yet, but it can be done with 
'Invoke()' or 'DefaultValue<T>::SetFactory()'

Original comment by sbe...@google.com on 8 Jul 2014 at 7:56

GoogleCodeExporter commented 9 years ago
Here is a patch we use.
It supports move-only parameters except in DoAll(), but that doesn't even make 
sense with non-copyable objects.
It contains a few other fixes as well.

It works if applied to r485 version, but all pumped sources must be regenerated 
after applying the patch.

I'm not sure if it works with non-C++11 compilers.
I tested it with Visual Studio 2013, gcc 4.8, 4.9, clang 3.4, 3.5

Original comment by n...@tresorit.com on 20 Nov 2014 at 4:13

Attachments:

GoogleCodeExporter commented 9 years ago
In the previous comment the r485 link incorrectly points to the GoogleTest 
revision, I meant the GoogleMock revision.

Original comment by n...@tresorit.com on 20 Nov 2014 at 4:14

GoogleCodeExporter commented 9 years ago
Is the patch in #7 a candidate for upstreaming?

Original comment by mfo...@google.com on 7 Aug 2015 at 10:47

GoogleCodeExporter commented 9 years ago
Here is my previous patch (in #7) rebased to current googlemock r566. It now 
contains pumped sources too, and it still contains some unrelated changes as 
well.

Original comment by n...@tresorit.com on 21 Aug 2015 at 8:59

Attachments: