Mir

Code review comment for lp:~alan-griffiths/mir/some-acceptance-tests-use-mir-Server-API

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> > typedef std::function<void(std::shared_ptr<frontend::Session> const&
> session)> ConnectHandler;
>
> using ConnectHandler = ... , it's more consistent with other type definitions
> in the class.

Fixed

> > auto open_client_socket() -> int;
>
> Although testing is a valid driver of the API, I am a bit skeptical of
> exposing functionality for tests only, since doing so may occasionally
> compromise the focus of the API, expose internals etc.

I totally agree. Although in the case of acceptance tests these should ideally be written in terms of the API.

Where acceptance tests need something other than the API exposes there should a clear distinction in the way such features are invoked.

> This particular change
> seems OK, though, and if we want to provide such functionality we should also
> provide the same stability guarantees as we do for the "normal" public API
> (and stability should be an important factor in deciding whether we want a
> test-only method appear in the public API).

I gave some thought to this and concluded supporting the test this way gives a potentially useful API: we don't always want to expose an endpoint on the filesystem for clients to use (doing so is a long standing issue with unity-system-compositor). It also allows a server to start in process "clients" using the normal client API.

« Back to merge proposal