[jira] [Commented] (MESOS-8991) `ObjectApprover::approved()` signature prevents it from being mockable

["Benjamin Bannier (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/MESOS-8991?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16512313#comment-16512313 ] 

Benjamin Bannier commented on MESOS-8991:
-----------------------------------------

Do we really want to break this interface (with {{noexcept}} being there for a reason), just because currently googlemock has issues with C++11 code? Upstream is working on moving googletest (which has absorbed googlemock) to C++11, see e.g., [https://github.com/google/googletest/issues/1366] or their mailing lists. I am not sure this will even be an issue in a couple of months.

In the meantime we could consider adding an intermediate adaptor class hiding the {{noexcept}} for googlemock, e.g.,
{code:java}
struct ObjectApprover {
  virtual ~ObjectApprover() = default;
  virtual void approved() const noexcept = 0;
};

// Transient class until googlemock supports multiple function
// qualifiers. Once upstream supports {{const noexcept}} functions we
// will remove this class and point any `MockApprover` to `ObjectApprover`
// instead of `AdaptApprover`.
struct AdaptApprover : ObjectApprover {
  void mock_approved() const {} // MOCK THIS FUNCTION.
  
  // The methods implementing the approver interface just forward to the
  // interface we can mock with the current googlemock implementation.
  void approved() const noexcept override { mock_approved(); }
};
{code}

> `ObjectApprover::approved()` signature prevents it from being mockable
> ----------------------------------------------------------------------
>
>                 Key: MESOS-8991
>                 URL: https://issues.apache.org/jira/browse/MESOS-8991
>             Project: Mesos
>          Issue Type: Task
>          Components: modules
>    Affects Versions: 1.6.0
>            Reporter: Alexander Rojas
>            Priority: Major
>              Labels: authorization, mesosphere, modularization
>
> The [{{ObjectApprover::approved()}}|https://github.com/apache/mesos/blob/8b93fa3/include/mesos/authorizer/authorizer.hpp#L221] method has the following signature:
> {code}
> virtual Try<bool> approved(
>       const Option<Object>& object) const noexcept = 0;
> {code}
> This is unfortunate since it is impossible to mock a function in google mock with [two qualifiers|https://groups.google.com/forum/#!topic/googlemock/LsbubY26qx4], which reduces the amount of tests we can perform.
> Moreover, the {{noexcept}} is not even needed in Mesos, since it does not use exception and it is compiled without exception support by default.
> The tricky situation here is that this is a public API so it would be tricky to replace since it will break backwards compatibility.
>  
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)