Mir

Merge lp://qastaging/~alan-griffiths/mir/some-acceptance-tests-use-mir-Server-API into lp://qastaging/mir

some-acceptance-tests-use-mir-Server-API
Merge into development-branch

Proposed by Alan Griffiths on 2014-10-23

Status:	Merged
Approved by:	Alan Griffiths on 2014-10-28
Approved revision:	no longer in the source branch.
Merged at revision:	2009
Proposed branch:	lp://qastaging/~alan-griffiths/mir/some-acceptance-tests-use-mir-Server-API
Merge into:	lp://qastaging/mir
Prerequisite:	lp://qastaging/~alan-griffiths/mir/Migrate-ServerConfigurationWrapping-to-Server-API
Diff against target:	678 lines (+387/-84) 12 files modified include/server/mir/server.h (+21/-1) server-ABI-sha1sums (+1/-1) src/server/server.cpp (+63/-0) src/server/symbols.map (+1/-0) tests/acceptance-tests/server_configuration_wrapping.cpp (+3/-41) tests/acceptance-tests/test_client_library.cpp (+21/-6) tests/include/mir_test_framework/headless_test.h (+71/-0) tests/include/mir_test_framework/temporary_environment_value.h (+41/-0) tests/mir_test_framework/CMakeLists.txt (+2/-0) tests/mir_test_framework/headless_test.cpp (+113/-0) tests/mir_test_framework/temporary_environment_value.cpp (+42/-0) tests/unit-tests/graphics/mesa/test_anonymous_shm_file.cpp (+8/-35)
To merge this branch:	bzr merge lp://qastaging/~alan-griffiths/mir/some-acceptance-tests-use-mir-Server-API
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration		Approve on 2014-10-28
Kevin DuBois (community)			Approve on 2014-10-28
Alberto Aguirre (community)			Approve on 2014-10-28
Andreas Pokorny (community)			Approve on 2014-10-28
Alexandros Frantzis (community)		2014-10-23	Needs Fixing on 2014-10-24
Review via email: mp+239406@code.qastaging.launchpad.net

Commit message

test: Move another of the acceptance test suites [ClientLibrary.*] to the mir::Server based API

Description of the change

test: Move another of the acceptance test suites [ClientLibrary.*] to the mir::Server based API

It cleans up the "AcceptanceTest" fixture from the prerequisite branch and introduces "HeadlessTest".

There's a similar "debt" with a new HeadlessInProcessServer fixtures and some ugly code in mir::Server that should evolve as the requirements of additional tests are addressed.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-23:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/1900/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-24:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/1908/rebuild

review: Approve (continuous-integration)

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-10-24:

> 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.

> 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. 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).

Needs fixing for the typedef.

review: Needs Fixing

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-10-24:

> > 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.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-24:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/1909/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-27:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/1917/rebuild

review: Approve (continuous-integration)

Revision history for this message

Kevin DuBois (kdub) wrote on 2014-10-27:

27 + auto open_client_socket() -> int;
Given the api, I'd assume that I own the returned fd after calling this function, but it would be a bit more clear if it returned a mir::Fd.

Otherwise, I'm persuaded that this can solve help solve the endpoint-on-filesystem and think its useful.

review: Needs Fixing

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-10-28:

+1 for mir::Fd

looks good otherwise

Revision history for this message

Andreas Pokorny (andreas-pokorny) on 2014-10-28:

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-28:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/1926/rebuild

review: Approve (continuous-integration)

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-10-28:

LGTM

review: Approve

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-10-28:

The "needs fixing" items from the other 2 reviewers have been addressed so TA'ing.

Revision history for this message

Kevin DuBois (kdub) wrote on 2014-10-28:

looks good to me

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-28:

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/795/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/2281
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/2288
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/2216
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-autolanding/65/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1153
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1153/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3192
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15146

review: Needs Fixing (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-10-28:

[ FAILED ] 17 tests, listed below:
[ FAILED ] AvailableSurfaceFormatsTest.surface_pixel_formats_reach_client
[ FAILED ] SurfaceFirstFrameSync.surface_not_rendered_until_buffer_is_pushed
[ FAILED ] SwapIntervalSignalTest.swapinterval_test
[ FAILED ] ServerShutdown.server_can_shut_down_when_clients_are_blocked
[ FAILED ] SessionMediatorReport.session_connect_called
[ FAILED ] SessionMediatorReport.session_create_surface_called
[ FAILED ] SessionMediatorReport.session_next_buffer_called
[ FAILED ] SessionMediatorReport.session_exchange_buffer_called
[ FAILED ] SessionMediatorReport.session_release_surface_called
[ FAILED ] SessionMediatorReport.session_disconnect_called
[ FAILED ] SessionMediatorReport.prompt_session_start_called
[ FAILED ] SessionMediatorReport.prompt_session_stop_called
[ FAILED ] Process.a_main_fn_is_executed
[ FAILED ] Process.a_successful_exit_function_succeeds
[ FAILED ] BespokeDisplayServerTestFixture.starting_display_server_starts_input_manager
[ FAILED ] BespokeDisplayServerTestFixture.client_drm_auth_magic_calls_platform
[ FAILED ] BespokeDisplayServerTestFixture.drm_auth_magic_platform_error_reaches_client