Merge lp:~xavi-garcia-mena/keeper/tests-with-propertieschanged-signal into lp:keeper/devel

Proposed by Xavi Garcia
Status: Merged
Merge reported by: Xavi Garcia
Merged at revision: not available
Proposed branch: lp:~xavi-garcia-mena/keeper/tests-with-propertieschanged-signal
Merge into: lp:keeper/devel
Diff against target: 819 lines (+550/-23)
14 files modified
src/service/keeper-task.cpp (+24/-1)
src/service/keeper-task.h (+2/-0)
src/service/private/keeper-task_p.h (+2/-0)
src/service/task-manager.cpp (+30/-16)
src/service/task-manager.h (+5/-0)
src/util/dbus-utils.cpp (+6/-1)
tests/CMakeLists.txt (+1/-0)
tests/integration/helpers/CMakeLists.txt (+3/-0)
tests/integration/helpers/helpers-test-failure.cpp (+16/-1)
tests/integration/helpers/helpers-test.cc (+14/-0)
tests/integration/helpers/test-helpers-base.cpp (+360/-3)
tests/integration/helpers/test-helpers-base.h (+7/-1)
tests/qdbus-stubs/CMakeLists.txt (+53/-0)
tests/qdbus-stubs/org.freedesktop.DBus.Properties.xml (+27/-0)
To merge this branch: bzr merge lp:~xavi-garcia-mena/keeper/tests-with-propertieschanged-signal
Reviewer Review Type Date Requested Status
unity-api-1-bot continuous-integration Needs Fixing
Charles Kerr (community) Approve
Review via email: mp+304928@code.launchpad.net

Commit message

Improve integration tests to listen to the PropertiesChanged signal from keeper.User dbus interface.

Description of the change

Improve integration tests to listen to the PropertiesChanged signal from keeper.User dbus interface.

The tests now check that the signals progress is correct. It records all the signals until completion of all tasks and checks that things like "action" state makes sense of "percent-done" is always greater or equal than the previous signal.

It also adds some code modifications to emit the PropertiesChanged signal as now the state lives in the TaskManager class.

To post a comment you must log in.
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) wrote :

Clean and straightforward. Overall, looks very good!

Several questions and minor/trivial suggestions inline, one borderline needs-fixing.

review: Needs Information
Revision history for this message
Xavi Garcia (xavi-garcia-mena) wrote :

Thanks for the review, Charles. I've added some comments inline.

When fixing the issues you found and suggested I realized that we were not sending a lot of PropertiesChanges signal at the beginning because when I did the TaskManager change I forgot to add them.

To fix this I've created a static method in the KeeperTask class that returns an initial state.

The idea would be to set that initial state for all tasks (which should appear as queued for the user) and then emit the PropertiesSignal to notify the state of all tasks all at once.

I also added a new change to detect when the previous state of a task is equal to the current one. In that case we don't emit the PropertiesChanged signal.

We have a big TODO here, though, and it's about i18n as stated in the code... I've created a Trello card for that.

Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Continuous integration, rev:101
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/40/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/561/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/567
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/395
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/395/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/395
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/395/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/395/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/395
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/395/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/395
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/395/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/395/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/395
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/395/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/395
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/395/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/395/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/40/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) wrote :

LGTM. Thanks for the changes!

review: Approve
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-autoland/5/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/568/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/574
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/402
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/402/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/402
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/402/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/402/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/402
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/402/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/402
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/402/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/402/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/402
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/402/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/402
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/402/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/402/console

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/service/keeper-task.cpp'
--- src/service/keeper-task.cpp 2016-08-31 14:40:50 +0000
+++ src/service/keeper-task.cpp 2016-09-06 13:08:57 +0000
@@ -114,7 +114,7 @@
114114
115void KeeperTaskPrivate::on_helper_percent_done_changed(float /*percent_done*/)115void KeeperTaskPrivate::on_helper_percent_done_changed(float /*percent_done*/)
116{116{
117 calculate_task_state();117 calculate_and_notify_state(helper_->state());
118}118}
119119
120QVariantMap KeeperTaskPrivate::calculate_task_state()120QVariantMap KeeperTaskPrivate::calculate_task_state()
@@ -149,6 +149,24 @@
149 Q_EMIT(q_ptr->task_state_changed(state));149 Q_EMIT(q_ptr->task_state_changed(state));
150}150}
151151
152QVariantMap KeeperTaskPrivate::get_initial_state(KeeperTask::TaskData const &td)
153{
154 QVariantMap ret;
155
156 auto const uuid = td.metadata.uuid();
157
158 ret.insert(QStringLiteral("action"), td.action);
159
160 // TODO review this when we add the restore tasks.
161 // TODO we maybe have different fields
162 ret.insert(QStringLiteral("display-name"), td.metadata.display_name());
163 ret.insert(QStringLiteral("speed"), 0);
164 ret.insert(QStringLiteral("percent-done"), double(0.0));
165 ret.insert(QStringLiteral("uuid"), uuid);
166
167 return ret;
168}
169
152KeeperTask::KeeperTask(TaskData const & task_data,170KeeperTask::KeeperTask(TaskData const & task_data,
153 QSharedPointer<HelperRegistry> const & helper_registry,171 QSharedPointer<HelperRegistry> const & helper_registry,
154 QSharedPointer<StorageFrameworkClient> const & storage,172 QSharedPointer<StorageFrameworkClient> const & storage,
@@ -178,3 +196,8 @@
178 Q_D(const KeeperTask);196 Q_D(const KeeperTask);
179 return d->state();197 return d->state();
180}198}
199
200QVariantMap KeeperTask::get_initial_state(KeeperTask::TaskData const &td)
201{
202 return KeeperTaskPrivate::get_initial_state(td);
203}
181204
=== modified file 'src/service/keeper-task.h'
--- src/service/keeper-task.h 2016-08-24 11:04:47 +0000
+++ src/service/keeper-task.h 2016-09-06 13:08:57 +0000
@@ -59,6 +59,8 @@
59 bool start();59 bool start();
60 QVariantMap state() const;60 QVariantMap state() const;
6161
62 static QVariantMap get_initial_state(KeeperTask::TaskData const &td);
63
62 Q_SIGNALS:64 Q_SIGNALS:
63 void task_state_changed(Helper::State state);65 void task_state_changed(Helper::State state);
64 void task_socket_ready(int socket_descriptor);66 void task_socket_ready(int socket_descriptor);
6567
=== modified file 'src/service/private/keeper-task_p.h'
--- src/service/private/keeper-task_p.h 2016-08-24 11:04:47 +0000
+++ src/service/private/keeper-task_p.h 2016-09-06 13:08:57 +0000
@@ -36,6 +36,8 @@
36 QVariantMap state() const;36 QVariantMap state() const;
37 void ask_for_storage_framework_socket(quint64 n_bytes);37 void ask_for_storage_framework_socket(quint64 n_bytes);
3838
39 static QVariantMap get_initial_state(KeeperTask::TaskData const &td);
40
39protected:41protected:
40 void set_current_task_action(QString const& action);42 void set_current_task_action(QString const& action);
41 void on_helper_percent_done_changed(float percent_done);43 void on_helper_percent_done_changed(float percent_done);
4244
=== modified file 'src/service/task-manager.cpp'
--- src/service/task-manager.cpp 2016-08-31 14:56:18 +0000
+++ src/service/task-manager.cpp 2016-09-06 13:08:57 +0000
@@ -79,8 +79,10 @@
79 td.type = type;79 td.type = type;
80 td.action = QStringLiteral("queued"); // TODO i18n80 td.action = QStringLiteral("queued"); // TODO i18n
8181
82// update_task_state(td);82 set_initial_task_state(td);
83 }83 }
84 // notify the initial state once for all tasks
85 notify_state_changed();
8486
85 start_next_task();87 start_next_task();
86 }88 }
@@ -222,20 +224,13 @@
222 update_task_state(it.value());224 update_task_state(it.value());
223 }225 }
224226
225 void update_task_state(KeeperTask::KeeperTask::TaskData& td)227 void set_initial_task_state(KeeperTask::KeeperTask::TaskData& td)
226 {228 {
227 state_[td.metadata.uuid()] = task_->state();229 state_[td.metadata.uuid()] = KeeperTask::get_initial_state(td);
228230 }
229#if 0231
230 // FIXME: we don't need this to work correctly for the sprint because Marcus is polling in a loop232 void notify_state_changed()
231 // but we will need this in order for him to stop doing that233 {
232
233 // TODO: compare old and new and decide if it's worth emitting a PropertyChanged signal;
234 // eg don't contribute to dbus noise for minor speed fluctuations
235
236 // TODO: this function is called inside a loop when initializing the state
237 // after start_tasks(), so also ensure we don't have a notify flood here
238
239 DBusUtils::notifyPropertyChanged(234 DBusUtils::notifyPropertyChanged(
240 QDBusConnection::sessionBus(),235 QDBusConnection::sessionBus(),
241 *q_ptr,236 *q_ptr,
@@ -243,7 +238,26 @@
243 DBusTypes::KEEPER_USER_INTERFACE,238 DBusTypes::KEEPER_USER_INTERFACE,
244 QStringList(QStringLiteral("State"))239 QStringList(QStringLiteral("State"))
245 );240 );
246#endif241
242 Q_EMIT(q_ptr->state_changed());
243 }
244
245 void update_task_state(KeeperTask::KeeperTask::TaskData& td)
246 {
247 auto task_state = task_->state();
248
249 // avoid sending repeated states to minimize the use of the bus
250 if (task_state != state_[td.metadata.uuid()] && !task_state.isEmpty())
251 {
252 state_[td.metadata.uuid()] = task_state;
253
254 // FIXME: we don't need this to work correctly for the sprint because Marcus is polling in a loop
255 // but we will need this in order for him to stop doing that
256
257 // TODO: compare old and new and decide if it's worth emitting a PropertyChanged signal;
258 // eg don't contribute to dbus noise for minor speed fluctuations
259 notify_state_changed();
260 }
247 }261 }
248262
249 void set_current_task_action(QString const& action)263 void set_current_task_action(QString const& action)
250264
=== modified file 'src/service/task-manager.h'
--- src/service/task-manager.h 2016-08-24 11:04:47 +0000
+++ src/service/task-manager.h 2016-09-06 13:08:57 +0000
@@ -46,6 +46,10 @@
4646
47 Q_DISABLE_COPY(TaskManager)47 Q_DISABLE_COPY(TaskManager)
4848
49 Q_PROPERTY(QVariantDictMap State
50 READ get_state
51 NOTIFY state_changed)
52
49 void start_tasks(QStringList const & task_uuids);53 void start_tasks(QStringList const & task_uuids);
5054
51 QVariantDictMap get_state() const;55 QVariantDictMap get_state() const;
@@ -54,6 +58,7 @@
5458
55Q_SIGNALS:59Q_SIGNALS:
56 void socket_ready(int reply);60 void socket_ready(int reply);
61 void state_changed();
5762
58private:63private:
59 QScopedPointer<TaskManagerPrivate> const d_ptr;64 QScopedPointer<TaskManagerPrivate> const d_ptr;
6065
=== modified file 'src/util/dbus-utils.cpp'
--- src/util/dbus-utils.cpp 2016-05-26 21:33:18 +0000
+++ src/util/dbus-utils.cpp 2016-09-06 13:08:57 +0000
@@ -65,7 +65,12 @@
65 signal << it.key().second;65 signal << it.key().second;
66 // Changed properties (name, value)66 // Changed properties (name, value)
67 signal << it.value().second;67 signal << it.value().second;
68 signal << QStringList();68 QStringList properties_changed;
69 for (auto iter_props = it.value().second.begin(); iter_props != it.value().second.end(); ++iter_props)
70 {
71 properties_changed << iter_props.key();
72 }
73 signal << properties_changed;
69 // Connection74 // Connection
70 it.value().first.send(signal);75 it.value().first.send(signal);
71 }76 }
7277
=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt 2016-08-31 14:40:50 +0000
+++ tests/CMakeLists.txt 2016-09-06 13:08:57 +0000
@@ -99,6 +99,7 @@
99add_subdirectory(utils)99add_subdirectory(utils)
100add_subdirectory(fakes)100add_subdirectory(fakes)
101add_subdirectory(integration)101add_subdirectory(integration)
102add_subdirectory(qdbus-stubs)
102103
103set(104set(
104 COVERAGE_TEST_TARGETS105 COVERAGE_TEST_TARGETS
105106
=== modified file 'tests/integration/helpers/CMakeLists.txt'
--- tests/integration/helpers/CMakeLists.txt 2016-08-30 12:50:46 +0000
+++ tests/integration/helpers/CMakeLists.txt 2016-09-06 13:08:57 +0000
@@ -23,6 +23,7 @@
23 ${UAL_INCLUDE_DIRS}23 ${UAL_INCLUDE_DIRS}
24 ${PROP-CPP_INCLUDE_DIRS}24 ${PROP-CPP_INCLUDE_DIRS}
25 ${HELPERS_TEST_DEPS_INCLUDE_DIRS}25 ${HELPERS_TEST_DEPS_INCLUDE_DIRS}
26 "${CMAKE_BINARY_DIR}/tests/qdbus-stubs"
26)27)
2728
28set(29set(
@@ -48,6 +49,7 @@
48 ${HELPERS_TEST}49 ${HELPERS_TEST}
49 ${HELPERS_TEST_DEPS_LDFLAGS}50 ${HELPERS_TEST_DEPS_LDFLAGS}
50 ${INTEGRATION_TEST_LIBRARIES}51 ${INTEGRATION_TEST_LIBRARIES}
52 qdbus-stubs-tests
51 Qt5::Core53 Qt5::Core
52 Qt5::DBus54 Qt5::DBus
53 Qt5::Test55 Qt5::Test
@@ -113,6 +115,7 @@
113 ${HELPERS_TEST_FAILURE}115 ${HELPERS_TEST_FAILURE}
114 ${HELPERS_TEST_DEPS_LDFLAGS}116 ${HELPERS_TEST_DEPS_LDFLAGS}
115 ${INTEGRATION_TEST_LIBRARIES}117 ${INTEGRATION_TEST_LIBRARIES}
118 qdbus-stubs-tests
116 Qt5::Core119 Qt5::Core
117 Qt5::DBus120 Qt5::DBus
118 Qt5::Test121 Qt5::Test
119122
=== modified file 'tests/integration/helpers/helpers-test-failure.cpp'
--- tests/integration/helpers/helpers-test-failure.cpp 2016-09-01 12:55:19 +0000
+++ tests/integration/helpers/helpers-test-failure.cpp 2016-09-06 13:08:57 +0000
@@ -67,11 +67,26 @@
67 QFile helper_mark(SIMPLE_HELPER_MARK_FILE_PATH);67 QFile helper_mark(SIMPLE_HELPER_MARK_FILE_PATH);
68 qDebug() << "Helper mark exists before calling StartBackup..." << helper_mark.exists();68 qDebug() << "Helper mark exists before calling StartBackup..." << helper_mark.exists();
6969
70 QSharedPointer<DBusPropertiesInterface> properties_interface(new DBusPropertiesInterface(
71 DBusTypes::KEEPER_SERVICE,
72 DBusTypes::KEEPER_USER_PATH,
73 dbus_test_runner.sessionConnection()
74 ) );
75
76 ASSERT_TRUE(properties_interface->isValid()) << qPrintable(QDBusConnection::sessionBus().lastError().message());
77
78 QSignalSpy spy(properties_interface.data(),&DBusPropertiesInterface::PropertiesChanged);
79
70 // Now we know the music folder uuid, let's start the backup for it.80 // Now we know the music folder uuid, let's start the backup for it.
71 QDBusReply<void> backup_reply = user_iface->call("StartBackup", QStringList{user_folder_uuid});81 QDBusReply<void> backup_reply = user_iface->call("StartBackup", QStringList{user_folder_uuid});
72 ASSERT_TRUE(backup_reply.isValid()) << qPrintable(dbus_test_runner.sessionConnection().lastError().message());82 ASSERT_TRUE(backup_reply.isValid()) << qPrintable(dbus_test_runner.sessionConnection().lastError().message());
7383
74 // wait until all the tasks have the action state "failed"84 // waits until all tasks are complete, recording PropertiesChanged signals
85 // and checks all the recorded values
86 EXPECT_TRUE(capture_and_check_state_until_all_tasks_complete(spy, {user_folder_uuid}, "failed"));
87
88 // wait until all the tasks have the action state "complete"
89 // this one uses pooling so it should just call Get once
75 EXPECT_TRUE(wait_for_all_tasks_have_action_state({user_folder_uuid}, "failed", user_iface));90 EXPECT_TRUE(wait_for_all_tasks_have_action_state({user_folder_uuid}, "failed", user_iface));
7691
77 // check that the content of the file is the expected92 // check that the content of the file is the expected
7893
=== modified file 'tests/integration/helpers/helpers-test.cc'
--- tests/integration/helpers/helpers-test.cc 2016-09-01 12:55:19 +0000
+++ tests/integration/helpers/helpers-test.cc 2016-09-06 13:08:57 +0000
@@ -104,12 +104,26 @@
104 ASSERT_FALSE(user_folder_uuid_2.isEmpty());104 ASSERT_FALSE(user_folder_uuid_2.isEmpty());
105 qDebug() << "User folder 2 UUID is:" << user_folder_uuid_2;105 qDebug() << "User folder 2 UUID is:" << user_folder_uuid_2;
106106
107 QSharedPointer<DBusPropertiesInterface> properties_interface(new DBusPropertiesInterface(
108 DBusTypes::KEEPER_SERVICE,
109 DBusTypes::KEEPER_USER_PATH,
110 dbus_test_runner.sessionConnection()
111 ) );
112
113 ASSERT_TRUE(properties_interface->isValid()) << qPrintable(QDBusConnection::sessionBus().lastError().message());
114
115 QSignalSpy spy(properties_interface.data(),&DBusPropertiesInterface::PropertiesChanged);
107116
108 // Now we know the music folder uuid, let's start the backup for it.117 // Now we know the music folder uuid, let's start the backup for it.
109 QDBusReply<void> backup_reply = user_iface->call("StartBackup", QStringList{user_folder_uuid, user_folder_uuid_2});118 QDBusReply<void> backup_reply = user_iface->call("StartBackup", QStringList{user_folder_uuid, user_folder_uuid_2});
110 ASSERT_TRUE(backup_reply.isValid()) << qPrintable(dbus_test_runner.sessionConnection().lastError().message());119 ASSERT_TRUE(backup_reply.isValid()) << qPrintable(dbus_test_runner.sessionConnection().lastError().message());
111120
121 // waits until all tasks are complete, recording PropertiesChanged signals
122 // and checks all the recorded values
123 EXPECT_TRUE(capture_and_check_state_until_all_tasks_complete(spy, {user_folder_uuid, user_folder_uuid_2}, "complete"));
124
112 // wait until all the tasks have the action state "complete"125 // wait until all the tasks have the action state "complete"
126 // this one uses pooling so it should just call Get once
113 EXPECT_TRUE(wait_for_all_tasks_have_action_state({user_folder_uuid, user_folder_uuid_2}, "complete", user_iface));127 EXPECT_TRUE(wait_for_all_tasks_have_action_state({user_folder_uuid, user_folder_uuid_2}, "complete", user_iface));
114128
115 // check that the content of the file is the expected129 // check that the content of the file is the expected
116130
=== modified file 'tests/integration/helpers/test-helpers-base.cpp'
--- tests/integration/helpers/test-helpers-base.cpp 2016-08-31 14:48:23 +0000
+++ tests/integration/helpers/test-helpers-base.cpp 2016-09-06 13:08:57 +0000
@@ -26,6 +26,242 @@
26using namespace QtDBusTest;26using namespace QtDBusTest;
27using namespace QtDBusMock;27using namespace QtDBusMock;
2828
29///
30/// State helpers
31//
32
33bool qvariant_to_map(QVariant const& variant, QVariantMap& map)
34{
35 if (variant.type() == QMetaType::QVariantMap)
36 {
37 map = variant.toMap();
38 return true;
39 }
40 qWarning() << Q_FUNC_INFO << ": Could not convert variant to QVariantMap. Variant received has type " << variant.typeName();
41
42 return false;
43}
44
45bool qdbus_argument_to_variant_dict_map(QVariant const& variant, QVariantDictMap& map)
46{
47 if (variant.canConvert<QDBusArgument>())
48 {
49 QDBusArgument value(variant.value<QDBusArgument>());
50 if (value.currentType() == QDBusArgument::MapType)
51 {
52 value >> map;
53 return true;
54 }
55 else
56 {
57 qWarning() << Q_FUNC_INFO << ": Could not convert variant to QVariantDictMap. Variant received has type " << value.currentType();
58 }
59 }
60 else
61 {
62 qWarning() << Q_FUNC_INFO << ": Could not convert variant to QDBusArgument.";
63 }
64 return false;
65}
66
67bool get_property_qvariant_dict_map(QString const & property, QVariant const &variant, QVariantDictMap & map)
68{
69 QVariantMap properties_map;
70 if (!qvariant_to_map(variant, properties_map))
71 {
72 qWarning() << Q_FUNC_INFO << ": Error converting variant in PropertiesChanged signal to QVariantMap";
73 return false;
74 }
75
76 auto iter = properties_map.find(property);
77 if (iter == properties_map.end())
78 {
79 qWarning() << Q_FUNC_INFO << ": Property [" << property << "] was not found in PropertiesChanged";
80 return false;
81 }
82
83 if(!qdbus_argument_to_variant_dict_map((*iter), map))
84 {
85 qWarning() << Q_FUNC_INFO << ": Error converting property [" << property << "] to QVariantDictMap";
86 return false;
87 }
88
89 return true;
90}
91
92bool all_tasks_has_state(QMap<QString, QString> const & tasks_state, QString const & state)
93{
94 for (auto iter = tasks_state.begin(); iter != tasks_state.end(); ++iter )
95 {
96 if ((*iter) != state)
97 {
98 return false;
99 }
100 }
101 return true;
102}
103
104bool get_task_property(QString const &property, QVariantMap const &values, QVariant &value)
105{
106 auto iter = values.find(property);
107 if (iter == values.end())
108 {
109 qWarning() << Q_FUNC_INFO << ": Property [" << property << "] was not found.";
110 return false;
111 }
112 value = (*iter);
113 return true;
114}
115
116bool analyze_task_percentage_values(QString const & uuid, QList<QVariantMap> const & recorded_values)
117{
118 double previous_percentage = -1.0;
119 for (auto iter = recorded_values.begin(); iter != recorded_values.end(); ++iter)
120 {
121 QVariant percentage;
122 if (!get_task_property("percent-done", (*iter), percentage))
123 {
124 qWarning() << Q_FUNC_INFO << ": Percentage was not found for task: " << uuid;
125 return false;
126 }
127 bool ok_double;
128 auto percentage_double = percentage.toDouble(&ok_double);
129 if (!ok_double)
130 {
131 qWarning() << Q_FUNC_INFO << ": Error converting percent-done to double for uuid: " << uuid << ". State: " << (*iter);
132 return false;
133 }
134 if (percentage_double < previous_percentage)
135 {
136 qWarning() << Q_FUNC_INFO << ": Eror, current percentage is less than previous: current=" << percentage_double << ", previous=" << previous_percentage;
137 return false;
138 }
139 previous_percentage = percentage_double;
140 }
141 return true;
142}
143
144bool check_valid_action_state_step(QString const &previous, QString const &current)
145{
146 if (current == "queued")
147 {
148 return previous == "none" || previous == "queued";
149 }
150 else if (current == "saving")
151 {
152 return previous == "saving" || previous == "queued";
153 }
154 else if (current == "finishing")
155 {
156 return previous == "finishing" || previous == "saving";
157 }
158 else if (current == "complete")
159 {
160 // we may pass from "saving" to "complete" if we don't have enough time
161 // to emit the "finishing" state change
162 return previous == "complete" || previous == "finishing" || previous == "saving";
163 }
164 else if (current == "failed")
165 {
166 // we can get to failed from any state except complete
167 return previous != "complete";
168 }
169 else
170 {
171 // for possible new states, please add your code here
172 qWarning() << "Unhandled state: " << current;
173 return false;
174 }
175}
176
177bool analyze_task_action_values(QString const & uuid, QList<QVariantMap> const & recorded_values)
178{
179 QString previous_action = "none";
180 for (auto iter = recorded_values.begin(); iter != recorded_values.end(); ++iter)
181 {
182 QVariant action;
183 if (!get_task_property("action", (*iter), action))
184 {
185 qWarning() << Q_FUNC_INFO << ": Action was not found for task: " << uuid;
186 return false;
187 }
188 auto current_action = action.toString();
189
190 if (!check_valid_action_state_step(previous_action, action.toString()))
191 {
192 qWarning() << Q_FUNC_INFO << ": Bad action state step: previous state=" << previous_action << ", current=" << action.toString();
193 return false;
194 }
195 previous_action = action.toString();
196 }
197 return true;
198}
199
200bool analyze_task_display_name_values(QString const & uuid, QList<QVariantMap> const & recorded_values)
201{
202 QString previous_name;
203 // check that the display name never changes between recorded states
204 for (auto iter = recorded_values.begin(); iter != recorded_values.end(); ++iter)
205 {
206 QVariant name;
207 if (!get_task_property("display-name", (*iter), name))
208 {
209 qWarning() << Q_FUNC_INFO << ": display-name was not found for task: " << uuid;
210 return false;
211 }
212 auto current_name = name.toString();
213
214 if (previous_name.isEmpty())
215 {
216 previous_name = name.toString();
217 }
218 else if(name.toString() != previous_name)
219 {
220 qWarning() << Q_FUNC_INFO << "ERROR: display-name for uuid: " << uuid << " changed from " << previous_name << " to " << name.toString();
221 return false;
222 }
223 }
224 return true;
225}
226
227bool analyze_tasks_values(QMap<QString, QList<QVariantMap>> const &uuids_state)
228{
229 for (auto iter = uuids_state.begin(); iter != uuids_state.end(); ++iter)
230 {
231 if(!analyze_task_display_name_values(iter.key(), (*iter)))
232 return false;
233 if(!analyze_task_action_values(iter.key(), (*iter)))
234 return false;
235 if(!analyze_task_percentage_values(iter.key(), (*iter)))
236 return false;
237 }
238 return true;
239}
240
241bool verify_signal_interface_and_invalidated_properties(QVariant const &interface,
242 QVariant const &invalidated_properties,
243 QString const & expected_interface,
244 QString const & expected_property)
245{
246 auto props_list = invalidated_properties.toStringList();
247 if (!props_list.contains(expected_property))
248 {
249 qWarning() << Q_FUNC_INFO << "ERROR: PropertiesChanged signal did not include the property: " << expected_property << " as the ones invalidated";
250 return false;
251 }
252
253 if (interface.toString() != expected_interface)
254 {
255 qWarning() << Q_FUNC_INFO << "ERROR: Interface: [" << interface.toString() << "] is not valid. Expecting: [" << expected_interface << "]";
256 return false;
257 }
258 return true;
259}
260
261///
262///
263///
264
29TestHelpersBase::TestHelpersBase()265TestHelpersBase::TestHelpersBase()
30 :dbus_mock(dbus_test_runner)266 :dbus_mock(dbus_test_runner)
31{267{
@@ -63,7 +299,13 @@
63 throw;299 throw;
64 }300 }
65301
66 system("dbus-send --session --dest=org.freedesktop.DBus --type=method_call --print-reply /org/freedesktop/DBus org.freedesktop.DBus.ListNames");302#if 0
303 // Enable this to get extra dbus information.
304 if (!start_dbus_monitor())
305 {
306 throw std::logic_error("Error starting dbus-monitor process.");
307 }
308#endif
67}309}
68310
69void TestHelpersBase::SetUp()311void TestHelpersBase::SetUp()
@@ -243,12 +485,12 @@
243 : -1;485 : -1;
244}486}
245487
246bool TestHelpersBase::wait_for_all_tasks_have_action_state(QStringList const & uuids, QString const & action_state, QSharedPointer<DBusInterfaceKeeperUser> const & keeper_user_iface, int max_timeout)488bool TestHelpersBase::wait_for_all_tasks_have_action_state(QStringList const & uuids, QString const & action_state, QSharedPointer<DBusInterfaceKeeperUser> const & keeper_user_iface, int max_timeout_msec)
247{489{
248 QElapsedTimer timer;490 QElapsedTimer timer;
249 timer.start();491 timer.start();
250 bool finished = false;492 bool finished = false;
251 while (!timer.hasExpired(max_timeout) && !finished)493 while (!timer.hasExpired(max_timeout_msec) && !finished)
252 {494 {
253 auto state = keeper_user_iface->state();495 auto state = keeper_user_iface->state();
254 auto all_helpers_finished = true;496 auto all_helpers_finished = true;
@@ -278,6 +520,98 @@
278 return (*iter_props).toString() == action_state;520 return (*iter_props).toString() == action_state;
279}521}
280522
523bool TestHelpersBase::capture_and_check_state_until_all_tasks_complete(QSignalSpy & spy, QStringList const & uuids, QString const & action_state, int max_timeout_msec)
524{
525 QMap<QString, QList<QVariantMap>> uuids_state;
526 QMap<QString, QString> uuids_current_state;
527
528 // initialize the current state map to wait for all expected uuids
529 for (auto const& uuid : uuids)
530 {
531 uuids_current_state[uuid] = "none";
532 uuids_state[uuid] = QList<QVariantMap>();
533 }
534
535 QElapsedTimer timer;
536 timer.start();
537 bool finished = false;
538 while (!timer.hasExpired(max_timeout_msec) && !finished)
539 {
540 spy.wait();
541
542 qDebug() << "PropertiesChanged SIGNALS RECEIVED: " << spy.count();
543 while (spy.count())
544 {
545 auto arguments = spy.takeFirst();
546
547 if (arguments.size() != 3)
548 {
549 qWarning() << "Bad number of arguments in PropertiesChanged signal";
550 return false;
551 }
552
553 // verify interface and invalidated_properties arguments
554 if(!verify_signal_interface_and_invalidated_properties(arguments.at(0), arguments.at(2), DBusTypes::KEEPER_USER_INTERFACE, "State"))
555 {
556 return false;
557 }
558 QVariantDictMap keeper_state;
559 if (!get_property_qvariant_dict_map("State", arguments.at(1), keeper_state))
560 {
561 return false;
562 }
563 for (auto iter = keeper_state.begin(); iter != keeper_state.end(); ++iter )
564 {
565 // check for unexpected uuids
566 if (!uuids.contains(iter.key()))
567 {
568 qWarning() << "State contains unexpected uuid: " << iter.key();
569 return false;
570 }
571
572 QVariantMap task_state;
573 if (!qvariant_to_map((*iter), task_state))
574 {
575 qWarning() << "Error converting second argument in PropertiesChanged signal to QVariantMap for uuid: " << iter.key();
576 return false;
577 }
578 qDebug() << "State for uuid: " << iter.key() << " : " << task_state;
579
580 QVariant action;
581 if (get_task_property("action", task_state, action))
582 {
583 if (action.type() != QVariant::String)
584 {
585 qWarning() << "Property [action] is not a string";
586 return false;
587 }
588 // store the current action state
589 uuids_current_state[iter.key()] = action.toString();
590
591 bool store = true;
592 if (action.toString() == action_state)
593 {
594 // check if it worths storing this new event
595 store = uuids_state[iter.key()].last() != task_state;
596 }
597 if (store)
598 {
599 // store the current state for later inspection
600 uuids_state[iter.key()].push_back(task_state);
601 }
602 }
603 }
604 }
605 finished = all_tasks_has_state(uuids_current_state, action_state);
606 if (finished)
607 {
608 qDebug() << "ALL TASKS FINISHED =========================================";
609 }
610 }
611 // check for the recorded values
612 return analyze_tasks_values(uuids_state);
613}
614
281QString TestHelpersBase::get_uuid_for_xdg_folder_path(QString const &path, QVariantDictMap const & choices) const615QString TestHelpersBase::get_uuid_for_xdg_folder_path(QString const &path, QVariantDictMap const & choices) const
282{616{
283 for(auto iter = choices.begin(); iter != choices.end(); ++iter)617 for(auto iter = choices.begin(); iter != choices.end(); ++iter)
@@ -296,3 +630,26 @@
296630
297 return QString();631 return QString();
298}632}
633
634bool TestHelpersBase::start_dbus_monitor()
635{
636 if (!dbus_monitor_process)
637 {
638 dbus_monitor_process.reset(new QProcess());
639
640 dbus_monitor_process->setProcessChannelMode(QProcess::ForwardedChannels);
641
642 dbus_monitor_process->start("dbus-monitor", QStringList() << "--session");
643 if (!dbus_monitor_process->waitForStarted())
644 {
645 qWarning() << "Error, starting dbus-monitor: " << dbus_monitor_process->errorString();
646 return false;
647 }
648 }
649 else
650 {
651 qWarning() << "Error, dbus-monitor should be called one time per test.";
652 return false;
653 }
654 return true;
655}
299656
=== modified file 'tests/integration/helpers/test-helpers-base.h'
--- tests/integration/helpers/test-helpers-base.h 2016-08-30 12:50:46 +0000
+++ tests/integration/helpers/test-helpers-base.h 2016-09-06 13:08:57 +0000
@@ -22,6 +22,7 @@
2222
23#include <helper/backup-helper.h>23#include <helper/backup-helper.h>
24#include <qdbus-stubs/dbus-types.h>24#include <qdbus-stubs/dbus-types.h>
25#include "DBusPropertiesInterface.h"
25#include "qdbus-stubs/keeper_user_interface.h"26#include "qdbus-stubs/keeper_user_interface.h"
2627
27#include "tests/fakes/fake-backup-helper.h"28#include "tests/fakes/fake-backup-helper.h"
@@ -73,6 +74,7 @@
73 QtDBusMock::DBusMock dbus_mock;74 QtDBusMock::DBusMock dbus_mock;
74 QSharedPointer<QtDBusTest::QProcessDBusService> keeper_service;75 QSharedPointer<QtDBusTest::QProcessDBusService> keeper_service;
75 QSharedPointer<QtDBusTest::QProcessDBusService> upstart_service;76 QSharedPointer<QtDBusTest::QProcessDBusService> upstart_service;
77 QScopedPointer<QProcess> dbus_monitor_process;
7678
77protected:79protected:
78 void start_tasks();80 void start_tasks();
@@ -93,11 +95,15 @@
9395
94 QString get_last_storage_framework_file();96 QString get_last_storage_framework_file();
9597
96 bool wait_for_all_tasks_have_action_state(QStringList const & uuids, QString const & action_state, QSharedPointer<DBusInterfaceKeeperUser> const & keeper_user_iface, int max_timeout = 15000);98 bool wait_for_all_tasks_have_action_state(QStringList const & uuids, QString const & action_state, QSharedPointer<DBusInterfaceKeeperUser> const & keeper_user_iface, int max_timeout_msec = 15000);
9799
98 bool check_task_has_action_state(QVariantDictMap const & state, QString const & uuid, QString const & action_state);100 bool check_task_has_action_state(QVariantDictMap const & state, QString const & uuid, QString const & action_state);
99101
102 bool capture_and_check_state_until_all_tasks_complete(QSignalSpy & spy, QStringList const & uuids, QString const & action_state, int max_timeout_msec = 15000);
103
100 QString get_uuid_for_xdg_folder_path(QString const &path, QVariantDictMap const & choices) const;104 QString get_uuid_for_xdg_folder_path(QString const &path, QVariantDictMap const & choices) const;
105
106 bool start_dbus_monitor();
101};107};
102108
103#define EXPECT_ENV(expected, envvars, key) EXPECT_EQ(expected, get_env(envvars, key)) << "for key " << key109#define EXPECT_ENV(expected, envvars, key) EXPECT_EQ(expected, get_env(envvars, key)) << "for key " << key
104110
=== added directory 'tests/qdbus-stubs'
=== added file 'tests/qdbus-stubs/CMakeLists.txt'
--- tests/qdbus-stubs/CMakeLists.txt 1970-01-01 00:00:00 +0000
+++ tests/qdbus-stubs/CMakeLists.txt 2016-09-06 13:08:57 +0000
@@ -0,0 +1,53 @@
1# Copyright © 2015 Canonical Ltd.
2#
3# This program is free software: you can redistribute it and/or modify it
4# under the terms of the GNU Lesser General Public License version 3,
5# as published by the Free Software Foundation.
6#
7# This program is distributed in the hope that it will be useful,
8# but WITHOUT ANY WARRANTY; without even the implied warranty of
9# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
10# GNU Lesser General Public License for more details.
11#
12# You should have received a copy of the GNU Lesser General Public License
13# along with this program. If not, see <http://www.gnu.org/licenses/>.
14#
15# Authors:
16# Xavi Garcia <marcus.tomlinson@canonical.com>
17
18# disable compilier warnings in machine-generated code
19add_compile_options(-w)
20
21# XML
22
23set(
24 properties_xml
25 "org.freedesktop.DBus.Properties.xml"
26)
27
28set_source_files_properties(
29 "${properties_xml}"
30 PROPERTIES
31 NO_NAMESPACE YES
32 CLASSNAME DBusPropertiesInterface
33)
34
35qt5_add_dbus_interface(
36 interface_files
37 ${properties_xml}
38 DBusPropertiesInterface
39)
40
41#
42#
43
44set(
45 STUBS_LIB
46 qdbus-stubs-tests
47)
48
49add_library(
50 ${STUBS_LIB}
51 STATIC
52 ${interface_files}
53)
054
=== added file 'tests/qdbus-stubs/org.freedesktop.DBus.Properties.xml'
--- tests/qdbus-stubs/org.freedesktop.DBus.Properties.xml 1970-01-01 00:00:00 +0000
+++ tests/qdbus-stubs/org.freedesktop.DBus.Properties.xml 2016-09-06 13:08:57 +0000
@@ -0,0 +1,27 @@
1<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN"
2 "http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd">
3<node>
4 <interface name="org.freedesktop.DBus.Properties">
5 <method name="Get">
6 <arg type="s" name="interface_name" direction="in"/>
7 <arg type="s" name="property_name" direction="in"/>
8 <arg type="v" name="value" direction="out"/>
9 </method>
10 <method name="GetAll">
11 <arg type="s" name="interface_name" direction="in"/>
12 <arg type="a{sv}" name="properties" direction="out"/>
13 <annotation name="org.qtproject.QtDBus.QtTypeName.Out0" value="QVariantMap"/>
14 </method>
15 <method name="Set">
16 <arg type="s" name="interface_name" direction="in"/>
17 <arg type="s" name="property_name" direction="in"/>
18 <arg type="v" name="value" direction="in"/>
19 </method>
20 <signal name="PropertiesChanged">
21 <arg type="s" name="interface_name"/>
22 <arg type="a{sv}" name="changed_properties"/>
23 <annotation name="org.qtproject.QtDBus.QtTypeName.In1" value="QVariantMap"/>
24 <arg type="as" name="invalidated_properties"/>
25 </signal>
26 </interface>
27</node>

Subscribers

People subscribed via source and target branches

to all changes: