diff options
| -rw-r--r-- | src/adbd-client.cpp | 25 | ||||
| -rw-r--r-- | src/greeter.cpp | 192 | ||||
| -rw-r--r-- | src/greeter.h | 10 | ||||
| -rw-r--r-- | src/usb-manager.cpp | 103 | ||||
| -rw-r--r-- | src/usb-snap.cpp | 1 | ||||
| -rw-r--r-- | tests/integration/usb-manager-test.cpp | 16 | ||||
| -rw-r--r-- | tests/unit/CMakeLists.txt | 5 | ||||
| -rw-r--r-- | tests/unit/adbd-client-test.cpp | 5 | ||||
| -rw-r--r-- | tests/unit/greeter-test.cpp | 159 | ||||
| -rw-r--r-- | tests/unit/usb-snap-test.cpp | 13 | ||||
| -rw-r--r-- | tests/utils/adbd-server.h | 20 | ||||
| -rw-r--r-- | tests/utils/gtest-print-helpers.h | 18 | ||||
| -rw-r--r-- | tests/utils/mock-greeter.h | 4 | ||||
| -rw-r--r-- | tests/utils/mock-unity-greeter.py | 41 |
14 files changed, 458 insertions, 154 deletions
diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp index 400c7c9..47914cb 100644 --- a/src/adbd-client.cpp +++ b/src/adbd-client.cpp @@ -23,6 +23,7 @@ #include <gio/gunixsocketaddress.h> #include <algorithm> +#include <atomic> #include <cctype> #include <cstring> #include <chrono> @@ -48,7 +49,9 @@ public: g_cancellable_cancel(m_cancellable); m_pkresponse_cv.notify_one(); m_sleep_cv.notify_one(); - m_worker_thread.join(); + if (m_worker_thread.joinable()) { + m_worker_thread.join(); + } g_clear_object(&m_cancellable); } @@ -66,7 +69,7 @@ private: GCancellable* cancellable = nullptr; const std::string public_key; - PKIdleData(Impl* self_, GCancellable* cancellable_, std::string public_key_): + PKIdleData(Impl* self_, GCancellable* cancellable_, const std::string& public_key_): self(self_), cancellable(G_CANCELLABLE(g_object_ref(cancellable_))), public_key(public_key_) {} @@ -104,8 +107,9 @@ private: void on_public_key_response(PKResponse response) { + g_debug("%s thread %p got response %d", G_STRLOC, g_thread_self(), int(response)); + // set m_pkresponse and wake up the waiting worker thread - std::unique_lock<std::mutex> lk(m_pkresponse_mutex); m_pkresponse = response; m_pkresponse_ready = true; m_pkresponse_cv.notify_one(); @@ -121,11 +125,11 @@ private: while (!g_cancellable_is_cancelled(m_cancellable)) { - g_debug("%s creating a client socket to '%s'", G_STRLOC, socket_path.c_str()); + g_debug("%s thread %p creating a client socket to '%s'", G_STRLOC, g_thread_self(), socket_path.c_str()); auto socket = create_client_socket(socket_path); bool got_valid_req = false; - g_debug("%s calling read_request", G_STRLOC); + g_debug("%s thread %p calling read_request", g_thread_self(), G_STRLOC); std::string reqstr; if (socket != nullptr) reqstr = read_request(socket); @@ -135,22 +139,25 @@ private: if (reqstr.substr(0,2) == "PK") { PKResponse response = PKResponse::DENY; const auto public_key = reqstr.substr(2); - g_debug("%s got pk [%s]", G_STRLOC, public_key.c_str()); + g_debug("%s thread %p got pk [%s]", G_STRLOC, g_thread_self(), public_key.c_str()); if (!public_key.empty()) { got_valid_req = true; std::unique_lock<std::mutex> lk(m_pkresponse_mutex); m_pkresponse_ready = false; + m_pkresponse = AdbdClient::PKResponse::DENY; pass_public_key_to_main_thread(public_key); m_pkresponse_cv.wait(lk, [this](){ return m_pkresponse_ready || g_cancellable_is_cancelled(m_cancellable); }); response = m_pkresponse; - g_debug("%s got response '%d', is-cancelled %d", G_STRLOC, + g_debug("%s thread %p got response '%d', is-cancelled %d", G_STRLOC, + g_thread_self(), int(response), int(g_cancellable_is_cancelled(m_cancellable))); } - if (!g_cancellable_is_cancelled(m_cancellable)) + if (!g_cancellable_is_cancelled(m_cancellable)) { send_pk_response(socket, response); + } } else if (!reqstr.empty()) { g_warning("Invalid ADB request: [%s]", reqstr.c_str()); } @@ -270,7 +277,7 @@ private: std::mutex m_pkresponse_mutex; std::condition_variable m_pkresponse_cv; - bool m_pkresponse_ready = false; + std::atomic<bool> m_pkresponse_ready {false}; PKResponse m_pkresponse = PKResponse::DENY; }; diff --git a/src/greeter.cpp b/src/greeter.cpp index f9cd965..3d0f347 100644 --- a/src/greeter.cpp +++ b/src/greeter.cpp @@ -26,117 +26,167 @@ class UnityGreeter::Impl { public: - Impl(): - m_cancellable{g_cancellable_new()} + Impl() { - g_bus_get(G_BUS_TYPE_SESSION, m_cancellable, on_bus_ready_static, this); + m_cancellable.reset( + g_cancellable_new(), + [](GCancellable* o){ + g_cancellable_cancel(o); + g_clear_object(&o); + } + ); + + g_bus_get(G_BUS_TYPE_SESSION, m_cancellable.get(), on_bus_ready, this); } - ~Impl() - { - g_cancellable_cancel(m_cancellable); - g_clear_object(&m_cancellable); - - if (m_subscription_id != 0) - g_dbus_connection_signal_unsubscribe (m_bus, m_subscription_id); + ~Impl() =default; - g_clear_object(&m_bus); - } - - core::Property<bool>& is_active() + core::Property<State>& state() { - return m_is_active; + return m_state; } private: - static void on_bus_ready_static(GObject* /*source*/, GAsyncResult* res, gpointer gself) + void set_state(const State& state) + { + m_state.set(state); + } + + static void on_bus_ready( + GObject* /*source*/, + GAsyncResult* res, + gpointer gself) { GError* error {}; - auto bus = g_bus_get_finish (res, &error); - if (error != nullptr) { + auto bus = g_bus_get_finish(res, &error); + if (error != nullptr) + { if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) - g_warning("UsbSnap: Error getting session bus: %s", error->message); + g_warning("Greeter: Error getting bus: %s", error->message); g_clear_error(&error); - } else { - static_cast<Impl*>(gself)->on_bus_ready(bus); } - g_clear_object(&bus); + else + { + auto self = static_cast<Impl*>(gself); + + const auto watcher_id = g_bus_watch_name_on_connection( + bus, + DBusNames::UnityGreeter::NAME, + G_BUS_NAME_WATCHER_FLAGS_AUTO_START, + on_greeter_appeared, + on_greeter_vanished, + gself, + nullptr); + + const auto subscription_id = g_dbus_connection_signal_subscribe( + bus, + DBusNames::UnityGreeter::NAME, + DBusNames::Properties::INTERFACE, + DBusNames::Properties::PropertiesChanged::NAME, + DBusNames::UnityGreeter::PATH, + DBusNames::UnityGreeter::INTERFACE, + G_DBUS_SIGNAL_FLAGS_NONE, + on_properties_changed_signal, + gself, + nullptr); + + self->m_bus.reset( + bus, + [watcher_id, subscription_id](GDBusConnection* o){ + g_bus_unwatch_name(watcher_id); + g_dbus_connection_signal_unsubscribe(o, subscription_id); + g_clear_object(&o); + } + ); + } + } + + static void on_greeter_appeared( + GDBusConnection* bus, + const char* /*name*/, + const char* name_owner, + gpointer gself) + { + auto self = static_cast<Impl*>(gself); + + self->m_owner = name_owner; + + g_dbus_connection_call( + bus, + DBusNames::UnityGreeter::NAME, + DBusNames::UnityGreeter::PATH, + DBusNames::Properties::INTERFACE, + "Get", + g_variant_new("(ss)", DBusNames::UnityGreeter::INTERFACE, "IsActive"), + G_VARIANT_TYPE("(v)"), + G_DBUS_CALL_FLAGS_NONE, + -1, + self->m_cancellable.get(), + on_get_is_active_ready, + gself); } - void on_bus_ready(GDBusConnection* bus) + static void on_greeter_vanished( + GDBusConnection* /*bus*/, + const char* /*name*/, + gpointer gself) { - m_bus = G_DBUS_CONNECTION(g_object_ref(G_OBJECT(bus))); - - g_dbus_connection_call(m_bus, - DBusNames::UnityGreeter::NAME, - DBusNames::UnityGreeter::PATH, - DBusNames::Properties::INTERFACE, - "Get", - g_variant_new("(ss)", DBusNames::UnityGreeter::INTERFACE, "IsActive"), - G_VARIANT_TYPE("(v)"), - G_DBUS_CALL_FLAGS_NONE, - -1, - m_cancellable, - on_get_is_active_ready, - this); - - m_subscription_id = g_dbus_connection_signal_subscribe(m_bus, - DBusNames::UnityGreeter::NAME, - DBusNames::Properties::INTERFACE, - DBusNames::Properties::PropertiesChanged::NAME, - DBusNames::UnityGreeter::PATH, - DBusNames::UnityGreeter::INTERFACE, - G_DBUS_SIGNAL_FLAGS_NONE, - on_properties_changed_signal, - this, - nullptr); + auto self = static_cast<Impl*>(gself); + + self->m_owner.clear(); + self->set_state(State::UNAVAILABLE); } - static void on_get_is_active_ready(GObject* source, GAsyncResult* res, gpointer gself) + static void on_get_is_active_ready( + GObject* source, + GAsyncResult* res, + gpointer gself) { GError* error {}; auto v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &error); if (error != nullptr) { - if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) - g_warning("UsbSnap: Error getting session bus: %s", error->message); + if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { + g_warning("Greeter: Error getting IsActive property: %s", error->message); + } g_clear_error(&error); } else { GVariant* is_active {}; g_variant_get_child(v, 0, "v", &is_active); - static_cast<Impl*>(gself)->m_is_active.set(g_variant_get_boolean(is_active)); + static_cast<Impl*>(gself)->set_state(g_variant_get_boolean(is_active) ? State::ACTIVE : State::INACTIVE); g_clear_pointer(&is_active, g_variant_unref); } g_clear_pointer(&v, g_variant_unref); } - static void on_properties_changed_signal(GDBusConnection* /*connection*/, - const gchar* /*sender_name*/, - const gchar* object_path, - const gchar* interface_name, - const gchar* signal_name, - GVariant* parameters, - gpointer gself) + static void on_properties_changed_signal( + GDBusConnection* /*bus*/, + const gchar* sender_name, + const gchar* object_path, + const gchar* interface_name, + const gchar* signal_name, + GVariant* parameters, + gpointer gself) { + auto self = static_cast<Impl*>(gself); + + g_return_if_fail(!g_strcmp0(sender_name, self->m_owner.c_str())); g_return_if_fail(!g_strcmp0(object_path, DBusNames::UnityGreeter::PATH)); g_return_if_fail(!g_strcmp0(interface_name, DBusNames::Properties::INTERFACE)); g_return_if_fail(!g_strcmp0(signal_name, DBusNames::Properties::PropertiesChanged::NAME)); g_return_if_fail(g_variant_is_of_type(parameters, G_VARIANT_TYPE(DBusNames::Properties::PropertiesChanged::ARGS_VARIANT_TYPE))); - auto v = g_variant_get_child_value (parameters, 1); + auto v = g_variant_get_child_value(parameters, 1); gboolean is_active {}; if (g_variant_lookup(v, "IsActive", "b", &is_active)) - { - g_debug("%s is_active changed to %d", G_STRLOC, int(is_active)); - static_cast<Impl*>(gself)->m_is_active.set(is_active); - } + self->set_state(is_active ? State::ACTIVE : State::INACTIVE); g_clear_pointer(&v, g_variant_unref); } - core::Property<bool> m_is_active; - GCancellable* m_cancellable {}; - GDBusConnection* m_bus {}; - unsigned int m_subscription_id {}; + core::Property<State> m_state {State::UNAVAILABLE}; + std::shared_ptr<GCancellable> m_cancellable; + std::shared_ptr<GDBusConnection> m_bus; + std::string m_owner; }; /*** @@ -154,8 +204,8 @@ UnityGreeter::UnityGreeter(): UnityGreeter::~UnityGreeter() =default; -core::Property<bool>& -UnityGreeter::is_active() +core::Property<Greeter::State>& +UnityGreeter::state() { - return impl->is_active(); + return impl->state(); } diff --git a/src/greeter.h b/src/greeter.h index e084d25..f3012f6 100644 --- a/src/greeter.h +++ b/src/greeter.h @@ -29,7 +29,13 @@ class Greeter public: Greeter(); virtual ~Greeter(); - virtual core::Property<bool>& is_active() =0; + + enum class State { UNAVAILABLE, INACTIVE, ACTIVE }; +static inline const char* state_str(const State& state) { + static constexpr char const * state_str[] = { "Unavailable", "Inactive", "Active" }; + return state_str[int(state)]; +} + virtual core::Property<State>& state() =0; }; @@ -38,7 +44,7 @@ class UnityGreeter: public Greeter public: UnityGreeter(); virtual ~UnityGreeter(); - core::Property<bool>& is_active() override; + core::Property<State>& state() override; protected: class Impl; diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp index 4d750c0..f83b5f1 100644 --- a/src/usb-manager.cpp +++ b/src/usb-manager.cpp @@ -46,72 +46,89 @@ public: m_greeter{greeter} { m_usb_monitor->on_usb_disconnected().connect([this](const std::string& /*usb_name*/) { - restart(); + m_req.reset(); }); - m_greeter->is_active().changed().connect([this](bool /*is_active*/) { - maybe_snap(); + m_greeter->state().changed().connect([this](const Greeter::State& state) { + if (state == Greeter::State::INACTIVE) { + maybe_snap(); + } else { + stop_snap(); + } }); - restart(); + // create a new adbd client + m_adbd_client.reset(new GAdbdClient{m_socket_path}); + m_adbd_client->on_pk_request().connect( + [this](const AdbdClient::PKRequest& req) { + g_debug("%s got pk request: %s, calling maybe_snap()", G_STRLOC, req.fingerprint.c_str()); + + m_response = AdbdClient::PKResponse::DENY; // set the fallback response + m_req.reset( + new AdbdClient::PKRequest(req), + [this](AdbdClient::PKRequest* r) { + stop_snap(); + r->respond(m_response); + delete r; + } + ); + maybe_snap(); + } + ); } ~Impl() { - if (m_restart_idle_tag) - g_source_remove(m_restart_idle_tag); - - clear(); + if (m_request_complete_idle_tag) { + g_source_remove(m_request_complete_idle_tag); + } } private: - void clear() + void stop_snap() { - // clear out old state m_snap_connections.clear(); m_snap.reset(); - m_req = decltype(m_req)(); - m_adbd_client.reset(); } - void restart() + void maybe_snap() { - clear(); + // only prompt if there's something to prompt about + if (!m_req) { + return; + } - // set a new client - m_adbd_client.reset(new GAdbdClient{m_socket_path}); - m_adbd_client->on_pk_request().connect( - [this](const AdbdClient::PKRequest& req) { - g_debug("%s got pk request: %s", G_STRLOC, req.fingerprint.c_str()); - m_req = req; - maybe_snap(); - } - ); - } + // only prompt in an unlocked session + if (m_greeter->state().get() != Greeter::State::INACTIVE) { + return; + } - void maybe_snap() - { - // don't prompt in the greeter! - if (!m_req.public_key.empty() && !m_greeter->is_active().get()) - snap(); + snap(); } void snap() { - m_snap = std::make_shared<UsbSnap>(m_req.fingerprint); + m_snap = std::make_shared<UsbSnap>(m_req->fingerprint); m_snap_connections.insert((*m_snap).on_user_response().connect( [this](AdbdClient::PKResponse response, bool remember_choice){ - g_debug("%s user responded! response %d, remember %d", G_STRLOC, int(response), int(remember_choice)); - m_req.respond(response); - if (remember_choice && (response == AdbdClient::PKResponse::ALLOW)) - write_public_key(m_req.public_key); - m_restart_idle_tag = g_idle_add([](gpointer gself){ - auto self = static_cast<Impl*>(gself); - self->m_restart_idle_tag = 0; - self->restart(); - return G_SOURCE_REMOVE; - }, this); + + if (remember_choice && (response == AdbdClient::PKResponse::ALLOW)) { + write_public_key(m_req->public_key); + } + + m_response = response; + + // defer finishing the request into an idle func because + // ScopedConnections can't be destroyed inside their callbacks + if (m_request_complete_idle_tag == 0) { + m_request_complete_idle_tag = g_idle_add([](gpointer gself){ + auto self = static_cast<Impl*>(gself); + self->m_request_complete_idle_tag = 0; + self->m_req.reset(); + return G_SOURCE_REMOVE; + }, this); + } } )); } @@ -152,12 +169,13 @@ private: const std::shared_ptr<UsbMonitor> m_usb_monitor; const std::shared_ptr<Greeter> m_greeter; - unsigned int m_restart_idle_tag {}; + unsigned int m_request_complete_idle_tag {}; std::shared_ptr<GAdbdClient> m_adbd_client; - AdbdClient::PKRequest m_req; std::shared_ptr<UsbSnap> m_snap; std::set<core::ScopedConnection> m_snap_connections; + AdbdClient::PKResponse m_response {AdbdClient::PKResponse::DENY}; + std::shared_ptr<AdbdClient::PKRequest> m_req; }; /*** @@ -177,4 +195,3 @@ UsbManager::UsbManager( UsbManager::~UsbManager() { } - diff --git a/src/usb-snap.cpp b/src/usb-snap.cpp index ba964fb..53db6c4 100644 --- a/src/usb-snap.cpp +++ b/src/usb-snap.cpp @@ -208,6 +208,7 @@ private: const bool remember_this_choice = response == AdbdClient::PKResponse::ALLOW; m_on_user_response(response, remember_this_choice); + m_notification_id = 0; } void on_notification_closed(uint32_t close_reason) diff --git a/tests/integration/usb-manager-test.cpp b/tests/integration/usb-manager-test.cpp index d62756f..805ba6e 100644 --- a/tests/integration/usb-manager-test.cpp +++ b/tests/integration/usb-manager-test.cpp @@ -143,7 +143,7 @@ TEST_F(UsbManagerFixture, Allow) ); // confirm that the AdbdServer got the right response - wait_for([adbd_server](){return !adbd_server->m_responses.empty();}, 2000); + wait_for([adbd_server](){return !adbd_server->m_responses.empty();}, 5000); ASSERT_EQ(1, adbd_server->m_responses.size()); EXPECT_EQ("OK", adbd_server->m_responses.front()); @@ -163,13 +163,16 @@ TEST_F(UsbManagerFixture, USBDisconnectedDuringPrompt) const std::shared_ptr<std::string> public_keys_path {new std::string{*m_tmpdir+"/adb_keys"}, file_deleter}; // start a mock AdbdServer ready to submit a request + const size_t N_TESTS {3}; const std::string public_key {"public_key"}; - auto adbd_server = std::make_shared<GAdbdServer>(*socket_path, std::vector<std::string>{"PK"+public_key}); + const std::vector<std::string> requests(N_TESTS, "PK"+public_key); + const std::vector<std::string> expected_responses(N_TESTS, "NO"); + auto adbd_server = std::make_shared<GAdbdServer>(*socket_path, requests); // set up a UsbManager to process the request auto usb_manager = std::make_shared<UsbManager>(*socket_path, *public_keys_path, m_usb_monitor, m_greeter); - for (int i=0; i<3; i++) + for (std::remove_const<decltype(N_TESTS)>::type i=0; i<N_TESTS; ++i) { // add a signal spy to listen to the notification daemon QSignalSpy notificationsSpy( @@ -194,6 +197,9 @@ TEST_F(UsbManagerFixture, USBDisconnectedDuringPrompt) EXPECT_EQ("CloseNotification", notificationsSpy.at(0).at(0)); notificationsSpy.clear(); } + + EXPECT_TRUE(wait_for([adbd_server, N_TESTS](){return adbd_server->m_responses.size() == N_TESTS;}, 5000)); + EXPECT_EQ(expected_responses, adbd_server->m_responses); } TEST_F(UsbManagerFixture, Greeter) @@ -206,7 +212,7 @@ TEST_F(UsbManagerFixture, Greeter) auto adbd_server = std::make_shared<GAdbdServer>(*socket_path, std::vector<std::string>{"PK"+public_key}); // set up a UsbManager to process the request - m_greeter->m_is_active.set(true); + m_greeter->m_state.set(Greeter::State::ACTIVE); auto usb_manager = std::make_shared<UsbManager>(*socket_path, *public_keys_path, m_usb_monitor, m_greeter); // add a signal spy to listen to the notification daemon @@ -219,7 +225,7 @@ TEST_F(UsbManagerFixture, Greeter) EXPECT_FALSE(notificationsSpy.wait(2000)); // disable the greeter, the notification should appear - m_greeter->m_is_active.set(false); + m_greeter->m_state.set(Greeter::State::INACTIVE); wait_for_signals(notificationsSpy, 1); EXPECT_EQ("Notify", notificationsSpy.at(0).at(0)); notificationsSpy.clear(); diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index fe70461..9d8cad2 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -14,6 +14,10 @@ set(TEST_LINK_LIBRARIES ${GMOCK_LIBRARIES} ) +add_definitions( + -DGREETER_TEMPLATE="${CMAKE_SOURCE_DIR}/tests/utils/mock-unity-greeter.py" +) + function(add_test_by_name name) set(TEST_NAME ${name}) add_executable (${TEST_NAME} ${TEST_NAME}.cpp) @@ -31,4 +35,5 @@ function(add_qt_test_by_name name) set_property(TEST ${TEST_NAME} APPEND PROPERTY ENVIRONMENT ${CTEST_ENVIRONMENT}) target_link_libraries(${TEST_NAME} ${SERVICE_LINK_LIBRARIES} ${QT_LINK_LIBRARIES} ${TEST_LINK_LIBRARIES} ${THREAD_LINK_LIBRARIES}) endfunction() +add_qt_test_by_name(greeter-test) add_qt_test_by_name(usb-snap-test) diff --git a/tests/unit/adbd-client-test.cpp b/tests/unit/adbd-client-test.cpp index 754f76c..d1ce740 100644 --- a/tests/unit/adbd-client-test.cpp +++ b/tests/unit/adbd-client-test.cpp @@ -73,7 +73,7 @@ TEST_F(AdbdClientFixture, SocketPlumbing) // start an AdbdClient that listens for PKRequests std::string pk; auto adbd_client = std::make_shared<GAdbdClient>(socket_path); - adbd_client->on_pk_request().connect([&pk, main_thread, test](const AdbdClient::PKRequest& req){ + auto connection = adbd_client->on_pk_request().connect([&pk, main_thread, test](const AdbdClient::PKRequest& req){ EXPECT_EQ(main_thread, g_thread_self()); g_message("in on_pk_request with %s", req.public_key.c_str()); pk = req.public_key; @@ -82,12 +82,13 @@ TEST_F(AdbdClientFixture, SocketPlumbing) // start a mock AdbdServer with to fire test key requests and wait for a response auto adbd_server = std::make_shared<GAdbdServer>(socket_path, std::vector<std::string>{test.request}); - wait_for([adbd_server](){return !adbd_server->m_responses.empty();}, 2000); + wait_for([adbd_server](){return !adbd_server->m_responses.empty();}, 5000); EXPECT_EQ(test.expected_pk, pk); ASSERT_EQ(1, adbd_server->m_responses.size()); EXPECT_EQ(test.expected_response, adbd_server->m_responses.front()); // cleanup + connection.disconnect(); adbd_client.reset(); adbd_server.reset(); g_unlink(socket_path.c_str()); diff --git a/tests/unit/greeter-test.cpp b/tests/unit/greeter-test.cpp new file mode 100644 index 0000000..bfa88e8 --- /dev/null +++ b/tests/unit/greeter-test.cpp @@ -0,0 +1,159 @@ +/* + * Copyright 2016 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 warranties of + * MERCHANTABILITY, SATISFACTORY QUALITY, 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/>. + * + * Authors: + * Charles Kerr <charles.kerr@canonical.com> + */ + +#include <tests/utils/qt-fixture.h> +#include <tests/utils/gtest-print-helpers.h> + +#include <src/dbus-names.h> +#include <src/greeter.h> + +#include <libqtdbustest/DBusTestRunner.h> +#include <libqtdbustest/QProcessDBusService.h> +#include <libqtdbusmock/DBusMock.h> + +class GreeterFixture: public QtFixture +{ +private: + + using super = QtFixture; + +public: + + GreeterFixture() =default; + ~GreeterFixture() =default; + +protected: + + std::shared_ptr<QtDBusTest::DBusTestRunner> m_dbus_runner; + std::shared_ptr<QtDBusMock::DBusMock> m_dbus_mock; + GDBusConnection* m_bus {}; + + void SetUp() override + { + super::SetUp(); + + // use a fresh bus for each test run + m_dbus_runner.reset(new QtDBusTest::DBusTestRunner()); + m_dbus_mock.reset(new QtDBusMock::DBusMock(*m_dbus_runner.get())); + + GError* error {}; + m_bus = g_bus_get_sync (G_BUS_TYPE_SESSION, nullptr, &error); + g_assert_no_error(error); + g_dbus_connection_set_exit_on_close(m_bus, FALSE); + } + + void TearDown() override + { + g_clear_object(&m_bus); + + super::TearDown(); + } + + void start_greeter_service(bool is_active) + { + // set a watcher to look for our mock greeter to appear + bool owned {}; + QDBusServiceWatcher watcher( + DBusNames::UnityGreeter::NAME, + m_dbus_runner->sessionConnection() + ); + QObject::connect( + &watcher, + &QDBusServiceWatcher::serviceRegistered, + [&owned](const QString&){owned = true;} + ); + + // start the mock greeter + QVariantMap parameters; + parameters["IsActive"] = QVariant(is_active); + m_dbus_mock->registerTemplate( + DBusNames::UnityGreeter::NAME, + GREETER_TEMPLATE, + parameters, + QDBusConnection::SessionBus + ); + m_dbus_runner->startServices(); + + // wait for the watcher + ASSERT_TRUE(wait_for([&owned]{return owned;})); + } +}; + +#define ASSERT_PROPERTY_EQ_EVENTUALLY(expected_in, property_in) \ + do { \ + const auto& e = expected_in; \ + const auto& p = property_in; \ + ASSERT_TRUE(wait_for([e, &p](){return e == p.get();})) \ + << "expected " << e << " but got " << p.get(); \ + } while(0) + +/** + * Test startup timing by looking at four different cases: + * [unity greeter shows up on bus (before, after) we start listening] + * x [unity greeter is (active, inactive)] + */ + +TEST_F(GreeterFixture, ActiveServiceStartsBeforeWatcher) +{ + constexpr bool is_active {true}; + constexpr Greeter::State expected {Greeter::State::ACTIVE}; + + start_greeter_service(is_active); + + UnityGreeter greeter; + + ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state()); +} + +TEST_F(GreeterFixture, WatcherStartsBeforeActiveService) +{ + constexpr bool is_active {true}; + constexpr Greeter::State expected {Greeter::State::ACTIVE}; + + UnityGreeter greeter; + + start_greeter_service(is_active); + + ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state()); +} + +TEST_F(GreeterFixture, InactiveServiceStartsBeforeWatcher) +{ + constexpr bool is_active {false}; + constexpr Greeter::State expected {Greeter::State::INACTIVE}; + + start_greeter_service(is_active); + + UnityGreeter greeter; + + ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state()); +} + +TEST_F(GreeterFixture, WatcherStartsBeforeInactiveService) +{ + constexpr bool is_active {false}; + constexpr Greeter::State expected {Greeter::State::INACTIVE}; + + UnityGreeter greeter; + + start_greeter_service(is_active); + + ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state()); +} + diff --git a/tests/unit/usb-snap-test.cpp b/tests/unit/usb-snap-test.cpp index 3b778dd..2be4b27 100644 --- a/tests/unit/usb-snap-test.cpp +++ b/tests/unit/usb-snap-test.cpp @@ -88,7 +88,7 @@ TEST_F(UsbSnapFixture, TestRoundTrip) auto snap = std::make_shared<UsbSnap>(test.fingerprint); AdbdClient::PKResponse user_response {}; bool user_response_set = false; - snap->on_user_response().connect([&user_response,&user_response_set](AdbdClient::PKResponse response, bool /*remember*/){ + auto connection = snap->on_user_response().connect([&user_response,&user_response_set](AdbdClient::PKResponse response, bool /*remember*/){ user_response = response; user_response_set = true; }); @@ -130,14 +130,9 @@ TEST_F(UsbSnapFixture, TestRoundTrip) EXPECT_TRUE(user_response_set); ASSERT_EQ(test.expected_response, user_response); - // confirm that the snap dtor cleans up the notification + // confirm that the snap dtor doesn't try to close + // the notification that's already been closed by user choice snap.reset(); - wait_for_signals(notificationsSpy, 1); - { - QVariantList const& call(notificationsSpy.at(0)); - EXPECT_EQ("CloseNotification", call.at(0)); - QVariantList const& args(call.at(1).toList()); - EXPECT_EQ(id, args.at(0)); - } + EXPECT_FALSE(notificationsSpy.wait(1000)); } } diff --git a/tests/utils/adbd-server.h b/tests/utils/adbd-server.h index b574622..585380f 100644 --- a/tests/utils/adbd-server.h +++ b/tests/utils/adbd-server.h @@ -38,7 +38,7 @@ public: GAdbdServer(const std::string& socket_path, const std::vector<std::string>& requests): m_requests{requests}, - m_socket_path{socket_path}, + m_server_socket{create_server_socket(socket_path)}, m_cancellable{g_cancellable_new()}, m_worker_thread{&GAdbdServer::worker_func, this} { @@ -50,6 +50,7 @@ public: g_cancellable_cancel(m_cancellable); m_worker_thread.join(); g_clear_object(&m_cancellable); + g_clear_object(&m_server_socket); } const std::vector<std::string> m_requests; @@ -59,18 +60,14 @@ private: void worker_func() // runs in worker thread { - auto server_socket = create_server_socket(m_socket_path); auto requests = m_requests; - GError* error {}; - g_socket_listen (server_socket, &error); - g_assert_no_error (error); - while (!g_cancellable_is_cancelled(m_cancellable) && !requests.empty()) { // wait for a client connection g_message("GAdbdServer::Impl::worker_func() calling g_socket_accept()"); - auto client_socket = g_socket_accept(server_socket, m_cancellable, &error); + GError* error {}; + auto client_socket = g_socket_accept(m_server_socket, m_cancellable, &error); if (error != nullptr) { if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) g_message("GAdbdServer: Error accepting socket connection: %s", error->message); @@ -121,8 +118,6 @@ private: // cleanup g_clear_object(&client_socket); } - - g_clear_object(&server_socket); } // bind to a local domain socket @@ -139,11 +134,14 @@ private: g_assert_no_error (error); g_clear_object (&address); + g_socket_listen (socket, &error); + g_assert_no_error (error); + return socket; } - const std::string m_socket_path; - GCancellable* m_cancellable = nullptr; + GSocket* m_server_socket {}; + GCancellable* m_cancellable {}; std::thread m_worker_thread; }; diff --git a/tests/utils/gtest-print-helpers.h b/tests/utils/gtest-print-helpers.h new file mode 100644 index 0000000..60f42b4 --- /dev/null +++ b/tests/utils/gtest-print-helpers.h @@ -0,0 +1,18 @@ + +#pragma once + +#include <src/greeter.h> + +inline void PrintTo(const Greeter::State& state, std::ostream* os) { + switch(state) { + case Greeter::State::ACTIVE: *os << "Active"; break; + case Greeter::State::INACTIVE: *os << "Inactive"; break; + case Greeter::State::UNAVAILABLE: *os << "Unavailable"; break; + } +} + +inline std::ostream& operator<<(std::ostream& os, const Greeter::State& state) { + PrintTo(state, &os); + return os; +} + diff --git a/tests/utils/mock-greeter.h b/tests/utils/mock-greeter.h index 5ac85a0..5015087 100644 --- a/tests/utils/mock-greeter.h +++ b/tests/utils/mock-greeter.h @@ -26,7 +26,7 @@ class MockGreeter: public Greeter public: MockGreeter() =default; virtual ~MockGreeter() =default; - core::Property<bool>& is_active() override {return m_is_active;} - core::Property<bool> m_is_active {false}; + core::Property<Greeter::State>& state() override {return m_state;} + core::Property<Greeter::State> m_state {State::INACTIVE}; }; diff --git a/tests/utils/mock-unity-greeter.py b/tests/utils/mock-unity-greeter.py new file mode 100644 index 0000000..70fb791 --- /dev/null +++ b/tests/utils/mock-unity-greeter.py @@ -0,0 +1,41 @@ +'''unity greeter mock template + +Very basic template that just mocks the greeter is-active flag +''' + +# This program is free software; you can redistribute it and/or modify it under +# the terms of the GNU Lesser General Public License as published by the Free +# Software Foundation; either version 3 of the License, or (at your option) any +# later version. See http://www.gnu.org/copyleft/lgpl.html for the full text +# of the license. + +__author__ = 'Charles Kerr' +__email__ = 'charles.kerr@canonical.com' +__copyright__ = '(c) 2016 Canonical Ltd.' +__license__ = 'LGPL 3+' + +import dbus +import os + +from dbusmock import MOCK_IFACE, mockobject + +BUS_NAME = 'com.canonical.UnityGreeter' +MAIN_OBJ = '/' +MAIN_IFACE = 'com.canonical.UnityGreeter' +SYSTEM_BUS = False + + +def load(mock, parameters): + mock.AddMethods( + MAIN_IFACE, [ + ('HideGreeter', '', '', 'self.Set("com.canonical.UnityGreeter", "IsActive", False)'), + ('ShowGreeter', '', '', 'self.Set("com.canonical.UnityGreeter", "IsActive", True)') + ] + ) + mock.AddProperties( + MAIN_IFACE, + dbus.Dictionary({ + 'IsActive': parameters.get('IsActive', False), + }, signature='sv') + ) + |