Merge lp:~raof/mir/a-little-more-error-checkin into lp:mir

[Chris Halse Rogers]

Improve error checking and reporting for the client library.

Some more miscellaneous error checking niceties that would have been great to have while developing privatise-all-the-things.

This starts fleshing out the error handling policy I think we settled on in London - *create calls always return a non-null object, but passing error objects to any method other than *_is_valid or *_release is fatal.

Oh, and it also fixes UsingStubClientPlatform so that it does something :).

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1875
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2529/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1494
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1500
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1477/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1051/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/414
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/414/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2559/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12257

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2529/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1876
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2533/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1499
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1505
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1482/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1055/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/419
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/419/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2563/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12270

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2533/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1878
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2552/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1525
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1531
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1508/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1074/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/445
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/445/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2586/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12340

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2552/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1880
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2564/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1547/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1553/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1529/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1086/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/466/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2564/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1881
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2567/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1550
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1556/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1532
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1089/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/469
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/469/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2608
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12454

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2567/rebuild

review: Needs Fixing (continuous-integration)

[Robert Carr (robertcarr)]

661, 672. Maybe use the new validity matchers here?

review: Needs Information

[Chris Halse Rogers (raof)]

Move to WIP - I can do this better with a real stub platform.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1883
http://jenkins.qa.ubuntu.com/job/mir-ci/1984/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/2350
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/2360/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/2285/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-ci/330/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1222
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1222/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3257/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15395

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

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

538 +TEST_F(ClientLibraryErrors, CreateSurfaceReturnsErrorObjectOnFailureInReplyProcessing)

this_is_how_we_name_test_cases_around_here

review: Needs Fixing

[Chris Halse Rogers (raof)]

Man, I hope gtest never does anything would break test_names_with_underscores_in_them, which the documentation explicitly tells you not to use.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1884
http://jenkins.qa.ubuntu.com/job/mir-ci/1991/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/2362
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/2372
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/2297
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-ci/337/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1234
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1234/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3269
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15435

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1884
http://jenkins.qa.ubuntu.com/job/mir-ci/1996/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/2367
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/2377/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/2302
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-ci/342/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1239
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1239/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3274
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15451

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1885
http://jenkins.qa.ubuntu.com/job/mir-ci/2006/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/2377
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/2387
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/2312
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-ci/352/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1249
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1249/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3282
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15474

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

review: Needs Fixing (continuous-integration)

[Chris Halse Rogers (raof)]

I am confused as to why “make test” doesn't catch these failures locally. Anyway, let's try this again.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1886
http://jenkins.qa.ubuntu.com/job/mir-ci/2013/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/2
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/2
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/2/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

67 + return mir_connection_api_impl->connect(mir_connection_api_impl->configuration_factory(),

Looks horrible. But I don't have a better suggestion in mind.

review: Approve

[Alberto Aguirre (albaguirre)]

LGTM

review: Approve

=== modified file 'src/client/mir_connection.cpp'
--- src/client/mir_connection.cpp	2014-11-03 19:06:48 +0000
+++ src/client/mir_connection.cpp	2014-11-05 22:26:46 +0000
@@ -34,6 +34,8 @@
 #include <unistd.h>
 #include <signal.h>
 
+#include <boost/exception/diagnostic_information.hpp>
+
 namespace mcl = mir::client;
 namespace mircv = mir::input::receiver;
 namespace gp = google::protobuf;
@@ -219,6 +221,7 @@
 void MirConnection::connected(mir_connected_callback callback, void * context)
 {
     bool safe_to_callback = true;
+    try
     {
         std::lock_guard<decltype(mutex)> lock(mutex);
 
@@ -262,6 +265,11 @@
         display_configuration->set_configuration(connect_result.display_configuration());
         lifecycle_control->set_lifecycle_event_handler(default_lifecycle_event_handler);
     }
+    catch (std::exception const& e)
+    {
+        connect_result.set_error(std::string{"Failed to process connect response: "} +
+                                 boost::diagnostic_information(e));
+    }
 
     if (safe_to_callback) callback(this, context);
     connect_wait_handle.result_received();

=== modified file 'src/client/mir_connection_api.cpp'
--- src/client/mir_connection_api.cpp	2014-10-01 06:25:56 +0000
+++ src/client/mir_connection_api.cpp	2014-11-05 22:26:46 +0000
@@ -53,6 +53,7 @@
 {
 public:
     MirWaitHandle* connect(
+        mcl::ConfigurationFactory configuration,
         char const* socket_file,
         char const* name,
         mir_connected_callback callback,
@@ -112,10 +113,13 @@
         delete connection;
     }
 
-    std::unique_ptr<mcl::ConnectionConfiguration> configuration(std::string const& socket) override
+    mcl::ConfigurationFactory configuration_factory() override
     {
-        return std::unique_ptr<mcl::ConnectionConfiguration>{
-            new mcl::DefaultConnectionConfiguration{socket}};
+        return [](std::string const& socket) {
+            return std::unique_ptr<mcl::ConnectionConfiguration>{
+                new mcl::DefaultConnectionConfiguration{socket}
+            };
+        };
     }
 };
 
@@ -132,7 +136,11 @@
 {
     try
     {
-        return mir_connection_api_impl->connect(socket_file, name, callback, context);
+        return mir_connection_api_impl->connect(mir_connection_api_impl->configuration_factory(),
+                                                socket_file,
+                                                name,
+                                                callback,
+                                                context);
     }
     catch (std::exception const&)
     {

=== modified file 'src/client/mir_connection_api.h'
--- src/client/mir_connection_api.h	2014-10-01 06:25:56 +0000
+++ src/client/mir_connection_api.h	2014-11-05 22:26:46 +0000
@@ -23,6 +23,7 @@
 
 #include <string>
 #include <memory>
+#include <functional>
 
 namespace mir
 {
@@ -30,12 +31,15 @@
 {
 class ConnectionConfiguration;
 
+using ConfigurationFactory = std::function<std::unique_ptr<ConnectionConfiguration>(std::string const&)>;
+
 class MirConnectionAPI
 {
 public:
     virtual ~MirConnectionAPI() = default;
 
     virtual MirWaitHandle* connect(
+        ConfigurationFactory configuration,
         char const* socket_file,
         char const* name,
         mir_connected_callback callback,
@@ -43,7 +47,7 @@
 
     virtual void release(MirConnection* connection) = 0;
 
-    virtual std::unique_ptr<ConnectionConfiguration> configuration(std::string const& socket) = 0;
+    virtual ConfigurationFactory configuration_factory() = 0;
 
 protected:
     MirConnectionAPI() = default;

=== modified file 'src/client/mir_surface.cpp'
--- src/client/mir_surface.cpp	2014-10-28 22:48:39 +0000
+++ src/client/mir_surface.cpp	2014-11-05 22:26:46 +0000
@@ -30,6 +30,8 @@
 #include <cassert>
 #include <unistd.h>
 
+#include <boost/exception/diagnostic_information.hpp>
+
 namespace geom = mir::geometry;
 namespace mcl = mir::client;
 namespace mircv = mir::input::receiver;
@@ -39,11 +41,22 @@
 namespace
 {
 void null_callback(MirSurface*, void*) {}
+mp::DisplayServer::Stub null_server{nullptr};
 
 std::mutex handle_mutex;
 std::unordered_set<MirSurface*> valid_surfaces;
 }
 
+MirSurface::MirSurface(std::string const& error)
+    : server{null_server},
+      connection{nullptr}
+{
+    surface.set_error(error);
+
+    std::lock_guard<decltype(handle_mutex)> lock(handle_mutex);
+    valid_surfaces.insert(this);
+}
+
 MirSurface::MirSurface(
     MirConnection *allocating_connection,
     mp::DisplayServer::Stub & server,
@@ -258,20 +271,29 @@
 {
     auto platform = connection->get_client_platform();
 
+    try
     {
-        std::lock_guard<decltype(mutex)> lock(mutex);
-
-        process_incoming_buffer();
-        accelerated_window = platform->create_egl_native_window(this);
-        
-        for(int i = 0; i < surface.attributes_size(); i++)
         {
-            auto const& attrib = surface.attributes(i);
-            attrib_cache[attrib.attrib()] = attrib.ivalue();
+            std::lock_guard<decltype(mutex)> lock(mutex);
+
+            process_incoming_buffer();
+            accelerated_window = platform->create_egl_native_window(this);
+
+            for(int i = 0; i < surface.attributes_size(); i++)
+            {
+                auto const& attrib = surface.attributes(i);
+                attrib_cache[attrib.attrib()] = attrib.ivalue();
+            }
         }
-    }
-
-    connection->on_surface_created(id(), this);
+
+        connection->on_surface_created(id(), this);
+    }
+    catch (std::exception const& error)
+    {
+        surface.set_error(std::string{"Error processing Surface creating response:"} +
+                          boost::diagnostic_information(error));
+    }
+
     callback(this, context);
     create_wait_handle.result_received();
 }
@@ -296,7 +318,18 @@
         valid_surfaces.erase(this);
     }
 
-    return connection->release_surface(this, callback, context);
+    MirWaitHandle* wait_handle{nullptr};
+    if (connection)
+    {
+        wait_handle = connection->release_surface(this, callback, context);
+    }
+    else
+    {
+        callback(this, context);
+        delete this;
+    }
+
+    return wait_handle;
 }
 
 MirNativeBuffer* MirSurface::get_current_buffer_package()

=== modified file 'src/client/mir_surface.h'
--- src/client/mir_surface.h	2014-10-27 22:31:16 +0000
+++ src/client/mir_surface.h	2014-11-05 22:26:46 +0000
@@ -59,6 +59,8 @@
     MirSurface(MirSurface const &) = delete;
     MirSurface& operator=(MirSurface const &) = delete;
 
+    MirSurface(std::string const& error);
+
     MirSurface(
         MirConnection *allocating_connection,
         mir::protobuf::DisplayServer::Stub & server,

=== modified file 'src/client/mir_surface_api.cpp'
--- src/client/mir_surface_api.cpp	2014-10-27 22:31:16 +0000
+++ src/client/mir_surface_api.cpp	2014-11-05 22:26:46 +0000
@@ -23,6 +23,8 @@
 #include "mir_surface.h"
 #include "error_connections.h"
 
+#include <boost/exception/diagnostic_information.hpp>
+
 namespace mcl = mir::client;
 
 namespace
@@ -43,15 +45,17 @@
     mir_surface_callback callback,
     void* context)
 {
-    if (mcl::ErrorConnections::instance().contains(connection)) return 0;
+    if (!mir_connection_is_valid(connection)) abort();
 
     try
     {
         return connection->create_surface(*params, callback, context);
     }
-    catch (std::exception const&)
+    catch (std::exception const& error)
     {
-        // TODO callback with an error surface
+        auto error_surf = new MirSurface{std::string{"Failed to create surface: "} +
+                                         boost::diagnostic_information(error)};
+        (*callback)(error_surf, context);
         return nullptr;
     }
 }

=== modified file 'tests/acceptance-tests/CMakeLists.txt'
--- tests/acceptance-tests/CMakeLists.txt	2014-11-03 19:06:48 +0000
+++ tests/acceptance-tests/CMakeLists.txt	2014-11-05 22:26:46 +0000
@@ -17,6 +17,7 @@
   server_signal_handling.cpp
   clients.cpp
   test_client_library.cpp
+  test_client_library_errors.cpp
   test_client_library_old.cpp
   test_client_surface_events.cpp
   test_client_surface_swap_buffers.cpp

=== modified file 'tests/acceptance-tests/test_client_library.cpp'
--- tests/acceptance-tests/test_client_library.cpp	2014-10-29 10:11:50 +0000
+++ tests/acceptance-tests/test_client_library.cpp	2014-11-05 22:26:46 +0000
@@ -53,8 +53,6 @@
 {
 struct ClientLibrary : mtf::HeadlessInProcessServer
 {
-    mtf::UsingStubClientPlatform using_stub_client_platform;
-
     std::set<MirSurface*> surfaces;
     MirConnection* connection = nullptr;
     MirSurface* surface  = nullptr;
@@ -597,42 +595,6 @@
     mir_connection_release(connection);
 }
 
-TEST_F(ClientLibrary, connect_errors_handled)
-{
-    mir_wait_for(mir_connect("garbage", __PRETTY_FUNCTION__, connection_callback, this));
-    ASSERT_THAT(connection, NotNull());
-
-    char const* error = mir_connection_get_error_message(connection);
-
-    if (std::strcmp("connect: No such file or directory", error) &&
-        std::strcmp("Can't find MIR server", error) &&
-        !std::strstr(error, "Failed to connect to server socket"))
-    {
-        FAIL() << error;
-    }
-}
-
-TEST_F(ClientLibrary, connect_errors_dont_blow_up)
-{
-    mir_wait_for(mir_connect("garbage", __PRETTY_FUNCTION__, connection_callback, this));
-
-    MirSurfaceParameters const request_params =
-    {
-        __PRETTY_FUNCTION__,
-        640, 480,
-        mir_pixel_format_abgr_8888,
-        mir_buffer_usage_hardware,
-        mir_display_output_id_invalid
-    };
-
-    mir_wait_for(mir_connection_create_surface(connection, &request_params, create_surface_callback, this));
-// TODO surface_create needs to fail safe too. After that is done we should add the following:
-// TODO    mir_wait_for(mir_surface_swap_buffers(surface, next_buffer_callback, this));
-// TODO    mir_wait_for(mir_surface_release( surface, release_surface_callback, this));
-
-    mir_connection_release(connection);
-}
-
 TEST_F(ClientLibrary, MultiSurfaceClientTracksBufferFdsCorrectly)
 {
     mir_wait_for(mir_connect(new_connection().c_str(), __PRETTY_FUNCTION__, connection_callback, this));

=== added file 'tests/acceptance-tests/test_client_library_errors.cpp'
--- tests/acceptance-tests/test_client_library_errors.cpp	1970-01-01 00:00:00 +0000
+++ tests/acceptance-tests/test_client_library_errors.cpp	2014-11-05 22:26:46 +0000
@@ -0,0 +1,338 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Christopher James Halse Rogers <[e-mail]>
+ */
+
+#include "mir_toolkit/mir_client_library.h"
+#include "mir_toolkit/debug/surface.h"
+
+#include "src/client/client_platform_factory.h"
+#include "src/client/client_platform.h"
+#include "src/client/client_buffer_factory.h"
+
+#include "mir_test/validity_matchers.h"
+
+#include "mir_test_framework/in_process_server.h"
+#include "mir_test_framework/stubbed_server_configuration.h"
+#include "mir_test_framework/using_stub_client_platform.h"
+#include "mir_test_framework/stub_client_connection_configuration.h"
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include <stdexcept>
+#include <boost/throw_exception.hpp>
+#include <cstring>
+
+namespace mcl = mir::client;
+namespace mtf = mir_test_framework;
+
+namespace
+{
+enum Method : uint64_t
+{
+    none = 0,
+    the_client_platform_factory = 1<<0,
+    create_client_platform      = 1<<1,
+    create_egl_native_window    = 1<<2,
+    create_buffer_factory       = 1<<3
+};
+
+std::string const exception_text{"Ducks!"};
+
+template<Method name, Method failure_set>
+bool should_fail()
+{
+    return (name & failure_set);
+}
+
+class StubClientBufferFactory : public mir::client::ClientBufferFactory
+{
+    std::shared_ptr<mir::client::ClientBuffer> create_buffer(const std::shared_ptr<MirBufferPackage>&,
+                                                             mir::geometry::Size,
+                                                             MirPixelFormat)
+    {
+        return std::shared_ptr<mir::client::ClientBuffer>{};
+    }
+};
+
+template<Method failure_set>
+class ConfigurableFailurePlatform : public mir::client::ClientPlatform
+{
+    std::shared_ptr<EGLNativeWindowType> create_egl_native_window(mir::client::ClientSurface *)
+    {
+        if (should_fail<Method::create_egl_native_window, failure_set>())
+        {
+            BOOST_THROW_EXCEPTION(std::runtime_error{exception_text});
+        }
+        return std::shared_ptr<EGLNativeWindowType>{};
+    }
+    MirPlatformType platform_type() const
+    {
+        BOOST_THROW_EXCEPTION(std::runtime_error{exception_text});
+        return MirPlatformType::mir_platform_type_gbm;
+    }
+    std::shared_ptr<mir::client::ClientBufferFactory> create_buffer_factory()
+    {
+        if (should_fail<Method::create_buffer_factory, failure_set>())
+        {
+            BOOST_THROW_EXCEPTION(std::runtime_error{exception_text});
+        }
+        return std::make_shared<StubClientBufferFactory>();
+    }
+    std::shared_ptr<EGLNativeDisplayType> create_egl_native_display()
+    {
+        return std::shared_ptr<EGLNativeDisplayType>{};
+    }
+    MirNativeBuffer *convert_native_buffer(mir::graphics::NativeBuffer*) const
+    {
+        BOOST_THROW_EXCEPTION(std::runtime_error{exception_text});
+        return nullptr;
+    }
+};
+
+template<Method failure_set>
+class ConfigurableFailureFactory: public mir::client::ClientPlatformFactory
+{
+    std::shared_ptr<mir::client::ClientPlatform>
+    create_client_platform(mir::client::ClientContext* /*context*/) override
+    {
+        if (should_fail<Method::create_client_platform, failure_set>())
+        {
+            BOOST_THROW_EXCEPTION(std::runtime_error{exception_text});
+        }
+        return std::make_shared<ConfigurableFailurePlatform<failure_set>>();
+    }
+};
+
+template<Method failure_set>
+class ConfigurableFailureConfiguration : public mtf::StubConnectionConfiguration
+{
+    using mtf::StubConnectionConfiguration::StubConnectionConfiguration;
+
+    std::shared_ptr<mir::client::ClientPlatformFactory> the_client_platform_factory() override
+    {
+        if (should_fail<Method::the_client_platform_factory, failure_set>())
+        {
+            BOOST_THROW_EXCEPTION(std::runtime_error{exception_text});
+        }
+        return std::make_shared<ConfigurableFailureFactory<failure_set>>();
+    }
+};
+
+class ClientLibraryErrors : public mtf::InProcessServer
+{
+private:
+    mtf::StubbedServerConfiguration config;
+
+    mir::DefaultServerConfiguration &server_config() override
+    {
+        return config;
+    }
+};
+}
+
+TEST_F(ClientLibraryErrors, exception_in_client_configuration_constructor_generates_error)
+{
+    mtf::UsingClientPlatform<ConfigurableFailureConfiguration<Method::the_client_platform_factory>> stubby;
+
+    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
+
+    EXPECT_FALSE(mir_connection_is_valid(connection));
+    EXPECT_THAT(mir_connection_get_error_message(connection), testing::HasSubstr(exception_text));
+    mir_connection_release(connection);
+}
+
+TEST_F(ClientLibraryErrors, exception_in_platform_construction_generates_error)
+{
+    mtf::UsingClientPlatform<ConfigurableFailureConfiguration<Method::create_client_platform>> stubby;
+
+    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
+
+    EXPECT_FALSE(mir_connection_is_valid(connection));
+    EXPECT_THAT(mir_connection_get_error_message(connection), testing::HasSubstr(exception_text));
+    mir_connection_release(connection);
+}
+
+TEST_F(ClientLibraryErrors, connecting_to_garbage_socket_returns_appropriate_error)
+{
+    using namespace testing;
+    mtf::UsingStubClientPlatform stubby;
+
+    auto connection = mir_connect_sync("garbage", __PRETTY_FUNCTION__);
+    ASSERT_THAT(connection, NotNull());
+
+    char const* error = mir_connection_get_error_message(connection);
+
+    if (std::strcmp("connect: No such file or directory", error) &&
+        std::strcmp("Can't find MIR server", error) &&
+        !std::strstr(error, "Failed to connect to server socket"))
+    {
+        FAIL() << error;
+    }
+    mir_connection_release(connection);
+}
+
+TEST_F(ClientLibraryErrors, create_surface_returns_error_object_on_failure)
+{
+    mtf::UsingClientPlatform<ConfigurableFailureConfiguration<Method::create_buffer_factory>> stubby;
+
+    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
+
+    ASSERT_THAT(connection, IsValid());
+
+    MirSurfaceParameters const request_params =
+    {
+        __PRETTY_FUNCTION__,
+        640, 480,
+        mir_pixel_format_abgr_8888,
+        mir_buffer_usage_hardware,
+        mir_display_output_id_invalid
+    };
+
+    auto surface = mir_connection_create_surface_sync(connection, &request_params);
+    ASSERT_NE(surface, nullptr);
+    EXPECT_FALSE(mir_surface_is_valid(surface));
+    EXPECT_THAT(mir_surface_get_error_message(surface), testing::HasSubstr(exception_text));
+
+    mir_surface_release_sync(surface);
+    mir_connection_release(connection);
+}
+
+namespace
+{
+void recording_surface_callback(MirSurface*, void* ctx)
+{
+    auto called = static_cast<bool*>(ctx);
+    *called = true;
+}
+}
+
+TEST_F(ClientLibraryErrors, surface_release_on_error_object_still_calls_callback)
+{
+    mtf::UsingClientPlatform<ConfigurableFailureConfiguration<Method::create_buffer_factory>> stubby;
+
+    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
+
+    ASSERT_THAT(connection, IsValid());
+
+    MirSurfaceParameters const request_params =
+    {
+        __PRETTY_FUNCTION__,
+        640, 480,
+        mir_pixel_format_abgr_8888,
+        mir_buffer_usage_hardware,
+        mir_display_output_id_invalid
+    };
+
+    auto surface = mir_connection_create_surface_sync(connection, &request_params);
+    ASSERT_NE(surface, nullptr);
+    EXPECT_FALSE(mir_surface_is_valid(surface));
+    EXPECT_THAT(mir_surface_get_error_message(surface), testing::HasSubstr(exception_text));
+
+    bool callback_called{false};
+    mir_surface_release(surface, &recording_surface_callback, &callback_called);
+    EXPECT_TRUE(callback_called);
+    mir_connection_release(connection);
+}
+
+
+TEST_F(ClientLibraryErrors, create_surface_returns_error_object_on_failure_in_reply_processing)
+{
+    mtf::UsingClientPlatform<ConfigurableFailureConfiguration<Method::create_egl_native_window>> stubby;
+
+    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
+
+    ASSERT_THAT(connection, IsValid());
+
+    MirSurfaceParameters const request_params =
+    {
+        __PRETTY_FUNCTION__,
+        640, 480,
+        mir_pixel_format_abgr_8888,
+        mir_buffer_usage_hardware,
+        mir_display_output_id_invalid
+    };
+
+    auto surface = mir_connection_create_surface_sync(connection, &request_params);
+    ASSERT_NE(surface, nullptr);
+    EXPECT_FALSE(mir_surface_is_valid(surface));
+    EXPECT_THAT(mir_surface_get_error_message(surface), testing::HasSubstr(exception_text));
+
+    mir_surface_release_sync(surface);
+    mir_connection_release(connection);
+}
+
+using ClientLibraryErrorsDeathTest = ClientLibraryErrors;
+
+
+TEST_F(ClientLibraryErrorsDeathTest, createing_surface_on_garbage_connection_is_fatal)
+{
+    mtf::UsingStubClientPlatform stubby;
+
+    auto connection = mir_connect_sync("garbage", __PRETTY_FUNCTION__);
+
+    MirSurfaceParameters const request_params =
+    {
+        __PRETTY_FUNCTION__,
+        640, 480,
+        mir_pixel_format_abgr_8888,
+        mir_buffer_usage_hardware,
+        mir_display_output_id_invalid
+    };
+
+    ASSERT_FALSE(mir_connection_is_valid(connection));
+    EXPECT_DEATH(mir_connection_create_surface_sync(connection, &request_params), "");
+}
+
+
+TEST_F(ClientLibraryErrorsDeathTest, creating_surface_synchronosly_on_malconstructed_connection_is_fatal)
+{
+    MirSurfaceParameters const request_params =
+    {
+        __PRETTY_FUNCTION__,
+        640, 480,
+        mir_pixel_format_abgr_8888,
+        mir_buffer_usage_hardware,
+        mir_display_output_id_invalid
+    };
+
+    mtf::UsingClientPlatform<ConfigurableFailureConfiguration<Method::the_client_platform_factory>> stubby;
+
+    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
+
+    ASSERT_FALSE(mir_connection_is_valid(connection));
+    EXPECT_DEATH(mir_connection_create_surface_sync(connection, &request_params), "");
+}
+
+TEST_F(ClientLibraryErrorsDeathTest, creating_surface_synchronosly_on_invalid_connection_is_fatal)
+{
+    MirSurfaceParameters const request_params =
+    {
+        __PRETTY_FUNCTION__,
+        640, 480,
+        mir_pixel_format_abgr_8888,
+        mir_buffer_usage_hardware,
+        mir_display_output_id_invalid
+    };
+
+    mtf::UsingClientPlatform<ConfigurableFailureConfiguration<Method::create_client_platform>> stubby;
+
+    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
+
+    ASSERT_FALSE(mir_connection_is_valid(connection));
+    EXPECT_DEATH(mir_connection_create_surface_sync(connection, &request_params), "");
+}

=== modified file 'tests/acceptance-tests/test_server_disconnect.cpp'
--- tests/acceptance-tests/test_server_disconnect.cpp	2014-10-01 06:25:56 +0000
+++ tests/acceptance-tests/test_server_disconnect.cpp	2014-11-05 22:26:46 +0000
@@ -107,7 +107,7 @@
                 mir_buffer_usage_hardware,
                 mir_display_output_id_invalid
             };
-            mir_connection_create_surface_sync(connection, &parameters);
+            surf = mir_connection_create_surface_sync(connection, &parameters);
         });
 
         configure_display.exec([&] {
@@ -117,6 +117,7 @@
         });
 
         disconnect.exec([&] {
+            mir_surface_release_sync(surf);
             mir_connection_release(connection);
         });
     }
@@ -129,6 +130,8 @@
     mt::CrossProcessAction create_surface;
     mt::CrossProcessAction configure_display;
     mt::CrossProcessAction disconnect;
+private:
+    MirSurface* surf;
 };
 
 /*

=== modified file 'tests/include/mir_test/stub_server_tool.h'
--- tests/include/mir_test/stub_server_tool.h	2014-10-01 06:25:56 +0000
+++ tests/include/mir_test/stub_server_tool.h	2014-11-05 22:26:46 +0000
@@ -86,7 +86,14 @@
                          ::google::protobuf::Closure* done) override
     {
         app_name = request->application_name();
-        connect_msg->set_error("");
+        // If you check out MirConnection::connected either the platform and display_configuration
+        // have to be set or the error has to be set, otherwise we die and fail to callback.
+        //
+        // Since setting the error to "" means that mir_connection_is_valid will return false
+        // with a confusing non-error-message, instead clear the error and set the platform et al.
+        connect_msg->clear_error();
+        connect_msg->set_allocated_platform(new mir::protobuf::Platform{});
+        connect_msg->set_allocated_display_configuration(new mir::protobuf::DisplayConfiguration{});
         done->Run();
     }
 

=== added file 'tests/include/mir_test/validity_matchers.h'
--- tests/include/mir_test/validity_matchers.h	1970-01-01 00:00:00 +0000
+++ tests/include/mir_test/validity_matchers.h	2014-11-05 22:26:46 +0000
@@ -0,0 +1,64 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Christopher James Halse Rogers <[e-mail]>
+ */
+
+#ifndef MIR_TEST_VALIDITY_MATCHERS_H_
+#define MIR_TEST_VALIDITY_MATCHERS_H_
+
+#include <gmock/gmock.h>
+
+#include "mir_toolkit/mir_client_library.h"
+
+using ::testing::MakePolymorphicMatcher;
+using ::testing::MatchResultListener;
+using ::testing::NotNull;
+using ::testing::PolymorphicMatcher;
+
+class IsValidMatcher {
+public:
+    // To implement a polymorphic matcher, first define a COPYABLE class
+    // that has three members MatchAndExplain(), DescribeTo(), and
+    // DescribeNegationTo(), like the following.
+
+    // In this example, we want to use NotNull() with any pointer, so
+    // MatchAndExplain() accepts a pointer of any type as its first argument.
+    // In general, you can define MatchAndExplain() as an ordinary method or
+    // a method template, or even overload it.
+    template <typename T>
+    bool MatchAndExplain(T* p, MatchResultListener* listener) const;
+
+    // Describes the property of a value matching this matcher.
+    void DescribeTo(::std::ostream* os) const { *os << "is valid"; }
+
+    // Describes the property of a value NOT matching this matcher.
+    void DescribeNegationTo(::std::ostream* os) const { *os << "is not valid"; }
+};
+
+template<>
+bool IsValidMatcher::MatchAndExplain(MirConnection* connection, MatchResultListener* listener) const;
+
+template<>
+bool IsValidMatcher::MatchAndExplain(MirSurface* surface, MatchResultListener* listener) const;
+
+// To construct a polymorphic matcher, pass an instance of the class
+// to MakePolymorphicMatcher().  Note the return type.
+inline PolymorphicMatcher<IsValidMatcher> IsValid()
+{
+    return MakePolymorphicMatcher(IsValidMatcher{});
+}
+
+#endif // MIR_TEST_VALIDITY_MATCHERS_H_

=== modified file 'tests/include/mir_test_framework/using_stub_client_platform.h'
--- tests/include/mir_test_framework/using_stub_client_platform.h	2014-10-01 06:25:56 +0000
+++ tests/include/mir_test_framework/using_stub_client_platform.h	2014-11-05 22:26:46 +0000
@@ -20,32 +20,76 @@
 #define MIR_TEST_FRAMEWORK_USING_STUB_CLIENT_PLATFORM_H_
 
 #include <memory>
-
-namespace mir
-{
-namespace client
-{
-class MirConnectionAPI;
-}
-}
+#include <functional>
+
+#include "mir_test_framework/stub_client_connection_configuration.h"
+#include "src/client/mir_connection_api.h"
+
+namespace mcl = mir::client;
 
 namespace mir_test_framework
 {
 
-class UsingStubClientPlatform
-{
-public:
-    UsingStubClientPlatform();
-    ~UsingStubClientPlatform();
-
-private:
-    UsingStubClientPlatform(UsingStubClientPlatform const&) = delete;
-    UsingStubClientPlatform operator=(UsingStubClientPlatform const&) = delete;
+class StubMirConnectionAPI : public mcl::MirConnectionAPI
+{
+public:
+    StubMirConnectionAPI(mcl::MirConnectionAPI* prev_api,
+                         mcl::ConfigurationFactory factory)
+        : prev_api{prev_api},
+          factory{factory}
+    {
+    }
+
+    MirWaitHandle* connect(
+        mcl::ConfigurationFactory configuration,
+        char const* socket_file,
+        char const* name,
+        mir_connected_callback callback,
+        void* context) override;
+
+    void release(MirConnection* connection) override;
+
+    mcl::ConfigurationFactory configuration_factory() override;
+
+private:
+    mcl::MirConnectionAPI* const prev_api;
+    mcl::ConfigurationFactory factory;
+};
+
+template<class ClientConfig>
+class UsingClientPlatform
+{
+public:
+    UsingClientPlatform()
+        : prev_api{mir_connection_api_impl},
+          stub_api{new StubMirConnectionAPI{prev_api, make_configuration_factory()}}
+    {
+        mir_connection_api_impl = stub_api.get();
+    }
+
+    ~UsingClientPlatform()
+    {
+        mir_connection_api_impl = prev_api;
+    }
+
+private:
+    UsingClientPlatform(UsingClientPlatform const&) = delete;
+    UsingClientPlatform operator=(UsingClientPlatform const&) = delete;
 
     mir::client::MirConnectionAPI* prev_api;
     std::unique_ptr<mir::client::MirConnectionAPI> stub_api;
+
+    mcl::ConfigurationFactory make_configuration_factory()
+    {
+        return [](std::string const& socket) {
+            return std::unique_ptr<mir::client::ConnectionConfiguration>{
+                new ClientConfig{socket}
+            };
+        };
+    }
 };
 
+using UsingStubClientPlatform = UsingClientPlatform<StubConnectionConfiguration>;
 }
 
 #endif /* MIR_TEST_FRAMEWORK_USING_STUB_CLIENT_PLATFORM_H_ */

=== modified file 'tests/integration-tests/client/test_client_render.cpp'
--- tests/integration-tests/client/test_client_render.cpp	2014-10-30 18:37:11 +0000
+++ tests/integration-tests/client/test_client_render.cpp	2014-11-05 22:26:46 +0000
@@ -28,6 +28,7 @@
 #include "examples/testdraw/patterns.h"
 #include "mir_test/stub_server_tool.h"
 #include "mir_test/test_protobuf_server.h"
+#include "mir_test/validity_matchers.h"
 
 #include "mir/frontend/connector.h"
 
@@ -92,7 +93,9 @@
             mir_buffer_usage_software, mir_display_output_id_invalid
         };
         auto connection = mir_connect_sync(socket_file, "test_renderer");
+        EXPECT_THAT(connection, IsValid());
         auto surface = mir_connection_create_surface_sync(connection, &surface_parameters);
+        EXPECT_THAT(surface, IsValid());
         MirGraphicsRegion graphics_region;
         for(int i=0u; i < num_frames; i++)
         {
@@ -119,6 +122,7 @@
         process_sync.wait_for_signal_ready_for();
 
         auto connection = mir_connect_sync(socket_file, "test_renderer");
+        EXPECT_THAT(connection, IsValid());
 
         /* set up egl context */
         int major, minor, n;
@@ -137,7 +141,9 @@
         eglInitialize(egl_display, &major, &minor);
         eglChooseConfig(egl_display, attribs, &egl_config, 1, &n);
 
-        auto mir_surface = create_mir_surface(connection, egl_display, egl_config);
+        auto mir_surface = create_mir_surface(connection, egl_display, egl_config);        
+        EXPECT_THAT(mir_surface, IsValid());
+
         auto native_window = static_cast<EGLNativeWindowType>(
             mir_surface_get_egl_native_window(mir_surface));
 

=== modified file 'tests/integration-tests/test_error_reporting.cpp'
--- tests/integration-tests/test_error_reporting.cpp	2014-10-01 06:25:56 +0000
+++ tests/integration-tests/test_error_reporting.cpp	2014-11-05 22:26:46 +0000
@@ -24,6 +24,7 @@
 #include "mir/frontend/connector.h"
 
 #include "src/server/frontend/display_server.h"
+#include "src/server/scene/surface_allocator.h"
 
 #include "mir_test_framework/display_server_test_fixture.h"
 #include "mir_test_doubles/stub_ipc_factory.h"
@@ -34,6 +35,7 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 #include "mir_test/gmock_fixes.h"
+#include "mir_test/validity_matchers.h"
 
 namespace mc = mir::compositor;
 namespace mf = mir::frontend;
@@ -45,32 +47,13 @@
 namespace
 {
 char const* const mir_test_socket = mtf::test_socket_file().c_str();
-
-struct ErrorServer : mf::detail::DisplayServer
+std::string const test_exception_text{"test exception text"};
+
+
+struct ConnectionErrorServer : mf::detail::DisplayServer
 {
     void client_pid(int /*pid*/) override {}
 
-    static std::string const test_exception_text;
-
-    void create_surface(
-        google::protobuf::RpcController*,
-        const mir::protobuf::SurfaceParameters*,
-        mir::protobuf::Surface*,
-        google::protobuf::Closure*)
-    {
-        throw std::runtime_error(test_exception_text);
-    }
-
-    void release_surface(
-        google::protobuf::RpcController*,
-        const mir::protobuf::SurfaceId*,
-        mir::protobuf::Void*,
-        google::protobuf::Closure*)
-    {
-        throw std::runtime_error(test_exception_text);
-    }
-
-
     void connect(
         ::google::protobuf::RpcController*,
         const ::mir::protobuf::ConnectParameters*,
@@ -90,8 +73,6 @@
     }
 };
 
-std::string const ErrorServer::test_exception_text{"test exception text"};
-
 struct SurfaceSync
 {
     SurfaceSync() :
@@ -193,7 +174,7 @@
 
 using ErrorReporting = BespokeDisplayServerTestFixture;
 
-TEST_F(ErrorReporting, c_api_returns_error)
+TEST_F(ErrorReporting, c_api_returns_connection_error)
 {
 
     struct ServerConfig : TestingServerConfiguration
@@ -201,7 +182,7 @@
         std::shared_ptr<mf::ProtobufIpcFactory> new_ipc_factory(
             std::shared_ptr<mf::SessionAuthorizer> const&) override
         {
-            static auto error_server = std::make_shared<ErrorServer>();
+            static auto error_server = std::make_shared<ConnectionErrorServer>();
             return std::make_shared<mtd::StubIpcFactory>(*error_server);
         }
     } server_config;
@@ -218,7 +199,46 @@
 
             ASSERT_TRUE(connection != NULL);
             EXPECT_FALSE(mir_connection_is_valid(connection));
-            EXPECT_TRUE(std::strstr(mir_connection_get_error_message(connection), ErrorServer::test_exception_text.c_str()));
+            EXPECT_TRUE(std::strstr(mir_connection_get_error_message(connection), test_exception_text.c_str()));
+
+            mir_connection_release(connection);
+        }
+    } client_config;
+
+    launch_client_process(client_config);
+}
+
+TEST_F(ErrorReporting, c_api_returns_surface_creation_error)
+{
+
+    struct ServerConfig : TestingServerConfiguration
+    {
+        class BoobytrappedSurfaceFactory : public mir::scene::SurfaceFactory
+        {
+            std::shared_ptr<mir::scene::Surface>
+            create_surface(mir::scene::SurfaceCreationParameters const&) override
+            {
+                throw std::runtime_error{test_exception_text};
+            }
+        };
+
+        std::shared_ptr<mir::scene::SurfaceFactory> the_surface_factory() override
+        {
+            return std::make_shared<BoobytrappedSurfaceFactory>();
+        }
+    } server_config;
+
+    launch_server_process(server_config);
+
+    struct ClientConfig : ClientConfigCommon
+    {
+        void exec()
+        {
+            mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
+
+            wait_for_connect();
+
+            ASSERT_THAT(connection, IsValid());
 
             MirSurfaceParameters const request_params =
             {
@@ -235,12 +255,7 @@
 
             ASSERT_TRUE(ssync->surface != NULL);
             EXPECT_FALSE(mir_surface_is_valid(ssync->surface));
-            EXPECT_TRUE(std::strstr(mir_surface_get_error_message(ssync->surface), ErrorServer::test_exception_text.c_str()));
-
-            EXPECT_NO_THROW({
-                MirSurfaceParameters response_params;
-                mir_surface_get_parameters(ssync->surface, &response_params);
-            });
+            EXPECT_TRUE(std::strstr(mir_surface_get_error_message(ssync->surface), test_exception_text.c_str()));
 
             mir_surface_release(ssync->surface, release_surface_callback, ssync);
 

=== modified file 'tests/mir_test/CMakeLists.txt'
--- tests/mir_test/CMakeLists.txt	2014-11-03 04:01:25 +0000
+++ tests/mir_test/CMakeLists.txt	2014-11-05 22:26:46 +0000
@@ -10,6 +10,7 @@
   popen.cpp
   wait_object.cpp
   current_thread_name.cpp
+  validity_matchers.cpp
 )
 
 target_link_libraries(mir-test mirprotobuf)

=== added file 'tests/mir_test/validity_matchers.cpp'
--- tests/mir_test/validity_matchers.cpp	1970-01-01 00:00:00 +0000
+++ tests/mir_test/validity_matchers.cpp	2014-11-05 22:26:46 +0000
@@ -0,0 +1,32 @@
+#include "mir_test/validity_matchers.h"
+#include "mir_toolkit/mir_client_library.h"
+
+template<>
+bool IsValidMatcher::MatchAndExplain(MirConnection* connection, MatchResultListener* listener) const
+{
+    if (connection == nullptr)
+    {
+        return false;
+    }
+    if (!mir_connection_is_valid(connection))
+    {
+        *listener << "error is: " << mir_connection_get_error_message(connection);
+        return false;
+    }
+    return true;
+}
+
+template<>
+bool IsValidMatcher::MatchAndExplain(MirSurface* surface, MatchResultListener* listener) const
+{
+    if (surface == nullptr)
+    {
+        return false;
+    }
+    if (!mir_surface_is_valid(surface))
+    {
+        *listener << "error is: " << mir_surface_get_error_message(surface);
+        return false;
+    }
+    return true;
+}

=== modified file 'tests/mir_test_framework/using_stub_client_platform.cpp'
--- tests/mir_test_framework/using_stub_client_platform.cpp	2014-10-27 15:22:31 +0000
+++ tests/mir_test_framework/using_stub_client_platform.cpp	2014-11-05 22:26:46 +0000
@@ -17,57 +17,27 @@
  */
 
 #include "mir_test_framework/using_stub_client_platform.h"
-#include "mir_test_framework/stub_client_connection_configuration.h"
 #include "mir_toolkit/mir_client_library.h"
-#include "src/client/mir_connection_api.h"
 
 namespace mtf = mir_test_framework;
 namespace mcl = mir::client;
 
-namespace
-{
-class StubMirConnectionAPI : public mcl::MirConnectionAPI
-{
-public:
-    StubMirConnectionAPI(mcl::MirConnectionAPI* prev_api)
-        : prev_api{prev_api}
-    {
-    }
-
-    MirWaitHandle* connect(
+MirWaitHandle* mtf::StubMirConnectionAPI::connect(
+        mcl::ConfigurationFactory /*configuration*/,
         char const* socket_file,
         char const* name,
         mir_connected_callback callback,
-        void* context) override
-    {
-        return prev_api->connect(socket_file, name, callback, context);
-    }
-
-    void release(MirConnection* connection) override
-    {
-        return prev_api->release(connection);
-    }
-
-    std::unique_ptr<mcl::ConnectionConfiguration> configuration(std::string const& socket) override
-    {
-        return std::unique_ptr<mcl::ConnectionConfiguration>{
-            new mtf::StubConnectionConfiguration{socket}};
-    }
-
-private:
-    mcl::MirConnectionAPI* const prev_api;
-};
-
-}
-
-mtf::UsingStubClientPlatform::UsingStubClientPlatform()
-    : prev_api{mir_connection_api_impl},
-      stub_api{new StubMirConnectionAPI{prev_api}}
-{
-    mir_connection_api_impl = stub_api.get();
-}
-
-mtf::UsingStubClientPlatform::~UsingStubClientPlatform()
-{
-    mir_connection_api_impl = prev_api;
+        void* context)
+{
+    return prev_api->connect(configuration_factory(), socket_file, name, callback, context);
+}
+
+void mtf::StubMirConnectionAPI::release(MirConnection* connection)
+{
+    return prev_api->release(connection);
+}
+
+mcl::ConfigurationFactory mtf::StubMirConnectionAPI::configuration_factory()
+{
+    return factory;
 }