Merge lp:~mikemc/unity-scope-click/dlmgr-tests-again-again into lp:unity-scope-click

Proposed by Mike McCracken
Status: Merged
Approved by: Mike McCracken
Approved revision: 137
Merged at revision: 127
Proposed branch: lp:~mikemc/unity-scope-click/dlmgr-tests-again-again
Merge into: lp:unity-scope-click
Diff against target: 684 lines (+322/-179)
12 files modified
scope/click/CMakeLists.txt (+2/-1)
scope/click/download-manager.cpp (+12/-11)
scope/click/download-manager.h (+3/-1)
scope/click/ubuntuone_credentials.cpp (+58/-0)
scope/click/ubuntuone_credentials.h (+65/-0)
scope/tests/CMakeLists.txt (+3/-1)
scope/tests/download_manager_tool/download_manager_tool.cpp (+7/-5)
scope/tests/download_manager_tool/download_manager_tool.h (+3/-2)
scope/tests/main.cpp (+0/-36)
scope/tests/mock_ubuntuone_credentials.h (+38/-0)
scope/tests/test_download_manager.cpp (+131/-44)
scope/tests/test_download_manager.h (+0/-78)
To merge this branch: bzr merge lp:~mikemc/unity-scope-click/dlmgr-tests-again-again
Reviewer Review Type Date Requested Status
Thomas Voß (community) Approve
PS Jenkins bot continuous-integration Approve
Manuel de la Peña (community) Approve
Review via email: mp+204880@code.launchpad.net

Commit message

- rewrite tests for download manager interface to use Google test and mock.

Description of the change

- rewrite download manager tests to use Google test and mock.
- includes a wrapper interface for SSOService to allow us to mock it.
- adds to existing mock NAM
- reenable building of download_manager_tool

Only adds one test for download manager. Proposing at this point in the interest of shorter diffs.

Run with 'make test'.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Manuel de la Peña (mandel) wrote :

explicit is onyl useful when we have a constructor that has an argument that can be castted, there is no need to use it here:

217 + explicit CredentialsService();

The following is probably a matter of conventions and personal stlye, but are we not using some kind of convention to indicate that we are using instance members, for example:

231 +private:
232 + QScopedPointer<UbuntuOne::SSOService> ssoService;

I would expect that to me m_ssoSrvice or _ssoService. It just gives context in the methods when working with the vars.

In the following connections:

69 + QMetaObject::Connection c = connect(impl->credentialsService.data(), &click::CredentialsService::credentialsFound,
70 this, &DownloadManager::handleCredentialsFound);
71 if(!c){
72 qDebug() << "failed to connect to credentialsFound";
73 }

Since we are using the new connection style that is checked at compile time.. do we really need to do that if?? If not I'd remove it, less code less bugs.

review: Needs Fixing
Revision history for this message
Thomas Voß (thomas-voss) wrote :

On first glance, one thing sticks out to me: Why do you need the custom test_main.cpp if the fixtures all have an internal QCoreApplication instance?

review: Needs Information
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
135. By Mike McCracken

remove unnecessary custom main

Revision history for this message
Mike McCracken (mikemc) wrote :

@thomas-voss : thanks for pointing that out, indeed the custom main is not needed.
It isn't used in the build, and should have been deleted already.

Revision history for this message
Mike McCracken (mikemc) wrote :

@mandel - I've removed the 'explicit'.

I'll make a note of the convention for using _ . I haven't changed it yet though, need to see if we're going to follow a particular convention everywhere.

As for the connection checks, let's leave them in until we know for sure that the new-style connections work on ARM.
As you probably remember, in an earlier version of this code, we had an issue where the connections compiled fine on both ARM and x86 and worked fine on x86, but failed to execute correctly on ARM. That was the initial reason for adding those checks and we haven't ironed that out yet.

136. By Mike McCracken

remove unneeded use of explicit

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alejandro J. Cura (alecu) wrote :

> @mandel - I've removed the 'explicit'.
>
> I'll make a note of the convention for using _ . I haven't changed it yet
> though, need to see if we're going to follow a particular convention
> everywhere.

As proposed by tvoss elsewhere, let's adhere to the style guidelines of the Scopes API and the Unity API teams:
http://bazaar.launchpad.net/~rocket-scientists/canonical-client-development-guidelines/trunk/view/head:/c%2B%2B/cppguide.xml#L2276
(please bookmark that).

According to the above, the trailing _ is not needed for classes with few member variables. And the leading m_ is only suggested for code that need to fit a framework where it's used (like Qt).

My guess is that's no decoration is needed in this case.

> As for the connection checks, let's leave them in until we know for sure that
> the new-style connections work on ARM.

Perhaps we can make it an assert then? That would be easier to catch in the arm jenkins.

Revision history for this message
Manuel de la Peña (mandel) wrote :

> > @mandel - I've removed the 'explicit'.
> >
> > I'll make a note of the convention for using _ . I haven't changed it yet
> > though, need to see if we're going to follow a particular convention
> > everywhere.
>
> As proposed by tvoss elsewhere, let's adhere to the style guidelines of the
> Scopes API and the Unity API teams:
> http://bazaar.launchpad.net/~rocket-scientists/canonical-client-development-
> guidelines/trunk/view/head:/c%2B%2B/cppguide.xml#L2276
> (please bookmark that).
>
> According to the above, the trailing _ is not needed for classes with few
> member variables. And the leading m_ is only suggested for code that need to
> fit a framework where it's used (like Qt).
>
> My guess is that's no decoration is needed in this case.

Ok, I mentioned because there is a _dm one being used in a diff class and would be nice to be consistent.

>
> > As for the connection checks, let's leave them in until we know for sure
> that
> > the new-style connections work on ARM.
>
> Perhaps we can make it an assert then? That would be easier to catch in the
> arm jenkins.

Sounds reasonable, certainly not something that should block the merge.

review: Approve
Revision history for this message
Mike McCracken (mikemc) wrote :

ping @thomas-voss - I addressed your needs-info, would you mind taking a look again?
Thanks!

137. By Mike McCracken

merge with trunk, fix minor conflict in cmakelists' adding test source files.

Revision history for this message
Mike McCracken (mikemc) wrote :

@thomas-voss, also please note that the actual test case added in this branch is completely reworked in the following branch: ~mikemc/unity-scope-click/dlmgr-network-error-tests

In particular, a bunch of code cleanup you did on the test case on Tuesday is incorporated there.
At the time I thought this branch was about to land, so I didn't include those changes here.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Thomas Voß (thomas-voss) wrote :

Thanks for addressing my question/comment. With that: LGTM.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'scope/click/CMakeLists.txt'
2--- scope/click/CMakeLists.txt 2014-02-04 23:53:28 +0000
3+++ scope/click/CMakeLists.txt 2014-02-06 18:36:09 +0000
4@@ -5,6 +5,7 @@
5 add_library(${SCOPE_LIB_NAME} SHARED
6 query.cpp
7 scope.cpp
8+ ubuntuone_credentials.cpp
9 download-manager.cpp
10 network_access_manager.cpp
11 index.cpp
12@@ -13,7 +14,7 @@
13
14 configure_file(config.h.in config.h)
15
16-include_directories (${CMAKE_CURRENT_BINARY_DIR})
17+include_directories (${CMAKE_BINARY_DIR}/scope)
18
19 qt5_use_modules (${SCOPE_LIB_NAME} Network)
20
21
22=== modified file 'scope/click/download-manager.cpp'
23--- scope/click/download-manager.cpp 2014-01-28 10:11:21 +0000
24+++ scope/click/download-manager.cpp 2014-02-06 18:36:09 +0000
25@@ -34,44 +34,45 @@
26 #include <QString>
27 #include <QTimer>
28
29-#include <ssoservice.h>
30+#include <ubuntuone_credentials.h>
31+
32 #include <token.h>
33-#include <requests.h>
34-#include <errormessages.h>
35
36 namespace u1 = UbuntuOne;
37
38 struct click::DownloadManager::Private
39 {
40- Private(const QSharedPointer<click::network::AccessManager>& networkAccessManager)
41- : nam(networkAccessManager)
42+ Private(const QSharedPointer<click::network::AccessManager>& networkAccessManager,
43+ const QSharedPointer<click::CredentialsService>& credentialsService)
44+ : nam(networkAccessManager), credentialsService(credentialsService)
45 {
46 }
47
48 void updateCredentialsFromService()
49 {
50- service.getCredentials();
51+ credentialsService->getCredentials();
52 }
53
54- u1::SSOService service;
55 QSharedPointer<click::network::AccessManager> nam;
56+ QSharedPointer<click::CredentialsService> credentialsService;
57 QSharedPointer<click::network::Reply> reply;
58 QString downloadUrl;
59 };
60
61 click::DownloadManager::DownloadManager(const QSharedPointer<click::network::AccessManager>& networkAccessManager,
62+ const QSharedPointer<click::CredentialsService>& credentialsService,
63 QObject *parent)
64 : QObject(parent),
65- impl(new Private(networkAccessManager))
66+ impl(new Private(networkAccessManager, credentialsService))
67 {
68- QMetaObject::Connection c = connect(&impl->service, &u1::SSOService::credentialsFound,
69+ QMetaObject::Connection c = connect(impl->credentialsService.data(), &click::CredentialsService::credentialsFound,
70 this, &DownloadManager::handleCredentialsFound);
71 if(!c){
72 qDebug() << "failed to connect to credentialsFound";
73 }
74
75- c = connect(&impl->service, &u1::SSOService::credentialsNotFound,
76- this, &DownloadManager::handleCredentialsNotFound);
77+ c = connect(impl->credentialsService.data(), &click::CredentialsService::credentialsNotFound,
78+ this, &DownloadManager::handleCredentialsNotFound);
79 if(!c){
80 qDebug() << "failed to connect to credentialsNotFound";
81 }
82
83=== modified file 'scope/click/download-manager.h'
84--- scope/click/download-manager.h 2014-02-04 22:45:20 +0000
85+++ scope/click/download-manager.h 2014-02-06 18:36:09 +0000
86@@ -30,9 +30,10 @@
87 #ifndef CLICK_DOWNLOAD_MANAGER_H
88 #define CLICK_DOWNLOAD_MANAGER_H
89
90-#include "config.h"
91+#include <click/config.h>
92
93 #include "network_access_manager.h"
94+#include "ubuntuone_credentials.h"
95
96 #include <QDebug>
97 #include <QNetworkReply>
98@@ -54,6 +55,7 @@
99
100 public:
101 DownloadManager(const QSharedPointer<click::network::AccessManager>& networkAccessManager,
102+ const QSharedPointer<click::CredentialsService>& ssoService,
103 QObject *parent = 0);
104 virtual ~DownloadManager();
105
106
107=== added file 'scope/click/ubuntuone_credentials.cpp'
108--- scope/click/ubuntuone_credentials.cpp 1970-01-01 00:00:00 +0000
109+++ scope/click/ubuntuone_credentials.cpp 2014-02-06 18:36:09 +0000
110@@ -0,0 +1,58 @@
111+/*
112+ * Copyright (C) 2014 Canonical Ltd.
113+ *
114+ * This program is free software: you can redistribute it and/or modify it
115+ * under the terms of the GNU General Public License version 3, as published
116+ * by the Free Software Foundation.
117+ *
118+ * This program is distributed in the hope that it will be useful, but
119+ * WITHOUT ANY WARRANTY; without even the implied warranties of
120+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
121+ * PURPOSE. See the GNU General Public License for more details.
122+ *
123+ * You should have received a copy of the GNU General Public License along
124+ * with this program. If not, see <http://www.gnu.org/licenses/>.
125+ *
126+ * In addition, as a special exception, the copyright holders give
127+ * permission to link the code of portions of this program with the
128+ * OpenSSL library under certain conditions as described in each
129+ * individual source file, and distribute linked combinations
130+ * including the two.
131+ * You must obey the GNU General Public License in all respects
132+ * for all of the code used other than OpenSSL. If you modify
133+ * file(s) with this exception, you may extend this exception to your
134+ * version of the file(s), but you are not obligated to do so. If you
135+ * do not wish to do so, delete this exception statement from your
136+ * version. If you delete this exception statement from all source
137+ * files in the program, then also delete it here.
138+ */
139+
140+#include "ubuntuone_credentials.h"
141+
142+namespace u1 = UbuntuOne;
143+
144+click::CredentialsService::CredentialsService()
145+{
146+ ssoService.reset(new u1::SSOService());
147+ // Forward signals directly:
148+ connect(ssoService.data(), &u1::SSOService::credentialsFound,
149+ this, &click::CredentialsService::credentialsFound);
150+ connect(ssoService.data(), &u1::SSOService::credentialsNotFound,
151+ this, &click::CredentialsService::credentialsNotFound);
152+ connect(ssoService.data(), &u1::SSOService::credentialsDeleted,
153+ this, &click::CredentialsService::credentialsDeleted);
154+}
155+
156+click::CredentialsService::~CredentialsService()
157+{
158+}
159+
160+void click::CredentialsService::getCredentials()
161+{
162+ ssoService->getCredentials();
163+}
164+
165+void click::CredentialsService::invalidateCredentials()
166+{
167+ ssoService->invalidateCredentials();
168+}
169
170=== added file 'scope/click/ubuntuone_credentials.h'
171--- scope/click/ubuntuone_credentials.h 1970-01-01 00:00:00 +0000
172+++ scope/click/ubuntuone_credentials.h 2014-02-06 18:36:09 +0000
173@@ -0,0 +1,65 @@
174+/*
175+ * Copyright (C) 2014 Canonical Ltd.
176+ *
177+ * This program is free software: you can redistribute it and/or modify it
178+ * under the terms of the GNU General Public License version 3, as published
179+ * by the Free Software Foundation.
180+ *
181+ * This program is distributed in the hope that it will be useful, but
182+ * WITHOUT ANY WARRANTY; without even the implied warranties of
183+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
184+ * PURPOSE. See the GNU General Public License for more details.
185+ *
186+ * You should have received a copy of the GNU General Public License along
187+ * with this program. If not, see <http://www.gnu.org/licenses/>.
188+ *
189+ * In addition, as a special exception, the copyright holders give
190+ * permission to link the code of portions of this program with the
191+ * OpenSSL library under certain conditions as described in each
192+ * individual source file, and distribute linked combinations
193+ * including the two.
194+ * You must obey the GNU General Public License in all respects
195+ * for all of the code used other than OpenSSL. If you modify
196+ * file(s) with this exception, you may extend this exception to your
197+ * version of the file(s), but you are not obligated to do so. If you
198+ * do not wish to do so, delete this exception statement from your
199+ * version. If you delete this exception statement from all source
200+ * files in the program, then also delete it here.
201+ */
202+
203+#ifndef _UBUNTUONE_CREDENTIALS_H_
204+#define _UBUNTUONE_CREDENTIALS_H_
205+
206+#include <ssoservice.h>
207+#include <token.h>
208+
209+namespace click
210+{
211+
212+class CredentialsService : public UbuntuOne::SSOService
213+{
214+ Q_OBJECT
215+
216+public:
217+ CredentialsService();
218+ CredentialsService(const CredentialsService&) = delete;
219+ virtual ~CredentialsService();
220+
221+ CredentialsService& operator=(const CredentialsService&) = delete;
222+
223+ virtual void getCredentials();
224+ virtual void invalidateCredentials();
225+
226+signals:
227+ void credentialsDeleted();
228+ void credentialsFound(const UbuntuOne::Token& token);
229+ void credentialsNotFound();
230+
231+private:
232+ QScopedPointer<UbuntuOne::SSOService> ssoService;
233+
234+}; // CredentialsService
235+
236+
237+} // namespace click
238+#endif /* _UBUNTUONE_CREDENTIALS_H_ */
239
240=== modified file 'scope/tests/CMakeLists.txt'
241--- scope/tests/CMakeLists.txt 2014-02-04 23:53:28 +0000
242+++ scope/tests/CMakeLists.txt 2014-02-06 18:36:09 +0000
243@@ -16,12 +16,14 @@
244
245 include_directories (
246 ${CMAKE_SOURCE_DIR}/scope
247+ ${CMAKE_BINARY_DIR}/scope
248 ${GTEST_INCLUDE_DIR}
249 ${GMOCK_INCLUDE_DIR}
250 )
251
252 add_executable (${CLICKSCOPE_TESTS_TARGET}
253 mock_network_access_manager.h
254+ test_download_manager.cpp
255 test_index.cpp
256 test_webclient.cpp
257 )
258@@ -46,4 +48,4 @@
259 )
260
261 add_subdirectory(integration)
262-#add_subdirectory(download_manager_tool)
263+add_subdirectory(download_manager_tool)
264
265=== modified file 'scope/tests/download_manager_tool/download_manager_tool.cpp'
266--- scope/tests/download_manager_tool/download_manager_tool.cpp 2014-01-27 18:20:27 +0000
267+++ scope/tests/download_manager_tool/download_manager_tool.cpp 2014-02-06 18:36:09 +0000
268@@ -38,9 +38,11 @@
269 DownloadManagerTool::DownloadManagerTool(QObject *parent):
270 QObject(parent)
271 {
272- QObject::connect(&_dm, &click::DownloadManager::clickTokenFetched,
273+ _dm = new click::DownloadManager(QSharedPointer<click::network::AccessManager>(new click::network::AccessManager()),
274+ QSharedPointer<click::CredentialsService>(new click::CredentialsService()));
275+ QObject::connect(_dm, &click::DownloadManager::clickTokenFetched,
276 this, &DownloadManagerTool::handleFetchResponse);
277- QObject::connect(&_dm, &click::DownloadManager::clickTokenFetchError,
278+ QObject::connect(_dm, &click::DownloadManager::clickTokenFetchError,
279 this, &DownloadManagerTool::handleFetchResponse);
280 }
281
282@@ -52,14 +54,14 @@
283
284 void DownloadManagerTool::fetchClickToken()
285 {
286- _dm.fetchClickToken(_url);
287+ _dm->fetchClickToken(_url);
288 }
289
290 int main(int argc, char *argv[])
291 {
292
293 QCoreApplication a(argc, argv);
294- DownloadManagerTool tool(&a);
295+ DownloadManagerTool tool;
296
297 if (argc != 2) {
298 QTextStream(stderr) << "Usage: download_manager_tool https://public.apps.ubuntu.com/download/<<rest of click package dl url>>"
299@@ -73,7 +75,7 @@
300 QObject::connect(&tool, SIGNAL(finished()), &a, SLOT(quit()));
301
302 QTimer::singleShot(0, &tool, SLOT(fetchClickToken()));
303-
304+ qInstallMessageHandler(0);
305 return a.exec();
306 }
307
308
309=== modified file 'scope/tests/download_manager_tool/download_manager_tool.h'
310--- scope/tests/download_manager_tool/download_manager_tool.h 2014-01-27 18:20:27 +0000
311+++ scope/tests/download_manager_tool/download_manager_tool.h 2014-02-06 18:36:09 +0000
312@@ -31,7 +31,7 @@
313 #define _DOWNLOAD_MANAGER_TOOL_H_
314
315 #include <QString>
316-#include <download-manager.h>
317+#include <click/download-manager.h>
318
319 class DownloadManagerTool : public QObject {
320 Q_OBJECT
321@@ -51,7 +51,8 @@
322
323 private:
324 QString _url;
325- click::DownloadManager _dm;
326+ click::DownloadManager *_dm;
327+
328 };
329
330 #endif /* _DOWNLOAD_MANAGER_TOOL_H_ */
331
332=== removed file 'scope/tests/main.cpp'
333--- scope/tests/main.cpp 2014-01-13 22:00:30 +0000
334+++ scope/tests/main.cpp 1970-01-01 00:00:00 +0000
335@@ -1,36 +0,0 @@
336-/*
337- * Copyright (C) 2014 Canonical Ltd.
338- *
339- * This program is free software: you can redistribute it and/or modify it
340- * under the terms of the GNU General Public License version 3, as published
341- * by the Free Software Foundation.
342- *
343- * This program is distributed in the hope that it will be useful, but
344- * WITHOUT ANY WARRANTY; without even the implied warranties of
345- * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
346- * PURPOSE. See the GNU General Public License for more details.
347- *
348- * You should have received a copy of the GNU General Public License along
349- * with this program. If not, see <http://www.gnu.org/licenses/>.
350- *
351- * In addition, as a special exception, the copyright holders give
352- * permission to link the code of portions of this program with the
353- * OpenSSL library under certain conditions as described in each
354- * individual source file, and distribute linked combinations
355- * including the two.
356- * You must obey the GNU General Public License in all respects
357- * for all of the code used other than OpenSSL. If you modify
358- * file(s) with this exception, you may extend this exception to your
359- * version of the file(s), but you are not obligated to do so. If you
360- * do not wish to do so, delete this exception statement from your
361- * version. If you delete this exception statement from all source
362- * files in the program, then also delete it here.
363- */
364-
365-#include <QCoreApplication>
366-#include "./test_runner.h"
367-
368-int main(int argc, char *argv[]) {
369- QCoreApplication a(argc, argv);
370- return RUN_ALL_QTESTS(argc, argv);
371-}
372
373=== added file 'scope/tests/mock_ubuntuone_credentials.h'
374--- scope/tests/mock_ubuntuone_credentials.h 1970-01-01 00:00:00 +0000
375+++ scope/tests/mock_ubuntuone_credentials.h 2014-02-06 18:36:09 +0000
376@@ -0,0 +1,38 @@
377+/*
378+ * Copyright (C) 2014 Canonical Ltd.
379+ *
380+ * This program is free software: you can redistribute it and/or modify it
381+ * under the terms of the GNU General Public License version 3, as published
382+ * by the Free Software Foundation.
383+ *
384+ * This program is distributed in the hope that it will be useful, but
385+ * WITHOUT ANY WARRANTY; without even the implied warranties of
386+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
387+ * PURPOSE. See the GNU General Public License for more details.
388+ *
389+ * You should have received a copy of the GNU General Public License along
390+ * with this program. If not, see <http://www.gnu.org/licenses/>.
391+ *
392+ * In addition, as a special exception, the copyright holders give
393+ * permission to link the code of portions of this program with the
394+ * OpenSSL library under certain conditions as described in each
395+ * individual source file, and distribute linked combinations
396+ * including the two.
397+ * You must obey the GNU General Public License in all respects
398+ * for all of the code used other than OpenSSL. If you modify
399+ * file(s) with this exception, you may extend this exception to your
400+ * version of the file(s), but you are not obligated to do so. If you
401+ * do not wish to do so, delete this exception statement from your
402+ * version. If you delete this exception statement from all source
403+ * files in the program, then also delete it here.
404+ */
405+
406+
407+class MockCredentialsService : public click::CredentialsService {
408+ public:
409+ MOCK_METHOD0(getCredentials,
410+ void());
411+ MOCK_METHOD0(invalidateCredentials,
412+ void());
413+};
414+
415
416=== modified file 'scope/tests/test_download_manager.cpp'
417--- scope/tests/test_download_manager.cpp 2014-01-27 18:20:27 +0000
418+++ scope/tests/test_download_manager.cpp 2014-02-06 18:36:09 +0000
419@@ -27,51 +27,138 @@
420 * files in the program, then also delete it here.
421 */
422
423-#include "test_download_manager.h"
424-
425-#define SCOPE_TEST_TIMEOUT_MSEC 5000
426-
427-static const QString TEST_URL("http://test.local/");
428-
429-void TestableDownloadManager::setShouldSignalCredsFound(bool shouldSignalCredsFound)
430-{
431- _shouldSignalCredsFound = shouldSignalCredsFound;
432-}
433-
434-void TestableDownloadManager::getCredentials()
435-{
436- if (_shouldSignalCredsFound) {
437- emit service.credentialsFound(_token);
438- }else{
439- emit service.credentialsNotFound();
440- }
441+#include <QCoreApplication>
442+#include <QDebug>
443+#include <QString>
444+#include <QThread>
445+#include <QTimer>
446+
447+#include <gmock/gmock.h>
448+#include <gtest/gtest.h>
449+
450+#include <click/download-manager.h>
451+
452+#include "mock_network_access_manager.h"
453+#include "mock_ubuntuone_credentials.h"
454+
455+
456+namespace
457+{
458+const QString TEST_URL("http://test.local/");
459+
460+struct DownloadManagerTest : public ::testing::Test
461+{
462+ DownloadManagerTest() : app(argc, argv)
463+ {
464+
465+ QSharedPointer<click::network::AccessManager> namPtr(&mockNam,
466+ [](click::network::AccessManager*) {});
467+
468+ QSharedPointer<click::CredentialsService> csPtr(&mockCredentialsService,
469+ [](click::CredentialsService*) {});
470+
471+ dm.reset(new click::DownloadManager(namPtr, csPtr));
472+
473+ QObject::connect(
474+ &testTimeout, &QTimer::timeout,
475+ [this]() { app.quit(); FAIL() << "Operation timed out."; } );
476+ }
477+
478+ void SetUp()
479+ {
480+ const int oneSecondInMsec = 1000;
481+ testTimeout.start(2 * oneSecondInMsec);
482+ }
483+
484+ void Quit()
485+ {
486+ app.quit();
487+ }
488+
489+ int argc = 0;
490+ char** argv = nullptr;
491+ QCoreApplication app;
492+ QTimer testTimeout;
493+ QScopedPointer<click::DownloadManager> dm;
494+ MockNetworkAccessManager mockNam;
495+ MockCredentialsService mockCredentialsService;
496+};
497+
498+
499+struct DownloadManagerMockClient {
500+ MOCK_METHOD1(onFetchClickErrorEmitted, void(QString errorMessage));
501+};
502+
503+} // anon namespace
504+
505+
506+TEST_F(DownloadManagerTest, testFetchClickTokenCredentialsNotFound)
507+{
508+ using namespace ::testing;
509+
510+ // Mock the credentials service, to signal that creds are not found:
511+ EXPECT_CALL(mockCredentialsService, getCredentials())
512+ .Times(1).WillOnce(
513+ InvokeWithoutArgs(&mockCredentialsService,
514+ &MockCredentialsService::credentialsNotFound));
515+
516+ // We should not hit the NAM without creds:
517+ EXPECT_CALL(mockNam, head(_)).Times(0);
518+
519+ // Connect the mock client to downloadManager's error signal,
520+ // to check that it sends appropriate signals:
521+
522+ DownloadManagerMockClient mockDownloadManagerClient;
523+ QObject::connect(dm.data(), &click::DownloadManager::clickTokenFetchError,
524+ [&mockDownloadManagerClient](const QString& error)
525+ {
526+ mockDownloadManagerClient.onFetchClickErrorEmitted(error);
527+ });
528+
529+ // The error signal should be sent once, and we clean up the
530+ // test's QCoreApplication in its handler:
531+ EXPECT_CALL(mockDownloadManagerClient, onFetchClickErrorEmitted(_))
532+ .Times(1)
533+ .WillOnce(
534+ InvokeWithoutArgs(
535+ this,
536+ &DownloadManagerTest::Quit));
537+
538+ // Now start the function we're testing, after a delay. This is
539+ // awkwardly verbose because QTimer::singleShot doesn't accept
540+ // arguments or lambdas.
541+
542+ // We need to delay the call until after the app.exec() call so
543+ // that when we call app.quit() on success, there is a running app
544+ // to quit.
545+ QScopedPointer<QTimer> timer(new QTimer());
546+ timer->setSingleShot(true);
547+ QObject::connect(timer.data(), &QTimer::timeout, [&]() {
548+ dm->fetchClickToken(TEST_URL);
549+ } );
550+ timer->start(0);
551+
552+ // now exec the app so events can proceed:
553+ app.exec();
554+
555 }
556
557 // Test Cases:
558
559-void TestDownloadManager::testFetchClickTokenCredentialsNotFound()
560-{
561- _tdm.setShouldSignalCredsFound(false);
562- FakeNam::shouldSignalNetworkError = false;
563- QSignalSpy spy(&_tdm, SIGNAL(clickTokenFetchError(QString)));
564- _tdm.fetchClickToken(TEST_URL);
565- QTRY_COMPARE_WITH_TIMEOUT(spy.count(), 1, SCOPE_TEST_TIMEOUT_MSEC);
566-}
567-
568-void TestDownloadManager::testFetchClickTokenCredsFoundButNetworkError()
569-{
570- _tdm.setShouldSignalCredsFound(true);
571- FakeNam::shouldSignalNetworkError = true;
572- QSignalSpy spy(&_tdm, SIGNAL(clickTokenFetchError(QString)));
573- _tdm.fetchClickToken(TEST_URL);
574- QTRY_COMPARE_WITH_TIMEOUT(spy.count(), 1, SCOPE_TEST_TIMEOUT_MSEC);
575-}
576-
577-void TestDownloadManager::testFetchClickTokenSuccess()
578-{
579- _tdm.setShouldSignalCredsFound(true);
580- FakeNam::shouldSignalNetworkError = false;
581- QSignalSpy spy(&_tdm, SIGNAL(clickTokenFetched(QString)));
582- _tdm.fetchClickToken(TEST_URL);
583- QTRY_COMPARE_WITH_TIMEOUT(spy.count(), 1, SCOPE_TEST_TIMEOUT_MSEC);
584-}
585+// void TestDownloadManager::testFetchClickTokenCredsFoundButNetworkError()
586+// {
587+// _tdm.setShouldSignalCredsFound(true);
588+// FakeNam::shouldSignalNetworkError = true;
589+// QSignalSpy spy(&_tdm, SIGNAL(clickTokenFetchError(QString)));
590+// _tdm.fetchClickToken(TEST_URL);
591+// QTRY_COMPARE_WITH_TIMEOUT(spy.count(), 1, SCOPE_TEST_TIMEOUT_MSEC);
592+// }
593+
594+// void TestDownloadManager::testFetchClickTokenSuccess()
595+// {
596+// _tdm.setShouldSignalCredsFound(true);
597+// FakeNam::shouldSignalNetworkError = false;
598+// QSignalSpy spy(&_tdm, SIGNAL(clickTokenFetched(QString)));
599+// _tdm.fetchClickToken(TEST_URL);
600+// QTRY_COMPARE_WITH_TIMEOUT(spy.count(), 1, SCOPE_TEST_TIMEOUT_MSEC);
601+// }
602
603=== removed file 'scope/tests/test_download_manager.h'
604--- scope/tests/test_download_manager.h 2014-01-27 18:20:27 +0000
605+++ scope/tests/test_download_manager.h 1970-01-01 00:00:00 +0000
606@@ -1,78 +0,0 @@
607-/*
608- * Copyright (C) 2014 Canonical Ltd.
609- *
610- * This program is free software: you can redistribute it and/or modify it
611- * under the terms of the GNU General Public License version 3, as published
612- * by the Free Software Foundation.
613- *
614- * This program is distributed in the hope that it will be useful, but
615- * WITHOUT ANY WARRANTY; without even the implied warranties of
616- * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
617- * PURPOSE. See the GNU General Public License for more details.
618- *
619- * You should have received a copy of the GNU General Public License along
620- * with this program. If not, see <http://www.gnu.org/licenses/>.
621- *
622- * In addition, as a special exception, the copyright holders give
623- * permission to link the code of portions of this program with the
624- * OpenSSL library under certain conditions as described in each
625- * individual source file, and distribute linked combinations
626- * including the two.
627- * You must obey the GNU General Public License in all respects
628- * for all of the code used other than OpenSSL. If you modify
629- * file(s) with this exception, you may extend this exception to your
630- * version of the file(s), but you are not obligated to do so. If you
631- * do not wish to do so, delete this exception statement from your
632- * version. If you delete this exception statement from all source
633- * files in the program, then also delete it here.
634- */
635-
636-#ifndef TEST_DOWNLOAD_MANAGER_H
637-#define TEST_DOWNLOAD_MANAGER_H
638-
639-#include <fake_nam.h>
640-
641-#include <QObject>
642-#include <QDebug>
643-#include <QString>
644-#include <QTest>
645-#include <QTimer>
646-#include <QSignalSpy>
647-
648-#include <ssoservice.h>
649-
650-#include <test_runner.h>
651-#include <download-manager.h>
652-
653-class TestableDownloadManager : public click::DownloadManager {
654- Q_OBJECT
655-
656-public:
657-
658- void setShouldSignalCredsFound(bool shouldSignalCredsFound);
659- void setShouldSignalNetworkError(bool shouldSignalNetworkError);
660-
661- void getCredentials() override;
662-
663-private:
664- bool _shouldSignalCredsFound = true;
665- bool _shouldSignalNetworkError = false;
666- UbuntuOne::Token _token;
667-};
668-
669-
670-class TestDownloadManager : public QObject
671-{
672- Q_OBJECT
673-
674-private slots:
675- void testFetchClickTokenCredentialsNotFound();
676- void testFetchClickTokenCredsFoundButNetworkError();
677- void testFetchClickTokenSuccess();
678-private:
679- TestableDownloadManager _tdm;
680-};
681-
682-DECLARE_TEST(TestDownloadManager)
683-
684-#endif // TEST_DOWNLOAD_MANAGER_H

Subscribers

People subscribed via source and target branches

to all changes: