Mir

Merge lp:~kdub/mir/device-detection into lp:mir

device-detection
Merge into development-branch

Proposed by Kevin DuBois on 2014-03-10

Status:	Merged
Merged at revision:	1648
Proposed branch:	lp:~kdub/mir/device-detection
Merge into:	lp:mir
Diff against target:	456 lines (+267/-0) 13 files modified CMakeLists.txt (+1/-0) cmake/FindAndroidProperties.cmake (+22/-0) debian/control (+1/-0) include/test/mir_test/android_device_detection.h (+36/-0) src/platform/graphics/android/CMakeLists.txt (+3/-0) src/platform/graphics/android/device_detector.cpp (+49/-0) src/platform/graphics/android/device_detector.h (+68/-0) tests/integration-tests/client/test_client_render.cpp (+5/-0) tests/integration-tests/graphics/android/test_buffer_integration.cpp (+4/-0) tests/integration-tests/graphics/android/test_display_integration.cpp (+2/-0) tests/integration-tests/graphics/android/test_internal_client.cpp (+2/-0) tests/unit-tests/graphics/android/CMakeLists.txt (+1/-0) tests/unit-tests/graphics/android/test_device_detection.cpp (+73/-0)
To merge this branch:	bzr merge lp:~kdub/mir/device-detection
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Alberto Aguirre (community)			Abstain on 2014-03-18
Daniel van Vugt			Needs Information on 2014-03-17
Alexandros Frantzis (community)			Approve on 2014-03-14
Alan Griffiths			Abstain on 2014-03-12
PS Jenkins bot (community)	continuous-integration		Approve on 2014-03-12
Chris Halse Rogers		2014-03-10	Needs Information on 2014-03-11
Review via email: mp+210292@code.launchpad.net

Commit message

Skip tests at runtime if the tests are being ran on a device without android hardware. This adds libandroid-properites1 as an android dependency.

Description of the change

Skip tests at runtime if the tests are being ran on a device without android hardware.

This adds libandroid-properites1 as a dependency, which is available is already in the system image for the phone images. This library will give the device name, (manta, mako, etc), or give back no device name if we're not on a ubuntu-touch device.

I figured its useful in the production code so android can log/blacklist/insert quirks based on the device name. I eventually want to log all the properties set, as they can effect driver behavior.

nb.
android has global settings stored in a properties cache (check it out with getprop/setprop). Because surfaceflinger is not configurable to the degree the driver vendors would like, oftentimes the property cache is used to specify different driver behavior (like swapinterval, number of overlays, etc). I'd like to avoid wading into setting the properties cache in mir, but being able to log the settings is useful for debug.

Revision history for this message

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

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-11:

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

review: Approve (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-03-11:

Is there some particular reason that you're manually adding a dependency on libandroid-properties1 to the platform-android package? The dpkg shlibs handling should pick this up automatically for you (if not, it's likely a bug in libandroid-properties-dev).

There's a lot of “char const[]” which are marked as static for no reason that's obvious to me. They're also explicitly sized to PROP_NAME_MAX, even though some of them are known to fit in smaller array - I presume this is because of some bizzare Android API?

If the buffers really need to be of size PROP_NAME_MAX, maybe this is a good time to use std::array? AFAIK annotating the formal parameters with an array size is only for API reference; it's entirely ignored by the compiler.

review: Needs Information

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-11:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-03-11:

94 + std::cout << "WARNING: test skipped due to lack of android drivers on system\n"; \

It seems inelegant to write directly to cout and not use the gtest output formatting but the latter appears to only be extensible when using XML output.

Revision history for this message

Kevin DuBois (kdub) wrote on 2014-03-11:

@Chris, good points, addressed.
@Alan, I didn't find a quick way to output using gtest's formatting either, will look some more though

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-03-11:

~~~
155+int mga::PropertiesOps::property_get(
156+ char const key[PROP_NAME_MAX],
157+ char value[PROP_VALUE_MAX],
158+ char const default_value[PROP_VALUE_MAX]) const
159+{
160+ return ::property_get(key, value, default_value);
161+}
~~~

why not:
std::string mga::PropertiesOps::property_get(std::string const& key, std::string const &default)
{
    char value[PROP_VALUE_MAX] = {'\0'};
    ::property_get(key.c_str(), value, default_value.c_str());
    value[PROP_VALUE_MAX - 1] = '\0';
    return std::string{value};
}

or return a std::pair<int, std::string> if you really want to check the return code of ::property_get.

~~~
165+ static char const key[PROP_NAME_MAX] = "ro.product.device";
166+ static char const default_value[PROP_VALUE_MAX] = "";
167+ static char value[PROP_VALUE_MAX] = "";
168+ properties.property_get(key, value, default_value);
169+ device_name_ = std::string{value};
170+ android_device_present_ = !(device_name_ == std::string{default_value});
~~~

Given above comment, then this can turn into:

std::string default_value{""};
device_name_ = properties.property_get("ro.product.device", default_value);
android_device_present_ = device_name_ != default_value;

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-11:

FAILED: Continuous integration, rev:1474
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1040/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1199/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1197
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/781/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/772/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/777/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/782/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-12:

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

review: Approve (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-03-12:

I can't help feeling that this approach is mildly wrong.

A test passing should mean one thing: that the code passes the test.

What we have here is a test passing indicates either that the test "decided" to pass without because of the environment or that the code passed the test.

The decision of whether a test is appropriate to the environment doesn't belong in the test execution - it should be determined before the test ever runs. And tests that are inappropriate shouldn't be run.

But that is probably significantly more work.

review: Abstain

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-03-14:

I agree this is not ideal (passing a test that we really just want to skip), but unfortunately googletest doesn't offer a straightforward way to achieve runtime skipping.

One suggestion I have found online is to detect whether we want to skip some tests when calling the test main() function, and pass the appropriate --gtest_filter pattern to the googletest init function. Unfortunately this is not ideal either, since it moves the skip logic away from the tests themselves.

review: Approve

Revision history for this message

Kevin DuBois (kdub) wrote on 2014-03-14:

@Alan, Alexandros

Like Alexandros mentions, the only things I could find for a more-integrated-with-gtest approach were to wade
into the gtest and the test's main function, which seemed quite a heavyweight solution.

@Alberto,
The point of the class is to be a lightweight, untested wrapper to facilitate easy testing of the class that is doing the logic. I want to keep it a 1-1 function signature match to keep it simple and keep bugs out of code that's tough to test (because of C linkage)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-03-17:

Looks good other than:

(1) I'm concerned that we're making it harder to port Mir to other distros (which probably don't have Android packages):
  -- checking for module 'libandroid-properties'
  -- package 'libandroid-properties' not found
  CMake Error at /usr/share/cmake-2.8/Modules/FindPkgConfig.cmake:283 (message):
    A required package was not found

Although that problem pre-dates this proposal, I'm wondering if there's a way to soften that requirement and be more flexible than: find_package(AndroidProperties REQUIRED)

review: Needs Information

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-03-18:

> @Alberto,
> The point of the class is to be a lightweight, untested wrapper to facilitate
> easy testing of the class that is doing the logic. I want to keep it a 1-1
> function signature match to keep it simple and keep bugs out of code that's
> tough to test (because of C linkage)

Then why wrap the api at all then? If you are going to wrap it, you might as well make it a bit nicer to call.

review: Abstain

Revision history for this message

Kevin DuBois (kdub) wrote on 2014-03-21:

Alan's point about 'reporting success when there it was merely skipped' is concerning to me to...

I'll table this until we see if this
https://code.launchpad.net/~afrantzis/mir/no-display-hw-tests-on-armhf-package-builds/+merge/212122
lands (as just passing the builders is the pressing issue):

Accessing this library is useful though for gathering debug information and potentially modifying driver configuration by setting different property keys so the MP would just drop the SKIP_IF_NO_ANDROID_HARDWARE_PRESENT parts

Revision history for this message

Kevin DuBois (kdub) wrote on 2014-03-21:

> concerning to me to...
to me too

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Emanuele Antonio Faraone

Gerry Boland

Kevin DuBois

Mir development team

 === modified file 'CMakeLists.txt'
 --- CMakeLists.txt	2014-03-06 06:05:17 +0000
 +++ CMakeLists.txt	2014-03-12 00:59:01 +0000
@@ -122,6 +122,7 @@
  find_package(GFlags REQUIRED)
  find_package(XKBCOMMON REQUIRED)
  find_package(LTTngUST REQUIRED)
++find_package(AndroidProperties REQUIRED)
  pkg_check_modules(UDEV REQUIRED libudev)
  include_directories (${GLESv2_INCLUDE_DIRS})
 === added file 'cmake/FindAndroidProperties.cmake'
 --- cmake/FindAndroidProperties.cmake	1970-01-01 00:00:00 +0000
 +++ cmake/FindAndroidProperties.cmake	2014-03-12 00:59:01 +0000
@@ -0,0 +1,22 @@
++# Variables defined by this module:
++#   ANDROID_PROPERTIES_FOUND
++#   ANDROID_PROPERTIES_LIBRARIES
++#   ANDROID_PROPERTIES_INCLUDE_DIRS
++
++INCLUDE(FindPackageHandleStandardArgs)
++
++find_package( PkgConfig )
++pkg_check_modules(ANDROID_PROPERTIES REQUIRED libandroid-properties)
++
++find_path(ANDROID_PROPERTIES_INCLUDE_DIR hybris/properties/properties.h
++          HINTS ${PC_ANDROID_PROPERTIES_INCLUDEDIR} ${PC_ANDROID_PROPERTIES_INCLUDE_DIRS})
++find_library(ANDROID_PROPERTIES_LIBRARIES
++             NAMES        libandroid-properties.so
++             HINTS ${PC_ANDROID_PROPERTIES_LIBDIR} ${PC_ANDROID_PROPERTIES_LIBRARY_DIRS})
++
++# handle the QUIETLY and REQUIRED arguments and set ANDROID_PROPERTIES_FOUND to TRUE
++# if all listed variables are TRUE
++find_package_handle_standard_args(ANDROID_PROPERTIES DEFAULT_MSG
++                                  ANDROID_PROPERTIES_LIBRARIES)
++
++mark_as_advanced(ANDROID_PROPERTIES_INCLUDE_DIR ANDROID_PROPERTIES_LIBRARY )
 === modified file 'debian/control'
 --- debian/control	2014-03-07 03:15:55 +0000
 +++ debian/control	2014-03-12 00:59:01 +0000
@@ -28,6 +28,7 @@
                 pkg-config,
                 android-headers (>=4.2.2) [i386 amd64 armhf],
                 libhardware-dev [i386 amd64 armhf],
++               libandroid-properties-dev,
                 libgoogle-glog-dev,
                 liblttng-ust-dev,
                 libxkbcommon-dev,
 === added file 'include/test/mir_test/android_device_detection.h'
 --- include/test/mir_test/android_device_detection.h	1970-01-01 00:00:00 +0000
 +++ include/test/mir_test/android_device_detection.h	2014-03-12 00:59:01 +0000
@@ -0,0 +1,36 @@
++/*
++ * 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: Kevin DuBois <kevin.dubois@canonical.com>
++ */
++
++#ifndef MIR_TEST_ANDROID_DEVICE_DETECTION_H_
++#define MIR_TEST_ANDROID_DEVICE_DETECTION_H_
++
++#include "src/platform/graphics/android/device_detector.h"
++#include <gtest/gtest.h>
++#include <iostream>
++
++#define SKIP_IF_NO_ANDROID_HARDWARE_PRESENT() \
++    mir::graphics::android::PropertiesOps ops; \
++    mir::graphics::android::DeviceDetector detector{ops}; \
++    if(!detector.android_device_present()) \
++    { \
++        std::cout << "WARNING: test skipped due to lack of android drivers on system\n"; \
++        SUCCEED(); \
++        return; \
++    }
++
++#endif /* MIR_TEST_ANDROID_DEVICE_DETECTION_H_ */
 === modified file 'src/platform/graphics/android/CMakeLists.txt'
 --- src/platform/graphics/android/CMakeLists.txt	2014-03-11 13:44:57 +0000
 +++ src/platform/graphics/android/CMakeLists.txt	2014-03-12 00:59:01 +0000
@@ -2,6 +2,7 @@
  include_directories(
      ${EGL_INCLUDE_DIRS}
      ${GLESv2_INCLUDE_DIRS}
++    ${ANDROID_PROPERTIES_INCLUDE_DIRS}
+ )
  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fpermissive")
@@ -32,6 +33,7 @@
    interpreter_cache.cpp
    internal_client.cpp
    gl_context.cpp
++  device_detector.cpp
    real_hwc_wrapper.cpp
+ )
@@ -50,6 +52,7 @@
    ${LIBHARDWARE_LIBRARIES}
    ${EGL_LDFLAGS} ${EGL_LIBRARIES}
    ${GLESv2_LDFLAGS} ${GLESv2_LIBRARIES}
++  ${ANDROID_PROPERTIES_LIBRARIES}
+ )
  install(TARGETS mirplatformgraphicsandroid LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}/mir/platformgraphics/android)
 === added file 'src/platform/graphics/android/device_detector.cpp'
 --- src/platform/graphics/android/device_detector.cpp	1970-01-01 00:00:00 +0000
 +++ src/platform/graphics/android/device_detector.cpp	2014-03-12 00:59:01 +0000
@@ -0,0 +1,49 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify
++ * it under the terms of the GNU Lesser 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 Lesser General Public License for more details.
++ *
++ * You should have received a copy of the GNU Lesser General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
++ */
++
++#include "device_detector.h"
++
++namespace mga=mir::graphics::android;
++
++int mga::PropertiesOps::property_get(
++    char const* key,
++    char* value,
++    char const* default_value) const
++{
++    return ::property_get(key, value, default_value);
++}
++
++mga::DeviceDetector::DeviceDetector(PropertiesWrapper const& properties)
++{
++    char const key[] = "ro.product.device";
++    char const default_value[] = "";
++    char value[PROP_VALUE_MAX] = "";
++    properties.property_get(key, value, default_value);
++    device_name_ = std::string{value};
++    android_device_present_ = !(device_name_ == std::string{default_value});
++}
++
++bool mga::DeviceDetector::android_device_present() const
++{
++    return android_device_present_;
++}
++
++std::string mga::DeviceDetector::device_name() const
++{
++    return device_name_;
++}
 === added file 'src/platform/graphics/android/device_detector.h'
 --- src/platform/graphics/android/device_detector.h	1970-01-01 00:00:00 +0000
 +++ src/platform/graphics/android/device_detector.h	2014-03-12 00:59:01 +0000
@@ -0,0 +1,68 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify
++ * it under the terms of the GNU Lesser 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 Lesser General Public License for more details.
++ *
++ * You should have received a copy of the GNU Lesser General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
++ */
++
++#ifndef MIR_GRAPHICS_ANDROID_DEVICE_DETECTOR_H_
++#define MIR_GRAPHICS_ANDROID_DEVICE_DETECTOR_H_
++
++#include <hybris/properties/properties.h>
++#include <string>
++
++namespace mir
++{
++namespace graphics
++{
++namespace android
++{
++class PropertiesWrapper
++{
++public:
++    PropertiesWrapper() = default;
++    virtual ~PropertiesWrapper() = default;
++    virtual int property_get(
++        char const* key,
++        char* value,
++        char const* default_value) const = 0;
++private:
++    PropertiesWrapper(PropertiesWrapper const&) = delete;
++    PropertiesWrapper& operator=(PropertiesWrapper const&) = delete;
++};
++
++class PropertiesOps : public PropertiesWrapper
++{
++public:
++    int property_get(
++        char const* key,
++        char* value,
++        char const* default_value) const;
++};
++
++class DeviceDetector
++{
++public:
++    DeviceDetector(PropertiesWrapper const& properties);
++
++    bool android_device_present() const;
++    std::string device_name() const;
++private:
++    std::string device_name_;
++    bool android_device_present_;
++};
++}
++}
++}
++#endif /* MIR_GRAPHICS_ANDROID_DEVICE_DETECTOR_H_ */
 === modified file 'tests/integration-tests/client/test_client_render.cpp'
 --- tests/integration-tests/client/test_client_render.cpp	2014-03-06 06:05:17 +0000
 +++ tests/integration-tests/client/test_client_render.cpp	2014-03-12 00:59:01 +0000
@@ -29,6 +29,7 @@
  #include "testdraw/patterns.h"
  #include "mir_test/stub_server_tool.h"
  #include "mir_test/test_protobuf_server.h"
++#include "mir_test/android_device_detection.h"
  #include "mir/frontend/connector.h"
@@ -346,6 +347,7 @@
  TEST_F(TestClientIPCRender, test_render_single)
+ {
++    SKIP_IF_NO_ANDROID_HARDWARE_PRESENT();
      sync1.try_signal_ready_for();
      /* wait for client to finish */
@@ -359,6 +361,7 @@
  TEST_F(TestClientIPCRender, test_render_double)
+ {
++    SKIP_IF_NO_ANDROID_HARDWARE_PRESENT();
      sync2.try_signal_ready_for();
      /* wait for client to finish */
@@ -375,6 +378,7 @@
  TEST_F(TestClientIPCRender, test_accelerated_render)
+ {
++    SKIP_IF_NO_ANDROID_HARDWARE_PRESENT();
      mtd::DrawPatternSolid red_pattern(0xFF0000FF);
      sync3.try_signal_ready_for();
@@ -389,6 +393,7 @@
  TEST_F(TestClientIPCRender, test_accelerated_render_double)
+ {
++    SKIP_IF_NO_ANDROID_HARDWARE_PRESENT();
      mtd::DrawPatternSolid red_pattern(0xFF0000FF);
      mtd::DrawPatternSolid green_pattern(0xFF00FF00);
 === modified file 'tests/integration-tests/graphics/android/test_buffer_integration.cpp'
 --- tests/integration-tests/graphics/android/test_buffer_integration.cpp	2014-03-11 13:44:57 +0000
 +++ tests/integration-tests/graphics/android/test_buffer_integration.cpp	2014-03-12 00:59:01 +0000
@@ -23,6 +23,7 @@
  #include "mir/graphics/android/native_buffer.h"
  #include "mir/graphics/buffer_properties.h"
++#include "mir_test/android_device_detection.h"
  #include "testdraw/graphics_region_factory.h"
  #include "testdraw/patterns.h"
@@ -84,6 +85,7 @@
  TEST_F(AndroidBufferIntegration, allocator_can_create_sw_buffer)
+ {
++    SKIP_IF_NO_ANDROID_HARDWARE_PRESENT();
      using namespace testing;
      auto allocator = std::make_shared<mga::AndroidGraphicBufferAllocator>(null_buffer_initializer);
@@ -100,6 +102,7 @@
  TEST_F(AndroidBufferIntegration, allocator_can_create_hw_buffer)
+ {
++    SKIP_IF_NO_ANDROID_HARDWARE_PRESENT();
      using namespace testing;
      mg::BufferProperties hw_properties{size, pf, mg::BufferUsage::hardware};
@@ -112,6 +115,7 @@
  TEST_F(AndroidBufferIntegration, swapper_creation_is_sane)
+ {
++    SKIP_IF_NO_ANDROID_HARDWARE_PRESENT();
      using namespace testing;
      auto allocator = std::make_shared<mga::AndroidGraphicBufferAllocator>(null_buffer_initializer);
 === modified file 'tests/integration-tests/graphics/android/test_display_integration.cpp'
 --- tests/integration-tests/graphics/android/test_display_integration.cpp	2014-03-11 13:44:57 +0000
 +++ tests/integration-tests/graphics/android/test_display_integration.cpp	2014-03-12 00:59:01 +0000
@@ -25,6 +25,7 @@
  #include "examples/graphics.h"
  #include "mir_test_doubles/mock_display_report.h"
++#include "mir_test/android_device_detection.h"
  #include <gtest/gtest.h>
  #include <stdexcept>
@@ -70,6 +71,7 @@
  TEST_F(AndroidDisplay, display_can_post)
+ {
++    SKIP_IF_NO_ANDROID_HARDWARE_PRESENT();
      auto mock_display_report = std::make_shared<testing::NiceMock<mtd::MockDisplayReport>>();
      auto buffer_initializer = std::make_shared<mg::NullBufferInitializer>();
      auto fb_allocator = std::make_shared<mga::AndroidGraphicBufferAllocator>(buffer_initializer);
 === modified file 'tests/integration-tests/graphics/android/test_internal_client.cpp'
 --- tests/integration-tests/graphics/android/test_internal_client.cpp	2014-03-06 06:05:17 +0000
 +++ tests/integration-tests/graphics/android/test_internal_client.cpp	2014-03-12 00:59:01 +0000
@@ -41,6 +41,7 @@
  #include "mir_test_doubles/stub_input_registrar.h"
  #include "mir_test_doubles/null_surface_configurator.h"
++#include "mir_test/android_device_detection.h"
  #include <EGL/egl.h>
  #include <gtest/gtest.h>
@@ -81,6 +82,7 @@
  TEST_F(AndroidInternalClient, internal_client_creation_and_use)
+ {
++    SKIP_IF_NO_ANDROID_HARDWARE_PRESENT();
      auto size = geom::Size{334, 122};
      auto pf  = mir_pixel_format_abgr_8888;
      msh::SurfaceCreationParameters params;
 === modified file 'tests/unit-tests/graphics/android/CMakeLists.txt'
 --- tests/unit-tests/graphics/android/CMakeLists.txt	2014-03-11 13:44:57 +0000
 +++ tests/unit-tests/graphics/android/CMakeLists.txt	2014-03-12 00:59:01 +0000
@@ -24,6 +24,7 @@
    ${CMAKE_CURRENT_SOURCE_DIR}/test_interpreter_buffer_cache.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_external_refcount.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_output_builder.cpp
++  ${CMAKE_CURRENT_SOURCE_DIR}/test_device_detection.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_hwc_wrapper.cpp
+ )
 === added file 'tests/unit-tests/graphics/android/test_device_detection.cpp'
 --- tests/unit-tests/graphics/android/test_device_detection.cpp	1970-01-01 00:00:00 +0000
 +++ tests/unit-tests/graphics/android/test_device_detection.cpp	2014-03-12 00:59:01 +0000
@@ -0,0 +1,73 @@
++/*
++ * 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: Kevin DuBois <kevin.dubois@canonical.com>
++ */
++
++#include "src/platform/graphics/android/device_detector.h"
++#include <gmock/gmock.h>
++#include <gtest/gtest.h>
++
++namespace mga = mir::graphics::android;
++
++namespace
++{
++struct MockOps : mga::PropertiesWrapper
++{
++    MOCK_CONST_METHOD3(property_get, int(
++        char const[PROP_NAME_MAX], char[PROP_VALUE_MAX], char const[PROP_VALUE_MAX]));
++};
++}
++
++TEST(DeviceDetection, detects_device)
++{
++    using namespace testing;
++    char const default_str[] = "";
++    char const name_str[] = "tunafish";
++
++    MockOps mock_ops;
++    EXPECT_CALL(mock_ops, property_get(StrEq("ro.product.device"), _, StrEq(default_str)))
++        .Times(1)
++        .WillOnce(Invoke([&]
++        (char const*, char* value, char const*)
++        {
++            strncpy(value, name_str, PROP_VALUE_MAX);
++            return 0;
++        }));
++
++    mga::DeviceDetector detector(mock_ops);
++    EXPECT_TRUE(detector.android_device_present());
++    EXPECT_EQ(std::string{name_str}, detector.device_name());
++}
++
++TEST(DeviceDetection, does_not_detect_device)
++{
++    using namespace testing;
++    char const default_str[] = "";
++
++    MockOps mock_ops;
++    EXPECT_CALL(mock_ops, property_get(StrEq("ro.product.device"), _, StrEq(default_str)))
++        .Times(1)
++        .WillOnce(Invoke([&]
++        (char const*, char* value, char const* default_value)
++        {
++            strncpy(value, default_value, PROP_VALUE_MAX);
++            return 0;
++        }));
++
++    mga::DeviceDetector detector(mock_ops);
++    EXPECT_FALSE(detector.android_device_present());
++    EXPECT_EQ(std::string{}, detector.device_name());
++}