Merge lp:~mardy/online-accounts-api/timeout-1603706 into lp:online-accounts-api

Proposed by Alberto Mardegan
Status: Merged
Approved by: James Henstridge
Approved revision: 24
Merged at revision: 24
Proposed branch: lp:~mardy/online-accounts-api/timeout-1603706
Merge into: lp:online-accounts-api
Diff against target: 255 lines (+68/-44)
3 files modified
tests/daemon/functional_tests/daemon_interface.cpp (+2/-2)
tests/daemon/functional_tests/daemon_interface.h (+1/-1)
tests/daemon/functional_tests/functional_tests.cpp (+65/-41)
To merge this branch: bzr merge lp:~mardy/online-accounts-api/timeout-1603706
Reviewer Review Type Date Requested Status
James Henstridge Approve
Review via email: mp+300416@code.launchpad.net

Commit message

Tests: keep the service alive while running the test

On slower machines, the D-Bus service can timeout during the execution of the testAccountChanges() test, causing it to fail.
With this change each test is run in its own D-Bus session, with a 30 seconds timeout, and only the testLifetime() has a shorter timeout, needed to test the auto-quitting functionality.

Description of the change

Tests: keep the service alive while running the test

On slower machines, the D-Bus service can timeout during the execution of the testAccountChanges() test, causing it to fail.
With this change each test is run in its own D-Bus session, with a 30 seconds timeout, and only the testLifetime() has a shorter timeout, needed to test the auto-quitting functionality.

To post a comment you must log in.
Revision history for this message
James Henstridge (jamesh) wrote :

There were two failed builds with this MP added to the silo. I've left details about them in the attached bug report together with links to the build logs:

https://bugs.launchpad.net/ubuntu/+source/online-accounts-api/+bug/1603706/comments/3

23. By Alberto Mardegan

Revert

24. By Alberto Mardegan

Tests: restart the D-Bus session with each test

Revision history for this message
James Henstridge (jamesh) wrote :

Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/daemon/functional_tests/daemon_interface.cpp'
2--- tests/daemon/functional_tests/daemon_interface.cpp 2015-07-01 13:49:00 +0000
3+++ tests/daemon/functional_tests/daemon_interface.cpp 2016-07-20 13:07:33 +0000
4@@ -38,11 +38,11 @@
5 }
6
7
8-DaemonInterface::DaemonInterface(QObject *parent):
9+DaemonInterface::DaemonInterface(const QDBusConnection &connection, QObject *parent):
10 QDBusAbstractInterface(ONLINE_ACCOUNTS_MANAGER_SERVICE_NAME,
11 ONLINE_ACCOUNTS_MANAGER_PATH,
12 ONLINE_ACCOUNTS_MANAGER_INTERFACE,
13- QDBusConnection::sessionBus(),
14+ connection,
15 parent)
16 {
17 setTimeout(INT_MAX);
18
19=== modified file 'tests/daemon/functional_tests/daemon_interface.h'
20--- tests/daemon/functional_tests/daemon_interface.h 2015-08-28 14:25:23 +0000
21+++ tests/daemon/functional_tests/daemon_interface.h 2016-07-20 13:07:33 +0000
22@@ -60,7 +60,7 @@
23 Q_OBJECT
24
25 public:
26- DaemonInterface(QObject *parent = 0);
27+ DaemonInterface(const QDBusConnection &connection, QObject *parent = 0);
28 ~DaemonInterface() {}
29
30 QDBusPendingCall getAccounts(const QVariantMap &filters) {
31
32=== modified file 'tests/daemon/functional_tests/functional_tests.cpp'
33--- tests/daemon/functional_tests/functional_tests.cpp 2015-11-03 14:44:56 +0000
34+++ tests/daemon/functional_tests/functional_tests.cpp 2016-07-20 13:07:33 +0000
35@@ -131,6 +131,31 @@
36 bool m_replyExpected;
37 };
38
39+class DBusService: public QtDBusTest::DBusTestRunner
40+{
41+public:
42+ DBusService();
43+
44+ FakeDBusApparmor &dbusApparmor() { return m_dbusApparmor; }
45+ FakeOnlineAccountsService &onlineAccounts() { return m_onlineAccounts; }
46+ FakeSignond &signond() { return m_signond; }
47+
48+private:
49+ QtDBusMock::DBusMock m_mock;
50+ FakeDBusApparmor m_dbusApparmor;
51+ FakeOnlineAccountsService m_onlineAccounts;
52+ FakeSignond m_signond;
53+};
54+
55+DBusService::DBusService():
56+ QtDBusTest::DBusTestRunner(TEST_DBUS_CONFIG_FILE),
57+ m_mock(*this),
58+ m_dbusApparmor(&m_mock),
59+ m_onlineAccounts(&m_mock),
60+ m_signond(&m_mock)
61+{
62+}
63+
64 class FunctionalTests: public QObject
65 {
66 Q_OBJECT
67@@ -143,6 +168,8 @@
68 FunctionalTests();
69
70 private Q_SLOTS:
71+ void init();
72+ void cleanup();
73 void testGetAccountsFiltering_data();
74 void testGetAccountsFiltering();
75 void testAuthenticate_data();
76@@ -151,18 +178,13 @@
77 void testRequestAccess();
78 void testAccountChanges();
79 void testLifetime();
80- void cleanupTestCase();
81
82 private:
83 void clearDb();
84
85 private:
86 EnvSetup m_env;
87- QtDBusTest::DBusTestRunner m_dbus;
88- QtDBusMock::DBusMock m_mock;
89- FakeDBusApparmor m_dbusApparmor;
90- FakeOnlineAccountsService m_onlineAccounts;
91- FakeSignond m_signond;
92+ DBusService *m_dbus;
93 int m_firstAccountId;
94 int m_account3CredentialsId;
95 };
96@@ -177,23 +199,19 @@
97
98 qputenv("SSO_USE_PEER_BUS", "0");
99
100- qputenv("OAD_TIMEOUT", "2");
101+ qputenv("OAD_TIMEOUT", "30");
102 qputenv("OAD_TESTING", "1");
103 }
104
105 FunctionalTests::FunctionalTests():
106 QObject(),
107- m_dbus(TEST_DBUS_CONFIG_FILE),
108- m_mock(m_dbus),
109- m_dbusApparmor(&m_mock),
110- m_onlineAccounts(&m_mock),
111- m_signond(&m_mock),
112+ m_dbus(0),
113 m_account3CredentialsId(35)
114 {
115 clearDb();
116
117 /* Populate the accounts DB */
118- Accounts::Manager *manager = new Accounts::Manager(this);
119+ Accounts::Manager *manager = new Accounts::Manager(Accounts::Manager::DisableNotifications, this);
120 Accounts::Service coolMail = manager->service("coolmail");
121 Accounts::Service coolShare = manager->service("com.ubuntu.tests_coolshare");
122 Accounts::Service oauth1auth = manager->service("oauth1auth");
123@@ -232,8 +250,6 @@
124 account3->syncAndBlock();
125
126 delete manager;
127-
128- m_dbus.startServices();
129 }
130
131 void FunctionalTests::clearDb()
132@@ -242,6 +258,17 @@
133 dbroot.remove("accounts.db");
134 }
135
136+void FunctionalTests::init()
137+{
138+ m_dbus = new DBusService();
139+ m_dbus->startServices();
140+}
141+
142+void FunctionalTests::cleanup()
143+{
144+ delete m_dbus;
145+}
146+
147 void FunctionalTests::testGetAccountsFiltering_data()
148 {
149 QTest::addColumn<QVariantMap>("filters");
150@@ -272,10 +299,10 @@
151 QFETCH(QString, securityContext);
152 QFETCH(QList<int>, expectedAccountIds);
153
154- DaemonInterface *daemon = new DaemonInterface;
155+ DaemonInterface *daemon = new DaemonInterface(m_dbus->sessionConnection());
156
157 TestProcess testProcess;
158- m_dbusApparmor.addClient(testProcess.uniqueName(), securityContext);
159+ m_dbus->dbusApparmor().addClient(testProcess.uniqueName(), securityContext);
160
161 QList<AccountInfo> accountInfos = testProcess.getAccounts(filters);
162 QList<int> accountIds;
163@@ -386,9 +413,9 @@
164 QFETCH(QVariantMap, expectedCredentials);
165 QFETCH(QString, errorName);
166
167- m_signond.addIdentity(m_account3CredentialsId, QVariantMap());
168+ m_dbus->signond().addIdentity(m_account3CredentialsId, QVariantMap());
169
170- DaemonInterface *daemon = new DaemonInterface;
171+ DaemonInterface *daemon = new DaemonInterface(m_dbus->sessionConnection());
172
173 QDBusPendingReply<QVariantMap> reply =
174 daemon->authenticate(m_firstAccountId + accountId, serviceId,
175@@ -475,10 +502,10 @@
176 QFETCH(QVariantMap, expectedCredentials);
177 QFETCH(QString, errorName);
178
179- m_onlineAccounts.setRequestAccessReply(accessReply);
180- m_signond.addIdentity(m_account3CredentialsId, QVariantMap());
181+ m_dbus->onlineAccounts().setRequestAccessReply(accessReply);
182+ m_dbus->signond().addIdentity(m_account3CredentialsId, QVariantMap());
183
184- DaemonInterface *daemon = new DaemonInterface;
185+ DaemonInterface *daemon = new DaemonInterface(m_dbus->sessionConnection());
186
187 QDBusPendingReply<AccountInfo,QVariantMap> reply =
188 daemon->requestAccess(serviceId, authParams);
189@@ -501,7 +528,7 @@
190
191 void FunctionalTests::testAccountChanges()
192 {
193- DaemonInterface *daemon = new DaemonInterface;
194+ DaemonInterface *daemon = new DaemonInterface(m_dbus->sessionConnection());
195
196 /* First, we make a call to the service so that it knows about our client
197 * and will later notify it about changes.
198@@ -576,19 +603,29 @@
199 expectedAccountInfo["serviceId"] = "com.ubuntu.tests_coolshare";
200 QCOMPARE(accountInfo.data(), expectedAccountInfo);
201
202+ delete manager;
203 delete daemon;
204 }
205
206 void FunctionalTests::testLifetime()
207 {
208+ /* Destroy the D-Bus daemon, and create one with the OAD_TIMEOUT variable
209+ * set to a lower value, to make this test meaningful */
210+ delete m_dbus;
211+
212+ qputenv("OAD_TIMEOUT", "2");
213+
214+ m_dbus = new DBusService();
215+ m_dbus->startServices();
216+
217 /* Make a dbus call, and have signond reply after 3 seconds; make sure that
218 * the online accounts daemon doesn't time out. */
219- m_signond.addIdentity(m_account3CredentialsId, QVariantMap());
220+ m_dbus->signond().addIdentity(m_account3CredentialsId, QVariantMap());
221
222- DaemonInterface *daemon = new DaemonInterface;
223+ DaemonInterface *daemon = new DaemonInterface(m_dbus->sessionConnection());
224
225 QDBusServiceWatcher watcher(ONLINE_ACCOUNTS_MANAGER_SERVICE_NAME,
226- QDBusConnection::sessionBus(),
227+ m_dbus->sessionConnection(),
228 QDBusServiceWatcher::WatchForUnregistration);
229 QSignalSpy unregistered(&watcher,
230 SIGNAL(serviceUnregistered(const QString &)));
231@@ -611,22 +648,9 @@
232 QCOMPARE(unregistered.count(), 0);
233
234 delete daemon;
235-}
236
237-void FunctionalTests::cleanupTestCase()
238-{
239- QDBusConnection conn = QDBusConnection::sessionBus();
240- QString serviceName(QStringLiteral(ONLINE_ACCOUNTS_MANAGER_SERVICE_NAME));
241- QDBusReply<bool> reply =
242- conn.interface()->isServiceRegistered(serviceName);
243- if (reply.value()) {
244- /* Let'äs wait for the daemon to quit */
245- QDBusServiceWatcher watcher(serviceName, conn,
246- QDBusServiceWatcher::WatchForUnregistration);
247- QSignalSpy unregistered(&watcher,
248- SIGNAL(serviceUnregistered(const QString &)));
249- QTRY_COMPARE(unregistered.count(), 1);
250- }
251+ /* We expect the OA service to exit within a couple of seconds */
252+ unregistered.wait();
253 }
254
255 QTEST_MAIN(FunctionalTests)

Subscribers

People subscribed via source and target branches