Merge lp:~alecu/unity-scope-click/preview-fixes into lp:unity-scope-click/devel
- preview-fixes
- Merge into devel
Status: | Merged |
---|---|
Approved by: | Paweł Stołowski |
Approved revision: | 354 |
Merged at revision: | 354 |
Proposed branch: | lp:~alecu/unity-scope-click/preview-fixes |
Merge into: | lp:unity-scope-click/devel |
Diff against target: |
264 lines (+158/-11) 4 files modified
libclickscope/click/preview.cpp (+20/-11) libclickscope/click/preview.h (+6/-0) libclickscope/tests/CMakeLists.txt (+1/-0) libclickscope/tests/test_preview.cpp (+131/-0) |
To merge this branch: | bzr merge lp:~alecu/unity-scope-click/preview-fixes |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot (community) | continuous-integration | Approve | |
Paweł Stołowski (community) | Approve | ||
dobey (community) | Approve | ||
Review via email: mp+227730@code.launchpad.net |
Commit message
Change the order of screenshot and header widgets; initial tests for previews
Description of the change
- 353. By Alejandro J. Cura
-
A comment about PreviewReply being hard to use in tests
dobey (dobey) : | # |
PS Jenkins bot (ps-jenkins) wrote : | # |
Paweł Stołowski (stolowski) wrote : | # |
233 +class FakePreviewReply : public scopes:
234 +{
235 +public:
236 + // TODO: make PreviewReply more easily mockable in scopes-api
Scopes API has MockPreviewReply in the testing namespace, is there any issue with it? (if so, please open a bug against scopes api). I think it does exactly what you need and implement with with this FakePreviewRepl
- 354. By Alejandro J. Cura
-
Use MockPreviewReply instead
Paweł Stołowski (stolowski) wrote : | # |
Looks good and works fine. +1
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:354
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Preview Diff
1 | === modified file 'libclickscope/click/preview.cpp' | |||
2 | --- libclickscope/click/preview.cpp 2014-07-16 21:49:11 +0000 | |||
3 | +++ libclickscope/click/preview.cpp 2014-07-22 16:06:10 +0000 | |||
4 | @@ -177,6 +177,16 @@ | |||
5 | 177 | { | 177 | { |
6 | 178 | } | 178 | } |
7 | 179 | 179 | ||
8 | 180 | void PreviewStrategy::pushPackagePreviewWidgets(const unity::scopes::PreviewReplyProxy &reply, | ||
9 | 181 | const PackageDetails &details, | ||
10 | 182 | const scopes::PreviewWidgetList& button_area_widgets) | ||
11 | 183 | { | ||
12 | 184 | reply->push(headerWidgets(details)); | ||
13 | 185 | reply->push(button_area_widgets); | ||
14 | 186 | reply->push(screenshotsWidgets(details)); | ||
15 | 187 | reply->push(descriptionWidgets(details)); | ||
16 | 188 | } | ||
17 | 189 | |||
18 | 180 | PreviewStrategy::~PreviewStrategy() | 190 | PreviewStrategy::~PreviewStrategy() |
19 | 181 | { | 191 | { |
20 | 182 | } | 192 | } |
21 | @@ -235,10 +245,9 @@ | |||
22 | 235 | } | 245 | } |
23 | 236 | } | 246 | } |
24 | 237 | 247 | ||
26 | 238 | scopes::PreviewWidgetList PreviewStrategy::headerWidgets(const click::PackageDetails& details) | 248 | scopes::PreviewWidgetList PreviewStrategy::screenshotsWidgets(const click::PackageDetails& details) |
27 | 239 | { | 249 | { |
28 | 240 | scopes::PreviewWidgetList widgets; | 250 | scopes::PreviewWidgetList widgets; |
29 | 241 | |||
30 | 242 | bool has_screenshots = !details.main_screenshot_url.empty() || !details.more_screenshots_urls.empty(); | 251 | bool has_screenshots = !details.main_screenshot_url.empty() || !details.more_screenshots_urls.empty(); |
31 | 243 | 252 | ||
32 | 244 | if (has_screenshots) | 253 | if (has_screenshots) |
33 | @@ -259,6 +268,12 @@ | |||
34 | 259 | gallery.add_attribute_value("sources", scopes::Variant(arr)); | 268 | gallery.add_attribute_value("sources", scopes::Variant(arr)); |
35 | 260 | widgets.push_back(gallery); | 269 | widgets.push_back(gallery); |
36 | 261 | } | 270 | } |
37 | 271 | return widgets; | ||
38 | 272 | } | ||
39 | 273 | |||
40 | 274 | scopes::PreviewWidgetList PreviewStrategy::headerWidgets(const click::PackageDetails& details) | ||
41 | 275 | { | ||
42 | 276 | scopes::PreviewWidgetList widgets; | ||
43 | 262 | 277 | ||
44 | 263 | scopes::PreviewWidget header("hdr", "header"); | 278 | scopes::PreviewWidget header("hdr", "header"); |
45 | 264 | header.add_attribute_value("title", scopes::Variant(details.package.title)); | 279 | header.add_attribute_value("title", scopes::Variant(details.package.title)); |
46 | @@ -423,9 +438,7 @@ | |||
47 | 423 | qDebug() << "Successfully created UDM Download."; | 438 | qDebug() << "Successfully created UDM Download."; |
48 | 424 | populateDetails([this, reply, object_path](const PackageDetails &details) { | 439 | populateDetails([this, reply, object_path](const PackageDetails &details) { |
49 | 425 | store_department(details); | 440 | store_department(details); |
53 | 426 | reply->push(headerWidgets(details)); | 441 | pushPackagePreviewWidgets(reply, details, progressBarWidget(object_path)); |
51 | 427 | reply->push(progressBarWidget(object_path)); | ||
52 | 428 | reply->push(descriptionWidgets(details)); | ||
54 | 429 | startLauncherAnimation(details); | 442 | startLauncherAnimation(details); |
55 | 430 | }, | 443 | }, |
56 | 431 | [this, reply](const ReviewList& reviewlist, | 444 | [this, reply](const ReviewList& reviewlist, |
57 | @@ -526,9 +539,7 @@ | |||
58 | 526 | getApplicationUri(manifest, [this, reply, manifest, app_name, &review](const std::string& uri) { | 539 | getApplicationUri(manifest, [this, reply, manifest, app_name, &review](const std::string& uri) { |
59 | 527 | populateDetails([this, reply, uri, manifest, app_name, &review](const PackageDetails &details){ | 540 | populateDetails([this, reply, uri, manifest, app_name, &review](const PackageDetails &details){ |
60 | 528 | store_department(details); | 541 | store_department(details); |
64 | 529 | reply->push(headerWidgets(details)); | 542 | pushPackagePreviewWidgets(reply, details, createButtons(uri, manifest)); |
62 | 530 | reply->push(createButtons(uri, manifest)); | ||
63 | 531 | reply->push(descriptionWidgets(details)); | ||
65 | 532 | 543 | ||
66 | 533 | if (review.rating == 0 && manifest.removable) { | 544 | if (review.rating == 0 && manifest.removable) { |
67 | 534 | scopes::PreviewWidgetList review_input; | 545 | scopes::PreviewWidgetList review_input; |
68 | @@ -743,9 +754,7 @@ | |||
69 | 743 | 754 | ||
70 | 744 | populateDetails([this, reply](const PackageDetails &details){ | 755 | populateDetails([this, reply](const PackageDetails &details){ |
71 | 745 | store_department(details); | 756 | store_department(details); |
75 | 746 | reply->push(headerWidgets(details)); | 757 | pushPackagePreviewWidgets(reply, details, uninstalledActionButtonWidgets(details)); |
73 | 747 | reply->push(uninstalledActionButtonWidgets(details)); | ||
74 | 748 | reply->push(descriptionWidgets(details)); | ||
76 | 749 | }, | 758 | }, |
77 | 750 | [this, reply](const ReviewList& reviewlist, | 759 | [this, reply](const ReviewList& reviewlist, |
78 | 751 | click::Reviews::Error error) { | 760 | click::Reviews::Error error) { |
79 | 752 | 761 | ||
80 | === modified file 'libclickscope/click/preview.h' | |||
81 | --- libclickscope/click/preview.h 2014-07-14 15:35:10 +0000 | |||
82 | +++ libclickscope/click/preview.h 2014-07-22 16:06:10 +0000 | |||
83 | @@ -54,6 +54,7 @@ | |||
84 | 54 | class DepartmentUpdater | 54 | class DepartmentUpdater |
85 | 55 | { | 55 | { |
86 | 56 | protected: | 56 | protected: |
87 | 57 | DepartmentUpdater() = default; | ||
88 | 57 | DepartmentUpdater(const std::shared_ptr<click::DepartmentsDb>& depts); | 58 | DepartmentUpdater(const std::shared_ptr<click::DepartmentsDb>& depts); |
89 | 58 | virtual ~DepartmentUpdater() = default; | 59 | virtual ~DepartmentUpdater() = default; |
90 | 59 | void store_department(const PackageDetails& pkg); | 60 | void store_department(const PackageDetails& pkg); |
91 | @@ -120,6 +121,7 @@ | |||
92 | 120 | std::function<void(const click::ReviewList&, | 121 | std::function<void(const click::ReviewList&, |
93 | 121 | click::Reviews::Error)> reviews_callback); | 122 | click::Reviews::Error)> reviews_callback); |
94 | 122 | virtual scopes::PreviewWidgetList headerWidgets(const PackageDetails &details); | 123 | virtual scopes::PreviewWidgetList headerWidgets(const PackageDetails &details); |
95 | 124 | virtual scopes::PreviewWidgetList screenshotsWidgets(const PackageDetails &details); | ||
96 | 123 | virtual scopes::PreviewWidgetList descriptionWidgets(const PackageDetails &details); | 125 | virtual scopes::PreviewWidgetList descriptionWidgets(const PackageDetails &details); |
97 | 124 | virtual scopes::PreviewWidgetList reviewsWidgets(const click::ReviewList &reviewlist); | 126 | virtual scopes::PreviewWidgetList reviewsWidgets(const click::ReviewList &reviewlist); |
98 | 125 | virtual scopes::PreviewWidgetList downloadErrorWidgets(); | 127 | virtual scopes::PreviewWidgetList downloadErrorWidgets(); |
99 | @@ -129,6 +131,9 @@ | |||
100 | 129 | const scopes::Variant& action_id, | 131 | const scopes::Variant& action_id, |
101 | 130 | const scopes::Variant& action_label, | 132 | const scopes::Variant& action_label, |
102 | 131 | const scopes::Variant& action_uri = scopes::Variant::null()); | 133 | const scopes::Variant& action_uri = scopes::Variant::null()); |
103 | 134 | virtual void pushPackagePreviewWidgets(const unity::scopes::PreviewReplyProxy &reply, | ||
104 | 135 | const PackageDetails& details, | ||
105 | 136 | const scopes::PreviewWidgetList& button_area_widgets); | ||
106 | 132 | scopes::Result result; | 137 | scopes::Result result; |
107 | 133 | QSharedPointer<click::web::Client> client; | 138 | QSharedPointer<click::web::Client> client; |
108 | 134 | QSharedPointer<click::Index> index; | 139 | QSharedPointer<click::Index> index; |
109 | @@ -151,6 +156,7 @@ | |||
110 | 151 | class InstallingPreview : public PreviewStrategy, public DepartmentUpdater | 156 | class InstallingPreview : public PreviewStrategy, public DepartmentUpdater |
111 | 152 | { | 157 | { |
112 | 153 | public: | 158 | public: |
113 | 159 | InstallingPreview(const unity::scopes::Result& result) : PreviewStrategy(result) {} | ||
114 | 154 | InstallingPreview(std::string const& download_url, | 160 | InstallingPreview(std::string const& download_url, |
115 | 155 | const unity::scopes::Result& result, | 161 | const unity::scopes::Result& result, |
116 | 156 | const QSharedPointer<click::web::Client>& client, | 162 | const QSharedPointer<click::web::Client>& client, |
117 | 157 | 163 | ||
118 | === modified file 'libclickscope/tests/CMakeLists.txt' | |||
119 | --- libclickscope/tests/CMakeLists.txt 2014-07-14 08:49:19 +0000 | |||
120 | +++ libclickscope/tests/CMakeLists.txt 2014-07-22 16:06:10 +0000 | |||
121 | @@ -30,6 +30,7 @@ | |||
122 | 30 | test_index.cpp | 30 | test_index.cpp |
123 | 31 | test_interface.cpp | 31 | test_interface.cpp |
124 | 32 | test_package.cpp | 32 | test_package.cpp |
125 | 33 | test_preview.cpp | ||
126 | 33 | test_reviews.cpp | 34 | test_reviews.cpp |
127 | 34 | test_smartconnect.cpp | 35 | test_smartconnect.cpp |
128 | 35 | test_webclient.cpp | 36 | test_webclient.cpp |
129 | 36 | 37 | ||
130 | === added file 'libclickscope/tests/test_preview.cpp' | |||
131 | --- libclickscope/tests/test_preview.cpp 1970-01-01 00:00:00 +0000 | |||
132 | +++ libclickscope/tests/test_preview.cpp 2014-07-22 16:06:10 +0000 | |||
133 | @@ -0,0 +1,131 @@ | |||
134 | 1 | /* | ||
135 | 2 | * Copyright (C) 2014 Canonical Ltd. | ||
136 | 3 | * | ||
137 | 4 | * This program is free software: you can redistribute it and/or modify it | ||
138 | 5 | * under the terms of the GNU General Public License version 3, as published | ||
139 | 6 | * by the Free Software Foundation. | ||
140 | 7 | * | ||
141 | 8 | * This program is distributed in the hope that it will be useful, but | ||
142 | 9 | * WITHOUT ANY WARRANTY; without even the implied warranties of | ||
143 | 10 | * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR | ||
144 | 11 | * PURPOSE. See the GNU General Public License for more details. | ||
145 | 12 | * | ||
146 | 13 | * You should have received a copy of the GNU General Public License along | ||
147 | 14 | * with this program. If not, see <http://www.gnu.org/licenses/>. | ||
148 | 15 | * | ||
149 | 16 | * In addition, as a special exception, the copyright holders give | ||
150 | 17 | * permission to link the code of portions of this program with the | ||
151 | 18 | * OpenSSL library under certain conditions as described in each | ||
152 | 19 | * individual source file, and distribute linked combinations | ||
153 | 20 | * including the two. | ||
154 | 21 | * You must obey the GNU General Public License in all respects | ||
155 | 22 | * for all of the code used other than OpenSSL. If you modify | ||
156 | 23 | * file(s) with this exception, you may extend this exception to your | ||
157 | 24 | * version of the file(s), but you are not obligated to do so. If you | ||
158 | 25 | * do not wish to do so, delete this exception statement from your | ||
159 | 26 | * version. If you delete this exception statement from all source | ||
160 | 27 | * files in the program, then also delete it here. | ||
161 | 28 | */ | ||
162 | 29 | |||
163 | 30 | #include <unity/scopes/testing/MockPreviewReply.h> | ||
164 | 31 | |||
165 | 32 | #include <gtest/gtest.h> | ||
166 | 33 | #include <click/preview.h> | ||
167 | 34 | |||
168 | 35 | using namespace ::testing; | ||
169 | 36 | using namespace unity::scopes; | ||
170 | 37 | |||
171 | 38 | class FakeResult : public Result | ||
172 | 39 | { | ||
173 | 40 | public: | ||
174 | 41 | FakeResult(VariantMap& vm) : Result(vm) | ||
175 | 42 | { | ||
176 | 43 | } | ||
177 | 44 | |||
178 | 45 | }; | ||
179 | 46 | |||
180 | 47 | class FakePreview : public click::PreviewStrategy | ||
181 | 48 | { | ||
182 | 49 | public: | ||
183 | 50 | FakePreview(Result& result) : click::PreviewStrategy::PreviewStrategy(result) | ||
184 | 51 | { | ||
185 | 52 | |||
186 | 53 | } | ||
187 | 54 | |||
188 | 55 | void run(const PreviewReplyProxy &/*reply*/) | ||
189 | 56 | { | ||
190 | 57 | |||
191 | 58 | } | ||
192 | 59 | using click::PreviewStrategy::screenshotsWidgets; | ||
193 | 60 | }; | ||
194 | 61 | |||
195 | 62 | class PreviewsBaseTest : public Test | ||
196 | 63 | { | ||
197 | 64 | public: | ||
198 | 65 | VariantMap vm; | ||
199 | 66 | VariantMap internal; | ||
200 | 67 | VariantMap attrs; | ||
201 | 68 | |||
202 | 69 | PreviewsBaseTest() | ||
203 | 70 | { | ||
204 | 71 | attrs["uri"] = "fake://result"; | ||
205 | 72 | vm["internal"] = internal; | ||
206 | 73 | vm["attrs"] = attrs; | ||
207 | 74 | } | ||
208 | 75 | }; | ||
209 | 76 | |||
210 | 77 | class PreviewStrategyTest : public PreviewsBaseTest | ||
211 | 78 | { | ||
212 | 79 | |||
213 | 80 | }; | ||
214 | 81 | |||
215 | 82 | TEST_F(PreviewStrategyTest, testScreenshotsWidget) | ||
216 | 83 | { | ||
217 | 84 | FakeResult result{vm}; | ||
218 | 85 | FakePreview preview{result}; | ||
219 | 86 | click::PackageDetails details; | ||
220 | 87 | details.main_screenshot_url = "sshot1"; | ||
221 | 88 | details.more_screenshots_urls = {"sshot2", "sshot3"}; | ||
222 | 89 | |||
223 | 90 | auto widgets = preview.screenshotsWidgets(details); | ||
224 | 91 | ASSERT_EQ(widgets.size(), 1); | ||
225 | 92 | auto first_widget = widgets.front(); | ||
226 | 93 | auto attributes = first_widget.attribute_values(); | ||
227 | 94 | auto sources = attributes["sources"].get_array(); | ||
228 | 95 | ASSERT_EQ(sources[0].get_string(), "sshot1"); | ||
229 | 96 | ASSERT_EQ(sources[1].get_string(), "sshot2"); | ||
230 | 97 | ASSERT_EQ(sources[2].get_string(), "sshot3"); | ||
231 | 98 | } | ||
232 | 99 | |||
233 | 100 | class FakePreviewStrategy : public click::PreviewStrategy | ||
234 | 101 | { | ||
235 | 102 | public: | ||
236 | 103 | FakePreviewStrategy(const unity::scopes::Result& result) : PreviewStrategy(result) {} | ||
237 | 104 | using click::PreviewStrategy::pushPackagePreviewWidgets; | ||
238 | 105 | std::vector<std::string> call_order; | ||
239 | 106 | virtual void run(unity::scopes::PreviewReplyProxy const& /*reply*/) | ||
240 | 107 | { | ||
241 | 108 | } | ||
242 | 109 | |||
243 | 110 | virtual unity::scopes::PreviewWidgetList screenshotsWidgets (const click::PackageDetails &/*details*/) { | ||
244 | 111 | call_order.push_back("screenshots"); | ||
245 | 112 | return {}; | ||
246 | 113 | } | ||
247 | 114 | virtual unity::scopes::PreviewWidgetList headerWidgets (const click::PackageDetails &/*details*/) { | ||
248 | 115 | call_order.push_back("header"); | ||
249 | 116 | return {}; | ||
250 | 117 | } | ||
251 | 118 | }; | ||
252 | 119 | |||
253 | 120 | TEST_F(PreviewStrategyTest, testScreenshotsPushedAfterHeader) | ||
254 | 121 | { | ||
255 | 122 | FakeResult result{vm}; | ||
256 | 123 | unity::scopes::testing::MockPreviewReply reply; | ||
257 | 124 | std::shared_ptr<unity::scopes::testing::MockPreviewReply> replyptr{&reply, [](unity::scopes::testing::MockPreviewReply*){}}; | ||
258 | 125 | click::PackageDetails details; | ||
259 | 126 | |||
260 | 127 | FakePreviewStrategy preview{result}; | ||
261 | 128 | preview.pushPackagePreviewWidgets(replyptr, details, {}); | ||
262 | 129 | std::vector<std::string> expected_order {"header", "screenshots"}; | ||
263 | 130 | ASSERT_EQ(expected_order, preview.call_order); | ||
264 | 131 | } |
FAILED: Continuous integration, rev:353 jenkins. qa.ubuntu. com/job/ unity-team- unity-scope- click-devel- ci/229/ jenkins. qa.ubuntu. com/job/ unity-team- unity-scope- click-devel- utopic- amd64-ci/ 204/console jenkins. qa.ubuntu. com/job/ unity-team- unity-scope- click-devel- utopic- armhf-ci/ 203/console jenkins. qa.ubuntu. com/job/ unity-team- unity-scope- click-devel- utopic- i386-ci/ 203/console
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/unity- team-unity- scope-click- devel-ci/ 229/rebuild
http://