Using gmock Invoke with move-only types.

[ami...@chromium.org]

Hi all,

I'm trying to put together a CL where I change a const-ref Callback to a move-only OnceCallback. There's an existing mock which uses gmock and ON_CALL().WillByDefault(Invoke(...)) to provide a default action. However, gmock doesn't like the move-only OnceCallback:

../../third_party/googletest/src/googlemock/include/gmock/gmock-generated-actions.h:113:42: error: call to deleted constructor of 'base::OnceCallback<void (bool)>'

           return (obj_ptr->*method_ptr)(get<0>(args), get<1>(args));

                                         ^~~~~~~~~~~~

../../third_party/googletest/src/googlemock/include/gmock/gmock-more-actions.h:81:49: note: in instantiation of function template specialization 'testing::internal::InvokeHelper<void, std::__1::tuple<base::OnceCallback<void (bool)>, bool> >::InvokeMethod<chromeos::disks::MockDiskMountManager, void (chromeos::disks::MockDiskMountManager::*)(base::OnceCallback<void (bool)>, bool)>' requested here

    return InvokeHelper<Result, ArgumentTuple>::InvokeMethod(

                                                ^

../../third_party/googletest/src/googlemock/include/gmock/gmock-actions.h:474:29: note: in instantiation of function template specialization 'testing::internal::InvokeMethodAction<chromeos::disks::MockDiskMountManager, void (chromeos::disks::MockDiskMountManager::*)(base::OnceCallback<void (bool)>, bool)>::Perform<void, std::__1::tuple<base::OnceCallback<void (bool)>, bool> >' requested here

      return impl_.template Perform<Result>(args);

                            ^

../../third_party/googletest/src/googlemock/include/gmock/gmock-actions.h:471:14: note: in instantiation of member function 'testing::PolymorphicAction<testing::internal::InvokeMethodAction<chromeos::disks::MockDiskMountManager, void (chromeos::disks::MockDiskMountManager::*)(base::OnceCallback<void (bool)>, bool)> >::MonomorphicImpl<void (base::OnceCallback<void (bool)>, bool)>::Perform' requested here

    explicit MonomorphicImpl(const Impl& impl) : impl_(impl) {}

             ^

../../third_party/googletest/src/googlemock/include/gmock/gmock-actions.h:461:26: note: in instantiation of member function 'testing::PolymorphicAction<testing::internal::InvokeMethodAction<chromeos::disks::MockDiskMountManager, void (chromeos::disks::MockDiskMountManager::*)(base::OnceCallback<void (bool)>, bool)> >::MonomorphicImpl<void (base::OnceCallback<void (bool)>, bool)>::MonomorphicImpl' requested here

    return Action<F>(new MonomorphicImpl<F>(impl_));

                         ^

../../chromeos/disks/mock_disk_mount_manager.cc:83:22: note: in instantiation of function template specialization 'testing::PolymorphicAction<testing::internal::InvokeMethodAction<chromeos::disks::MockDiskMountManager, void (chromeos::disks::MockDiskMountManager::*)(base::OnceCallback<void (bool)>, bool)> >::operator Action<void (base::OnceCallback<void (bool)>, bool)>' requested here

      .WillByDefault(Invoke(

                     ^

../../base/callback.h:68:3: note: 'OnceCallback' has been explicitly marked deleted here

  OnceCallback(const OnceCallback&) = delete;

I'm certain someone has run into this problem before. Is there a "standard" or pre-existing solution to this before I go off and invent my own?

[ami...@chromium.org]

So, the gmock cookbock has a workaround, which is to use a lambda instead of Invoke(). I only have two call sites to update, so this works fine for me. But I'm still wondering what other people do.

[Sylvain Defresne]

As far as I know, gmock does not support move only types, this include OnceCallback<...>.

A work-around that I've seen multiple time in code review is to add an overloaded method that takes the callback by pointer, mark that method as mockable, and write a manual override of the base method passing a pointer to the callback.

So you would have the following:

  class SomeClass {

   public:

    virtual void DoSomething(base::OnceClosure callback);

  };

  class MockSomeClass : public SomeClass {

   public:

    void  DoSomething  (base::OnceClosure  callback  ) override {

       DoSomething  (&completion);

    }

    MOCK_METHOD1(DoSomething, void(base::OnceClosure*));

  };

For example, this is used in mock_favicon_service.h.

-- Sylvain

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/282b77de-a333-453c-aa30-5a707838ddfb%40chromium.org.

[Lucas Gadani]

While the newest gmock supports this, the version of gmock that we have in Chromium doesn't support move types. You can try rolling the new version (might be a significant amount of breakage), or just use https://github.com/google/googletest/blob/master/googlemock/docs/CookBook.md#legacy-workarounds-for-move-only-types-legacymoveonly for a workaround.

--

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/621e309e-a6a1-4454-b9fe-62af10ff77a7%40chromium.org.

[Peter Kasting]

On Mon, Aug 27, 2018 at 10:47 AM Lucas Gadani <l...@chromium.org> wrote:

While the newest gmock supports this, the version of gmock that we have in Chromium doesn't support move types. You can try rolling the new version (might be a significant amount of breakage), or just use https://github.com/google/googletest/blob/master/googlemock/docs/CookBook.md#legacy-workarounds-for-move-only-types-legacymoveonly for a workaround.

+1 to at least trying to update gmock and seeing how broken it is.

PK 

[Anand Mistry]

A year and a little bit later, I have my answer. Well, at least for callbacks. We have base::test::RunCallback which I just fixed for move-only OnceCallbacks. No need to use a workaround for running a Callback.