Merge lp:~mikemc/unity-scope-click/startdownload-error-handling into lp:unity-scope-click
- startdownload-error-handling
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Alejandro J. Cura |
Approved revision: | 153 |
Merged at revision: | 162 |
Proposed branch: | lp:~mikemc/unity-scope-click/startdownload-error-handling |
Merge into: | lp:unity-scope-click |
Prerequisite: | lp:~diegosarmentero/unity-scope-click/fix-progress |
Diff against target: |
413 lines (+147/-38) 7 files modified
scope/click/download-manager.cpp (+23/-8) scope/click/download-manager.h (+5/-1) scope/click/preview.cpp (+51/-11) scope/click/preview.h (+15/-0) scope/click/scope.cpp (+13/-2) scope/tests/download_manager_tool/download_manager_tool.cpp (+9/-2) scope/tests/test_download_manager.cpp (+31/-14) |
To merge this branch: | bzr merge lp:~mikemc/unity-scope-click/startdownload-error-handling |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alejandro J. Cura (community) | Approve | ||
Diego Sarmentero (community) | Approve | ||
PS Jenkins bot | continuous-integration | Approve | |
Review via email:
|
Commit message
- Surface errors in the download and install process via ErrorPreview
Description of the change
- Surface errors in the download and install process via ErrorPreview
Shows user error messages when
- creds are not found
- any error is returned from download manager, including network errors or failed install
Other changes:
- fix tests to account for separate error signals for creds vs. downloading/
- fix a problem in the test matcher for the pkcon command
- use constants instead of strings in some places
- cleanup whitespace
TO TEST:
The download manager tests are currently disabled because they're flaky on ARM.
To run all the tests, which should pass on x86, do:
% GTEST_ALSO_
TO TEST IRL:
using unity-scope-tool or a device, try installing an app with:
- no credentials - you should get a specific error message for this
- no network - you should get a network error or a download/install error, depending on when you cut the net.
- install error - not sure how to trigger this on the device. maybe you could mess with permissions of the click directory temporarily. on desktop, it's easy, since pkcon doesn't work with clicks on desktop, according to the packagekit-
http://
In each case you should get a serviceable (untranslated, un-designed for wording) error message.
- 153. By Mike McCracken
-
remove unused boost/optional header
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
Preview Diff
1 | === modified file 'scope/click/download-manager.cpp' | |||
2 | --- scope/click/download-manager.cpp 2014-02-18 11:26:15 +0000 | |||
3 | +++ scope/click/download-manager.cpp 2014-02-20 06:27:03 +0000 | |||
4 | @@ -215,7 +215,7 @@ | |||
5 | 215 | void click::DownloadManager::handleCredentialsNotFound() | 215 | void click::DownloadManager::handleCredentialsNotFound() |
6 | 216 | { | 216 | { |
7 | 217 | qDebug() << "No credentials were found."; | 217 | qDebug() << "No credentials were found."; |
9 | 218 | emit clickTokenFetchError(QString("No creds found")); | 218 | emit credentialsNotFound(); |
10 | 219 | } | 219 | } |
11 | 220 | 220 | ||
12 | 221 | void click::DownloadManager::handleNetworkFinished() | 221 | void click::DownloadManager::handleNetworkFinished() |
13 | @@ -295,7 +295,8 @@ | |||
14 | 295 | // TODO, unimplemented. see https://bugs.launchpad.net/ubuntu-download-manager/+bug/1277814 | 295 | // TODO, unimplemented. see https://bugs.launchpad.net/ubuntu-download-manager/+bug/1277814 |
15 | 296 | } | 296 | } |
16 | 297 | 297 | ||
18 | 298 | void click::Downloader::startDownload(std::string url, std::string package_name, const std::function<void (std::string)>& callback) | 298 | void click::Downloader::startDownload(std::string url, std::string package_name, |
19 | 299 | const std::function<void (std::pair<std::string, click::InstallError >)>& callback) | ||
20 | 299 | { | 300 | { |
21 | 300 | qt::core::world::enter_with_task([this, callback, url, package_name] (qt::core::world::Environment& /*env*/) | 301 | qt::core::world::enter_with_task([this, callback, url, package_name] (qt::core::world::Environment& /*env*/) |
22 | 301 | { | 302 | { |
23 | @@ -304,16 +305,30 @@ | |||
24 | 304 | QObject::connect(&dm, &click::DownloadManager::downloadStarted, | 305 | QObject::connect(&dm, &click::DownloadManager::downloadStarted, |
25 | 305 | [callback](QString downloadId) | 306 | [callback](QString downloadId) |
26 | 306 | { | 307 | { |
30 | 307 | std::cout << "Download started: " << downloadId.toStdString() << std::endl; | 308 | qDebug() << "Download started, id: " << downloadId; |
31 | 308 | callback(downloadId.toUtf8().data()); | 309 | auto ret = std::pair<std::string, click::InstallError>(downloadId.toUtf8().data(), |
32 | 309 | }); | 310 | click::InstallError::NoError); |
33 | 311 | callback(ret); | ||
34 | 312 | }); | ||
35 | 313 | |||
36 | 314 | QObject::connect(&dm, &click::DownloadManager::credentialsNotFound, | ||
37 | 315 | [callback]() | ||
38 | 316 | { | ||
39 | 317 | qDebug() << "Credentials not found:"; | ||
40 | 318 | auto ret = std::pair<std::string, click::InstallError>(std::string(), | ||
41 | 319 | click::InstallError::CredentialsError); | ||
42 | 320 | callback(ret); | ||
43 | 321 | }); | ||
44 | 322 | |||
45 | 310 | QObject::connect(&dm, &click::DownloadManager::downloadError, | 323 | QObject::connect(&dm, &click::DownloadManager::downloadError, |
47 | 311 | [](QString errorMessage) | 324 | [callback](QString errorMessage) |
48 | 312 | { | 325 | { |
49 | 313 | // TODO: unclear how to handle errors | ||
50 | 314 | qDebug() << "Error creating download:" << errorMessage; | 326 | qDebug() << "Error creating download:" << errorMessage; |
51 | 327 | auto ret = std::pair<std::string, click::InstallError>(errorMessage.toStdString(), | ||
52 | 328 | click::InstallError::DownloadInstallError); | ||
53 | 329 | callback(ret); | ||
54 | 315 | }); | 330 | }); |
56 | 316 | std::cout << "About to call start download" << std::endl; | 331 | |
57 | 317 | dm.startDownload(QString::fromStdString(url), | 332 | dm.startDownload(QString::fromStdString(url), |
58 | 318 | QString::fromStdString(package_name)); | 333 | QString::fromStdString(package_name)); |
59 | 319 | }); | 334 | }); |
60 | 320 | 335 | ||
61 | === modified file 'scope/click/download-manager.h' | |||
62 | --- scope/click/download-manager.h 2014-02-18 11:26:15 +0000 | |||
63 | +++ scope/click/download-manager.h 2014-02-20 06:27:03 +0000 | |||
64 | @@ -67,6 +67,8 @@ | |||
65 | 67 | virtual void fetchClickToken(const QString& downloadUrl); | 67 | virtual void fetchClickToken(const QString& downloadUrl); |
66 | 68 | 68 | ||
67 | 69 | signals: | 69 | signals: |
68 | 70 | |||
69 | 71 | void credentialsNotFound(); | ||
70 | 70 | void downloadStarted(const QString& downloadObjectPath); | 72 | void downloadStarted(const QString& downloadObjectPath); |
71 | 71 | void downloadError(const QString& errorMessage); | 73 | void downloadError(const QString& errorMessage); |
72 | 72 | void clickTokenFetched(const QString& clickToken); | 74 | void clickTokenFetched(const QString& clickToken); |
73 | @@ -86,13 +88,15 @@ | |||
74 | 86 | QScopedPointer<Private> impl; | 88 | QScopedPointer<Private> impl; |
75 | 87 | }; | 89 | }; |
76 | 88 | 90 | ||
77 | 91 | enum class InstallError {NoError, CredentialsError, DownloadInstallError}; | ||
78 | 89 | 92 | ||
79 | 90 | class Downloader | 93 | class Downloader |
80 | 91 | { | 94 | { |
81 | 92 | public: | 95 | public: |
82 | 93 | Downloader(const QSharedPointer<click::network::AccessManager>& networkAccessManager); | 96 | Downloader(const QSharedPointer<click::network::AccessManager>& networkAccessManager); |
83 | 94 | void get_download_progress(std::string package_name, const std::function<void (std::string)>& callback); | 97 | void get_download_progress(std::string package_name, const std::function<void (std::string)>& callback); |
85 | 95 | void startDownload(std::string url, std::string package_name, const std::function<void (std::string)>& callback); | 98 | void startDownload(std::string url, std::string package_name, |
86 | 99 | const std::function<void (std::pair<std::string, InstallError>)>& callback); | ||
87 | 96 | private: | 100 | private: |
88 | 97 | QSharedPointer<click::network::AccessManager> networkAccessManager; | 101 | QSharedPointer<click::network::AccessManager> networkAccessManager; |
89 | 98 | }; | 102 | }; |
90 | 99 | 103 | ||
91 | === modified file 'scope/click/preview.cpp' | |||
92 | --- scope/click/preview.cpp 2014-02-20 06:27:03 +0000 | |||
93 | +++ scope/click/preview.cpp 2014-02-20 06:27:03 +0000 | |||
94 | @@ -41,6 +41,8 @@ | |||
95 | 41 | 41 | ||
96 | 42 | #include <QDebug> | 42 | #include <QDebug> |
97 | 43 | 43 | ||
98 | 44 | #include <boost/optional.hpp> | ||
99 | 45 | |||
100 | 44 | #include <sstream> | 46 | #include <sstream> |
101 | 45 | 47 | ||
102 | 46 | namespace | 48 | namespace |
103 | @@ -125,12 +127,14 @@ | |||
104 | 125 | reply->push(widgets); | 127 | reply->push(widgets); |
105 | 126 | } | 128 | } |
106 | 127 | 129 | ||
108 | 128 | void buildErrorPreview(scopes::PreviewReplyProxy const& reply) | 130 | void buildErrorPreview(scopes::PreviewReplyProxy const& reply, |
109 | 131 | const std::string& error_message) | ||
110 | 129 | { | 132 | { |
111 | 130 | scopes::PreviewWidgetList widgets; | 133 | scopes::PreviewWidgetList widgets; |
112 | 131 | 134 | ||
113 | 132 | scopes::PreviewWidget header("hdr", "header"); | 135 | scopes::PreviewWidget header("hdr", "header"); |
114 | 133 | header.add_attribute("title", scopes::Variant("Error")); | 136 | header.add_attribute("title", scopes::Variant("Error")); |
115 | 137 | header.add_attribute("subtitle", scopes::Variant(error_message)); | ||
116 | 134 | widgets.push_back(header); | 138 | widgets.push_back(header); |
117 | 135 | 139 | ||
118 | 136 | scopes::PreviewWidget buttons("buttons", "actions"); | 140 | scopes::PreviewWidget buttons("buttons", "actions"); |
119 | @@ -151,6 +155,7 @@ | |||
120 | 151 | 155 | ||
121 | 152 | scopes::PreviewWidget header("hdr", "header"); | 156 | scopes::PreviewWidget header("hdr", "header"); |
122 | 153 | header.add_attribute("title", scopes::Variant("Login Error")); | 157 | header.add_attribute("title", scopes::Variant("Login Error")); |
123 | 158 | header.add_attribute("subtitle", scopes::Variant("Please log in to your Ubuntu One account.")); | ||
124 | 154 | widgets.push_back(header); | 159 | widgets.push_back(header); |
125 | 155 | 160 | ||
126 | 156 | scopes::PreviewWidget buttons("buttons", "actions"); | 161 | scopes::PreviewWidget buttons("buttons", "actions"); |
127 | @@ -312,9 +317,6 @@ | |||
128 | 312 | case Type::UNINSTALLED: | 317 | case Type::UNINSTALLED: |
129 | 313 | buildUninstalledPreview(reply, details); | 318 | buildUninstalledPreview(reply, details); |
130 | 314 | break; | 319 | break; |
131 | 315 | case Type::ERROR: | ||
132 | 316 | buildErrorPreview(reply); | ||
133 | 317 | break; | ||
134 | 318 | case Type::LOGIN: | 320 | case Type::LOGIN: |
135 | 319 | buildLoginErrorPreview(reply); | 321 | buildLoginErrorPreview(reply); |
136 | 320 | break; | 322 | break; |
137 | @@ -328,8 +330,12 @@ | |||
138 | 328 | buildPurchasingPreview(reply, details); | 330 | buildPurchasingPreview(reply, details); |
139 | 329 | break; | 331 | break; |
140 | 330 | case Type::DEFAULT: | 332 | case Type::DEFAULT: |
141 | 333 | case Type::ERROR: | ||
142 | 334 | // Don't showPreview() with errors. always use the error string. | ||
143 | 331 | default: | 335 | default: |
145 | 332 | buildDefaultPreview(reply, details); | 336 | qDebug() << "reached default preview type, returning internal error preview"; |
146 | 337 | buildErrorPreview(reply, | ||
147 | 338 | std::string("Internal Error, please close and try again.")); | ||
148 | 333 | break; | 339 | break; |
149 | 334 | }; | 340 | }; |
150 | 335 | } | 341 | } |
151 | @@ -339,6 +345,30 @@ | |||
152 | 339 | this->type = type; | 345 | this->type = type; |
153 | 340 | } | 346 | } |
154 | 341 | 347 | ||
155 | 348 | |||
156 | 349 | // ErrorPreview | ||
157 | 350 | |||
158 | 351 | ErrorPreview::ErrorPreview(const std::string& error_message, | ||
159 | 352 | const QSharedPointer<click::Index>& index, | ||
160 | 353 | const unity::scopes::Result &result) | ||
161 | 354 | : Preview(result.uri(), index, result), error_message(error_message) | ||
162 | 355 | { | ||
163 | 356 | qDebug() << "in ErrorPreview constructor, error_message is " << QString::fromStdString(error_message); | ||
164 | 357 | } | ||
165 | 358 | |||
166 | 359 | ErrorPreview::~ErrorPreview() | ||
167 | 360 | { | ||
168 | 361 | |||
169 | 362 | } | ||
170 | 363 | |||
171 | 364 | void ErrorPreview::run(const unity::scopes::PreviewReplyProxy &reply) | ||
172 | 365 | { | ||
173 | 366 | buildErrorPreview(reply, error_message); | ||
174 | 367 | } | ||
175 | 368 | |||
176 | 369 | |||
177 | 370 | // InstallPreview | ||
178 | 371 | |||
179 | 342 | InstallPreview::InstallPreview(const std::string &download_url, const QSharedPointer<Index> &index, | 372 | InstallPreview::InstallPreview(const std::string &download_url, const QSharedPointer<Index> &index, |
180 | 343 | const unity::scopes::Result &result, | 373 | const unity::scopes::Result &result, |
181 | 344 | const QSharedPointer<click::network::AccessManager> &nam) | 374 | const QSharedPointer<click::network::AccessManager> &nam) |
182 | @@ -356,12 +386,22 @@ | |||
183 | 356 | void InstallPreview::run(const unity::scopes::PreviewReplyProxy &reply) | 386 | void InstallPreview::run(const unity::scopes::PreviewReplyProxy &reply) |
184 | 357 | { | 387 | { |
185 | 358 | qDebug() << "about to call startDownload in run()"; | 388 | qDebug() << "about to call startDownload in run()"; |
192 | 359 | downloader->startDownload(download_url, result["name"].get_string(),[this, reply](std::string obj_path) { | 389 | downloader->startDownload(download_url, result["name"].get_string(),[this, reply](std::pair<std::string, click::InstallError> pair) { |
193 | 360 | qDebug() << "got object path: " << QString::fromStdString(obj_path); | 390 | |
194 | 361 | index->get_details(result["name"].get_string(), [this, reply, obj_path](const click::PackageDetails& details) | 391 | auto error = pair.second; |
195 | 362 | { | 392 | |
196 | 363 | buildInstallingPreview(reply, details, obj_path); | 393 | if (error == InstallError::NoError) { |
197 | 364 | }); | 394 | auto obj_path = pair.first; |
198 | 395 | qDebug() << "got object path: " << QString::fromStdString(obj_path); | ||
199 | 396 | index->get_details(result["name"].get_string(), [this, reply, obj_path](const click::PackageDetails& details) | ||
200 | 397 | { | ||
201 | 398 | buildInstallingPreview(reply, details, obj_path); | ||
202 | 399 | }); | ||
203 | 400 | } else if (error == InstallError::CredentialsError) { | ||
204 | 401 | buildLoginErrorPreview(reply); | ||
205 | 402 | } else { | ||
206 | 403 | buildErrorPreview(reply, pair.first); | ||
207 | 404 | } | ||
208 | 365 | }); | 405 | }); |
209 | 366 | qDebug() << "after startDownload in run()"; | 406 | qDebug() << "after startDownload in run()"; |
210 | 367 | } | 407 | } |
211 | 368 | 408 | ||
212 | === modified file 'scope/click/preview.h' | |||
213 | --- scope/click/preview.h 2014-02-18 12:44:49 +0000 | |||
214 | +++ scope/click/preview.h 2014-02-20 06:27:03 +0000 | |||
215 | @@ -108,6 +108,21 @@ | |||
216 | 108 | const click::PackageDetails& details); | 108 | const click::PackageDetails& details); |
217 | 109 | }; | 109 | }; |
218 | 110 | 110 | ||
219 | 111 | class ErrorPreview : public Preview | ||
220 | 112 | { | ||
221 | 113 | protected: | ||
222 | 114 | std::string error_message; | ||
223 | 115 | public: | ||
224 | 116 | ErrorPreview(const std::string& error_message, | ||
225 | 117 | const QSharedPointer<click::Index>& index, | ||
226 | 118 | const unity::scopes::Result& result); | ||
227 | 119 | |||
228 | 120 | virtual ~ErrorPreview(); | ||
229 | 121 | |||
230 | 122 | void run(unity::scopes::PreviewReplyProxy const& reply) override; | ||
231 | 123 | |||
232 | 124 | }; | ||
233 | 125 | |||
234 | 111 | class InstallPreview : public Preview | 126 | class InstallPreview : public Preview |
235 | 112 | { | 127 | { |
236 | 113 | protected: | 128 | protected: |
237 | 114 | 129 | ||
238 | === modified file 'scope/click/scope.cpp' | |||
239 | --- scope/click/scope.cpp 2014-02-20 06:27:03 +0000 | |||
240 | +++ scope/click/scope.cpp 2014-02-20 06:27:03 +0000 | |||
241 | @@ -101,10 +101,16 @@ | |||
242 | 101 | 101 | ||
243 | 102 | if (metadata.scope_data().which() != scopes::Variant::Type::Null) { | 102 | if (metadata.scope_data().which() != scopes::Variant::Type::Null) { |
244 | 103 | auto metadict = metadata.scope_data().get_dict(); | 103 | auto metadict = metadata.scope_data().get_dict(); |
246 | 104 | if (metadict.count("download_completed") != 0) { | 104 | |
247 | 105 | if(metadict.count(click::Preview::Actions::DOWNLOAD_FAILED) != 0) { | ||
248 | 106 | return scopes::QueryBase::UPtr{new ErrorPreview(std::string("Download or install failed. Please try again."), | ||
249 | 107 | index, result)}; | ||
250 | 108 | |||
251 | 109 | } else if (metadict.count(click::Preview::Actions::DOWNLOAD_COMPLETED) != 0) { | ||
252 | 105 | Preview* prev = new Preview(result.uri(), index, result); | 110 | Preview* prev = new Preview(result.uri(), index, result); |
253 | 106 | prev->setPreview(click::Preview::Type::INSTALLED); | 111 | prev->setPreview(click::Preview::Type::INSTALLED); |
254 | 107 | return scopes::QueryBase::UPtr{prev}; | 112 | return scopes::QueryBase::UPtr{prev}; |
255 | 113 | |||
256 | 108 | } else if (metadict.count("action_id") != 0 && | 114 | } else if (metadict.count("action_id") != 0 && |
257 | 109 | metadict.count("download_url") != 0) { | 115 | metadict.count("download_url") != 0) { |
258 | 110 | action_id = metadict["action_id"].get_string(); | 116 | action_id = metadict["action_id"].get_string(); |
259 | @@ -132,8 +138,13 @@ | |||
260 | 132 | activation->setHint("action_id", unity::scopes::Variant(action_id)); | 138 | activation->setHint("action_id", unity::scopes::Variant(action_id)); |
261 | 133 | qDebug() << "returning ShowPreview"; | 139 | qDebug() << "returning ShowPreview"; |
262 | 134 | activation->setStatus(unity::scopes::ActivationResponse::Status::ShowPreview); | 140 | activation->setStatus(unity::scopes::ActivationResponse::Status::ShowPreview); |
263 | 141 | |||
264 | 142 | } else if (action_id == click::Preview::Actions::DOWNLOAD_FAILED) { | ||
265 | 143 | activation->setHint(click::Preview::Actions::DOWNLOAD_FAILED, unity::scopes::Variant(true)); | ||
266 | 144 | activation->setStatus(unity::scopes::ActivationResponse::Status::ShowPreview); | ||
267 | 145 | |||
268 | 135 | } else if (action_id == click::Preview::Actions::DOWNLOAD_COMPLETED) { | 146 | } else if (action_id == click::Preview::Actions::DOWNLOAD_COMPLETED) { |
270 | 136 | activation->setHint("download_completed", unity::scopes::Variant(true)); | 147 | activation->setHint(click::Preview::Actions::DOWNLOAD_COMPLETED, unity::scopes::Variant(true)); |
271 | 137 | activation->setStatus(unity::scopes::ActivationResponse::Status::ShowPreview); | 148 | activation->setStatus(unity::scopes::ActivationResponse::Status::ShowPreview); |
272 | 138 | } | 149 | } |
273 | 139 | return scopes::ActivationBase::UPtr(activation); | 150 | return scopes::ActivationBase::UPtr(activation); |
274 | 140 | 151 | ||
275 | === modified file 'scope/tests/download_manager_tool/download_manager_tool.cpp' | |||
276 | --- scope/tests/download_manager_tool/download_manager_tool.cpp 2014-02-18 07:55:54 +0000 | |||
277 | +++ scope/tests/download_manager_tool/download_manager_tool.cpp 2014-02-20 06:27:03 +0000 | |||
278 | @@ -35,6 +35,8 @@ | |||
279 | 35 | 35 | ||
280 | 36 | #include <iostream> | 36 | #include <iostream> |
281 | 37 | 37 | ||
282 | 38 | #include <boost/optional.hpp> | ||
283 | 39 | |||
284 | 38 | #include <download_manager_tool.h> | 40 | #include <download_manager_tool.h> |
285 | 39 | 41 | ||
286 | 40 | DownloadManagerTool::DownloadManagerTool(QObject *parent): | 42 | DownloadManagerTool::DownloadManagerTool(QObject *parent): |
287 | @@ -91,8 +93,13 @@ | |||
288 | 91 | 93 | ||
289 | 92 | QObject::connect(&timer, &QTimer::timeout, [&]() { | 94 | QObject::connect(&timer, &QTimer::timeout, [&]() { |
290 | 93 | downloader.startDownload(std::string(argv[1]), std::string(argv[2]), | 95 | downloader.startDownload(std::string(argv[1]), std::string(argv[2]), |
293 | 94 | [&a] (std::string download_id){ | 96 | [&a] (std::pair<std::string, click::InstallError> arg){ |
294 | 95 | std::cout << " Success, got download ID:" << download_id << std::endl; | 97 | auto error = arg.second; |
295 | 98 | if (error == click::InstallError::NoError) { | ||
296 | 99 | std::cout << " Success, got download ID:" << arg.first << std::endl; | ||
297 | 100 | } else { | ||
298 | 101 | std::cout << " Error:" << arg.first << std::endl; | ||
299 | 102 | } | ||
300 | 96 | a.quit(); | 103 | a.quit(); |
301 | 97 | }); | 104 | }); |
302 | 98 | 105 | ||
303 | 99 | 106 | ||
304 | === modified file 'scope/tests/test_download_manager.cpp' | |||
305 | --- scope/tests/test_download_manager.cpp 2014-02-17 21:32:33 +0000 | |||
306 | +++ scope/tests/test_download_manager.cpp 2014-02-20 06:27:03 +0000 | |||
307 | @@ -184,11 +184,11 @@ | |||
308 | 184 | }); | 184 | }); |
309 | 185 | signalTimer.start(10); | 185 | signalTimer.start(10); |
310 | 186 | } | 186 | } |
311 | 187 | |||
312 | 188 | }; | 187 | }; |
313 | 189 | 188 | ||
314 | 190 | struct DownloadManagerMockClient | 189 | struct DownloadManagerMockClient |
315 | 191 | { | 190 | { |
316 | 191 | MOCK_METHOD0(onCredentialsNotFoundEmitted, void()); | ||
317 | 192 | MOCK_METHOD1(onClickTokenFetchedEmitted, void(QString clickToken)); | 192 | MOCK_METHOD1(onClickTokenFetchedEmitted, void(QString clickToken)); |
318 | 193 | MOCK_METHOD1(onClickTokenFetchErrorEmitted, void(QString errorMessage)); | 193 | MOCK_METHOD1(onClickTokenFetchErrorEmitted, void(QString errorMessage)); |
319 | 194 | MOCK_METHOD1(onDownloadStartedEmitted, void(QString id)); | 194 | MOCK_METHOD1(onDownloadStartedEmitted, void(QString id)); |
320 | @@ -256,6 +256,12 @@ | |||
321 | 256 | 256 | ||
322 | 257 | DownloadManagerMockClient mockDownloadManagerClient; | 257 | DownloadManagerMockClient mockDownloadManagerClient; |
323 | 258 | 258 | ||
324 | 259 | QObject::connect(&dm, &click::DownloadManager::credentialsNotFound, | ||
325 | 260 | [&mockDownloadManagerClient]() | ||
326 | 261 | { | ||
327 | 262 | mockDownloadManagerClient.onCredentialsNotFoundEmitted(); | ||
328 | 263 | }); | ||
329 | 264 | |||
330 | 259 | QObject::connect(&dm, &click::DownloadManager::clickTokenFetchError, | 265 | QObject::connect(&dm, &click::DownloadManager::clickTokenFetchError, |
331 | 260 | [&mockDownloadManagerClient](const QString& error) | 266 | [&mockDownloadManagerClient](const QString& error) |
332 | 261 | { | 267 | { |
333 | @@ -282,12 +288,23 @@ | |||
334 | 282 | 288 | ||
335 | 283 | } else { | 289 | } else { |
336 | 284 | 290 | ||
343 | 285 | EXPECT_CALL(mockDownloadManagerClient, onClickTokenFetchErrorEmitted(_)) | 291 | if (p.credsFound) { |
344 | 286 | .Times(1) | 292 | |
345 | 287 | .WillOnce( | 293 | EXPECT_CALL(mockDownloadManagerClient, onClickTokenFetchErrorEmitted(_)) |
346 | 288 | InvokeWithoutArgs( | 294 | .Times(1) |
347 | 289 | this, | 295 | .WillOnce( |
348 | 290 | &DISABLED_DownloadManagerCredsNetworkTest::Quit)); | 296 | InvokeWithoutArgs( |
349 | 297 | this, | ||
350 | 298 | &DISABLED_DownloadManagerCredsNetworkTest::Quit)); | ||
351 | 299 | } else { | ||
352 | 300 | |||
353 | 301 | EXPECT_CALL(mockDownloadManagerClient, onCredentialsNotFoundEmitted()) | ||
354 | 302 | .Times(1) | ||
355 | 303 | .WillOnce( | ||
356 | 304 | InvokeWithoutArgs( | ||
357 | 305 | this, | ||
358 | 306 | &DISABLED_DownloadManagerCredsNetworkTest::Quit)); | ||
359 | 307 | } | ||
360 | 291 | 308 | ||
361 | 292 | EXPECT_CALL(mockDownloadManagerClient, onClickTokenFetchedEmitted(_)).Times(0); | 309 | EXPECT_CALL(mockDownloadManagerClient, onClickTokenFetchedEmitted(_)).Times(0); |
362 | 293 | 310 | ||
363 | @@ -326,12 +343,12 @@ | |||
364 | 326 | { | 343 | { |
365 | 327 | auto commandList = arg.getMetadata()["post-download-command"].toStringList(); | 344 | auto commandList = arg.getMetadata()["post-download-command"].toStringList(); |
366 | 328 | return arg.getUrl() == TEST_URL | 345 | return arg.getUrl() == TEST_URL |
368 | 329 | && arg.getHash() == "" | 346 | && arg.getHash() == "" |
369 | 330 | && arg.getAlgorithm() == "" | 347 | && arg.getAlgorithm() == "" |
370 | 331 | && arg.getMetadata()["app_id"] == QVariant(TEST_APP_ID) | 348 | && arg.getMetadata()["app_id"] == QVariant(TEST_APP_ID) |
371 | 332 | && commandList[0] == "/bin/sh" | 349 | && commandList[0] == "/bin/sh" |
372 | 333 | && commandList[1] == "-c" | 350 | && commandList[1] == "-c" |
374 | 334 | && commandList[3] == "$files" | 351 | && commandList[3] == "$file" |
375 | 335 | && arg.getHeaders()["X-Click-Token"] == TEST_CLICK_TOKEN_VALUE; | 352 | && arg.getHeaders()["X-Click-Token"] == TEST_CLICK_TOKEN_VALUE; |
376 | 336 | } | 353 | } |
377 | 337 | 354 | ||
378 | @@ -385,14 +402,14 @@ | |||
379 | 385 | }); | 402 | }); |
380 | 386 | } | 403 | } |
381 | 387 | 404 | ||
383 | 388 | EXPECT_CALL(*mockSystemDownloadManager, | 405 | EXPECT_CALL(*mockSystemDownloadManager, |
384 | 389 | createDownload(DownloadStructIsValid())).Times(1).WillOnce(InvokeWithoutArgs(downloadCreatedSignalFunc)); | 406 | createDownload(DownloadStructIsValid())).Times(1).WillOnce(InvokeWithoutArgs(downloadCreatedSignalFunc)); |
386 | 390 | } | 407 | } |
387 | 391 | 408 | ||
388 | 392 | EXPECT_CALL(*mockCredentialsService, getCredentials()) | 409 | EXPECT_CALL(*mockCredentialsService, getCredentials()) |
389 | 393 | .Times(1).WillOnce(InvokeWithoutArgs(clickTokenSignalFunc)); | 410 | .Times(1).WillOnce(InvokeWithoutArgs(clickTokenSignalFunc)); |
390 | 394 | 411 | ||
392 | 395 | 412 | ||
393 | 396 | DownloadManagerMockClient mockDownloadManagerClient; | 413 | DownloadManagerMockClient mockDownloadManagerClient; |
394 | 397 | 414 | ||
395 | 398 | QObject::connect(&dm, &click::DownloadManager::downloadError, | 415 | QObject::connect(&dm, &click::DownloadManager::downloadError, |
396 | @@ -418,7 +435,7 @@ | |||
397 | 418 | &DISABLED_DownloadManagerStartDownloadTest::Quit)); | 435 | &DISABLED_DownloadManagerStartDownloadTest::Quit)); |
398 | 419 | 436 | ||
399 | 420 | EXPECT_CALL(mockDownloadManagerClient, onDownloadErrorEmitted(_)).Times(0); | 437 | EXPECT_CALL(mockDownloadManagerClient, onDownloadErrorEmitted(_)).Times(0); |
401 | 421 | 438 | ||
402 | 422 | // when https://bugs.launchpad.net/ubuntu-download-manager/+bug/1278789 | 439 | // when https://bugs.launchpad.net/ubuntu-download-manager/+bug/1278789 |
403 | 423 | // is fixed, we can add this assertion: | 440 | // is fixed, we can add this assertion: |
404 | 424 | // EXPECT_CALL(successfulDownload, start()).Times(1); | 441 | // EXPECT_CALL(successfulDownload, start()).Times(1); |
405 | @@ -435,7 +452,7 @@ | |||
406 | 435 | EXPECT_CALL(mockDownloadManagerClient, onDownloadStartedEmitted(_)).Times(0); | 452 | EXPECT_CALL(mockDownloadManagerClient, onDownloadStartedEmitted(_)).Times(0); |
407 | 436 | 453 | ||
408 | 437 | } | 454 | } |
410 | 438 | 455 | ||
411 | 439 | QTimer timer; | 456 | QTimer timer; |
412 | 440 | timer.setSingleShot(true); | 457 | timer.setSingleShot(true); |
413 | 441 | QObject::connect(&timer, &QTimer::timeout, [&dm]() { | 458 | QObject::connect(&timer, &QTimer::timeout, [&dm]() { |
PASSED: Continuous integration, rev:153 jenkins. qa.ubuntu. com/job/ unity-scope- click-ci/ 326/ jenkins. qa.ubuntu. com/job/ unity-scope- click-trusty- amd64-ci/ 227 jenkins. qa.ubuntu. com/job/ unity-scope- click-trusty- armhf-ci/ 224 jenkins. qa.ubuntu. com/job/ unity-scope- click-trusty- armhf-ci/ 224/artifact/ work/output/ *zip*/output. zip
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/unity- scope-click- ci/326/ rebuild
http://