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

Add MockFunction support for std::function #2277

Closed
adambadura opened this issue Jun 9, 2019 · 6 comments
Closed

Add MockFunction support for std::function #2277

adambadura opened this issue Jun 9, 2019 · 6 comments

Comments

@adambadura
Copy link
Contributor

adambadura commented Jun 9, 2019

MockFunction requires function signature R(Args...) as its template argument. This makes it difficult to use it with std::function hidden under some type name. std::function doesn't provide any member type to allow to (safely and easily) reflect its own arguments (the signature).

The problem shows when we want to refer to std::function through its type name like:

using my_callback = std::function<bool(int)>;

Now, we cannot do ::testing::MockFunction<my_callback>. We have to do ::testing::MockFunction<bool(int)>, which is a maintenance issue requiring us to repeat the type definition.

Below is my proposal to add the support:

diff --git a/googlemock/include/gmock/gmock-spec-builders.h b/googlemock/include/gmock/gmock-spec-builders.h
index 1f261bd2..47c34980 100644
--- a/googlemock/include/gmock/gmock-spec-builders.h
+++ b/googlemock/include/gmock/gmock-spec-builders.h
@@ -1836,6 +1836,24 @@ void ReportUninterestingCall(CallReaction reaction, const std::string& msg);
 //   EXPECT_CALL(callback, Call("bar")).WillOnce(Return(1));
 //   Foo(callback.AsStdFunction());
 // }
+template <typename F>
+struct SignatureOf;
+
+template <typename R, typename... Args>
+struct SignatureOf<R(Args...)> {
+ using type = R(Args...);
+};
+
+template <typename R, typename... Args>
+struct SignatureOf<std::function<R(Args...)>> {
+ using type = R(Args...);
+};
+
+template <typename F>
+using SignatureOfT = typename SignatureOf<F>::type;
+
+namespace internal {
+
 template <typename F>
 class MockFunction;
 
@@ -1858,20 +1876,25 @@ class MockFunction<R(Args...)> {
     return mock_.Invoke(std::forward<Args>(args)...);
   }
 
-  internal::MockSpec<R(Args...)> gmock_Call(Matcher<Args>... m) {
+  MockSpec<R(Args...)> gmock_Call(Matcher<Args>... m) {
     mock_.RegisterOwner(this);
     return mock_.With(std::move(m)...);
   }
 
-  internal::MockSpec<R(Args...)> gmock_Call(const internal::WithoutMatchers&,
-                                            R (*)(Args...)) {
+  MockSpec<R(Args...)> gmock_Call(const WithoutMatchers&,
+                                  R (*)(Args...)) {
     return this->gmock_Call(::testing::A<Args>()...);
   }
 
  private:
-  mutable internal::FunctionMocker<R(Args...)> mock_;
+  mutable FunctionMocker<R(Args...)> mock_;
 };
 
+}  // namespace internal
+
+template <typename F>
+using MockFunction = internal::MockFunction<SignatureOfT<F>>;
+
 // The style guide prohibits "using" statements in a namespace scope
 // inside a header file.  However, the MockSpec class template is
 // meant to be defined in the ::testing namespace.  The following line

The solution is based on the introduction of SignatureOf meta-function which translates types provided to MetaFunction into function signature. This allows to support std::function and could be later easily extended further, including user extending it with user-types. (boost::function and folly::Function being good examples of extension candidates.)

The meta-function based solution as compared to a more classic approach of specialization also avoids the choice between duplicating code or using inheritance, which then creates an issue of virtual destructor.

@adambadura adambadura changed the title Add MockFunction specialization for std::function Add MockFunction support for std::function Jun 10, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Jul 11, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Jul 11, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Jul 27, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Aug 3, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Aug 8, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Aug 16, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Aug 19, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Aug 21, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Aug 24, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Aug 26, 2019
@akonradi
Copy link
Contributor

I was just thinking today that it would be nice to have support for instantiating something like MockFunction<MyCallbackType> where MyCallbackType = std::function<void(Arg)>. Is that still on the horizon?

@adambadura
Copy link
Contributor Author

adambadura commented Aug 27, 2019

@akonradi, I'm ready any time! The pull request (#2350) is rebased over the head, tests pass. I cannot do anything more than wait. :)

@sbenzaquen
Copy link
Collaborator

You can use the SignatureOfT metafunction on your side.
We have many different templates that take function types. Changing just this one to also accept some non-function type would lead to inconsistency and confused users.

@adambadura
Copy link
Contributor Author

@sbenzaquen, I hoped this will be the first to change to make user-extensions easier! :)

While consistency is important I think that adapting all cases at once is not practical and would be difficult to review. None the less - if that would be yes-or-no then I will try to adapt other cases as well.

We can also skip documenting the SignatureOfT and just mention that std::function can be used directly. Then we will document it only once all cases are ready. So, users will not be confused.

Finally, I can adapt the change to cover std::function only without the abstraction layer of SignaturOfT. This would still be better than the current state.

While you are right that the SignatreOfT can be done by the user, it is an extra code that you have to know you can write and how to do it. And then also somehow keep it available everywhere you need.

Why not make it easier for the user? After all the MockFunction itself could be written by the user (in fact a few years ago I kept reimplementing it in each subsequent project until finally, some reviewer told me this is already in the library) and yet we do have it! :)

adambadura added a commit to adambadura/googletest that referenced this issue Aug 28, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Aug 29, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Sep 16, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Sep 30, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Oct 8, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Oct 29, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Oct 29, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Oct 30, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Nov 1, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Nov 2, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Nov 6, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Nov 10, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Nov 18, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Nov 28, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Dec 9, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Dec 16, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Dec 26, 2019
Add tests checking that ::testing::MockFunction template argument can
be deduced in a function call context. This is a property raised in the
review, however, not checked before by any tests.
adambadura added a commit to adambadura/googletest that referenced this issue Dec 26, 2019
adambadura added a commit to adambadura/googletest that referenced this issue Jan 4, 2020
Add tests checking that ::testing::MockFunction template argument can
be deduced in a function call context. This is a property raised in the
review, however, not checked before by any tests.
adambadura added a commit to adambadura/googletest that referenced this issue Jan 4, 2020
adambadura added a commit to adambadura/googletest that referenced this issue Jan 25, 2020
Add tests checking that ::testing::MockFunction template argument can
be deduced in a function call context. This is a property raised in the
review, however, not checked before by any tests.
adambadura added a commit to adambadura/googletest that referenced this issue Jan 25, 2020
adambadura added a commit to adambadura/googletest that referenced this issue Jan 28, 2020
Add tests checking that ::testing::MockFunction template argument can
be deduced in a function call context. This is a property raised in the
review, however, not checked before by any tests.
adambadura added a commit to adambadura/googletest that referenced this issue Jan 28, 2020
adambadura added a commit to adambadura/googletest that referenced this issue Feb 10, 2020
Add tests checking that ::testing::MockFunction template argument can
be deduced in a function call context. This is a property raised in the
review, however, not checked before by any tests.
adambadura added a commit to adambadura/googletest that referenced this issue Feb 10, 2020
adambadura added a commit to adambadura/googletest that referenced this issue Mar 1, 2020
Add tests checking that ::testing::MockFunction template argument can
be deduced in a function call context. This is a property raised in the
review, however, not checked before by any tests.
adambadura added a commit to adambadura/googletest that referenced this issue Mar 1, 2020
adambadura added a commit to adambadura/googletest that referenced this issue Mar 18, 2020
Add tests checking that ::testing::MockFunction template argument can
be deduced in a function call context. This is a property raised in the
review, however, not checked before by any tests.
adambadura added a commit to adambadura/googletest that referenced this issue Mar 18, 2020
@akonradi
Copy link
Contributor

Looks like this is fixed by #2350.

@adambadura
Copy link
Contributor Author

@akonradi, yes, it is. Thanks for the reminder, I'm closing this.

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

No branches or pull requests

3 participants