Merge lp:~mikemc/unity-scope-click/dlmgr-tests-again-again into lp:unity-scope-click
- dlmgr-tests-again-again
- Merge into trunk
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 |
Related bugs: |
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_
Only adds one test for download manager. Proposing at this point in the interest of shorter diffs.
Run with 'make test'.
PS Jenkins bot (ps-jenkins) wrote : | # |
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 CredentialsServ
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<
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:
70 this, &DownloadManage
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.
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?
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:134
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 135. By Mike McCracken
-
remove unnecessary custom main
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.
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
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:136
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
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://
(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.
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://
> guidelines/
> (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.
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.
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/
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.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:137
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Thomas Voß (thomas-voss) wrote : | # |
Thanks for addressing my question/comment. With that: LGTM.
Preview Diff
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 |
PASSED: Continuous integration, rev:133 jenkins. qa.ubuntu. com/job/ unity-scope- click-ci/ 219/ jenkins. qa.ubuntu. com/job/ unity-scope- click-trusty- amd64-ci/ 117 jenkins. qa.ubuntu. com/job/ unity-scope- click-trusty- armhf-ci/ 117
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/unity- scope-click- ci/219/ rebuild
http://