Merge lp:~elachuni/software-center/reviews-tests into lp:software-center
- reviews-tests
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 2689 |
Proposed branch: | lp:~elachuni/software-center/reviews-tests |
Merge into: | lp:software-center |
Diff against target: |
311 lines (+180/-37) 2 files modified
softwarecenter/ui/gtk3/review_gui_helper.py (+5/-31) test/gtk3/test_reviews.py (+175/-6) |
To merge this branch: | bzr merge lp:~elachuni/software-center/reviews-tests |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Michael Nelson | Approve | ||
Review via email: mp+89677@code.launchpad.net |
Commit message
Description of the change
This branch includes several tests for code in the softwarecenter.
- 2686. By Anthony Lenton
-
Merged in latest changes from trunk.
Gary Lasker (gary-lasker) wrote : | # |
Hello Anthony! I merged your branch, thanks! Note that I had to make some small tweaks (a path update) to prevent import errors. However, even with this, I am finding that the tests hang with the dialog not closing out at the end (running the case as 'cd test/;PYTHONPATH=. python gtk3/test_
Please take a look at my tweaks branch here for the changes that I made before merging:
lp:~gary-lasker/software-center/achuni-reviews-tests-tweaks
Not sure what's causing the hang, could be something obvious that's just eluding me. Maybe you have an insight. Please take a look and then we can get this test re-enabled.
Preview Diff
1 | === modified file 'softwarecenter/ui/gtk3/review_gui_helper.py' |
2 | --- softwarecenter/ui/gtk3/review_gui_helper.py 2012-01-19 09:23:54 +0000 |
3 | +++ softwarecenter/ui/gtk3/review_gui_helper.py 2012-01-23 13:29:23 +0000 |
4 | @@ -200,12 +200,6 @@ |
5 | self.pending_delete.empty()): |
6 | return |
7 | |
8 | - # usefulness |
9 | - def queue_usefulness(self, usefulness): |
10 | - """ queue a new usefulness report for sending to LP """ |
11 | - logging.debug("queue_usefulness %s %s %s" % usefulness) |
12 | - self.pending_usefulness.put(usefulness) |
13 | - |
14 | def _submit_usefulness_if_pending(self): |
15 | """ the actual usefulness function """ |
16 | while not self.pending_usefulness.empty(): |
17 | @@ -224,12 +218,6 @@ |
18 | self._write_exception_html_log_if_needed(e) |
19 | self._transmit_state = TRANSMIT_STATE_ERROR |
20 | self.pending_usefulness.task_done() |
21 | - |
22 | - #modify |
23 | - def queue_modify(self, modification): |
24 | - """ queue a new review modification request for sending to LP """ |
25 | - logging.debug("queue_modify %s %s" % modification) |
26 | - self.pending_modify.put(modification) |
27 | |
28 | def _submit_modify_if_pending(self): |
29 | """ the actual modify function """ |
30 | @@ -254,13 +242,7 @@ |
31 | self._transmit_state = TRANSMIT_STATE_ERROR |
32 | self._transmit_error_str = err_str |
33 | self.pending_modify.task_done() |
34 | - |
35 | - #delete |
36 | - def queue_delete(self, deletion): |
37 | - """ queue a new deletion request for sending to LP """ |
38 | - logging.debug("queue_delete review id: %s" % deletion) |
39 | - self.pending_delete.put(deletion) |
40 | - |
41 | + |
42 | def _submit_delete_if_pending(self): |
43 | """ the actual deletion """ |
44 | while not self.pending_delete.empty(): |
45 | @@ -278,12 +260,6 @@ |
46 | self._transmit_state = TRANSMIT_STATE_ERROR |
47 | self.pending_delete.task_done() |
48 | |
49 | - # reports |
50 | - def queue_report(self, report): |
51 | - """ queue a new report for sending to LP """ |
52 | - logging.debug("queue_report %s %s %s" % report) |
53 | - self.pending_reports.put(report) |
54 | - |
55 | def _submit_reports_if_pending(self): |
56 | """ the actual report function """ |
57 | while not self.pending_reports.empty(): |
58 | @@ -532,7 +508,7 @@ |
59 | |
60 | def _change_status(self, type, message): |
61 | """method to separate the updating of status icon/spinner and message in the submit review window, |
62 | - takes a type (progress, fail, success) as a string and a message string then updates status area accordingly""" |
63 | + takes a type (progress, fail, success, clear, warning) as a string and a message string then updates status area accordingly""" |
64 | self._clear_status_imagery() |
65 | self.label_transmit_status.set_text("") |
66 | |
67 | @@ -781,13 +757,13 @@ |
68 | if self.star_rating.get_rating() != self.orig_star_rating: |
69 | return False |
70 | #compare summary text |
71 | - if self.review_summary_entry.get_text() != self.orig_summary_text: |
72 | + if self.review_summary_entry.get_text().decode('utf-8') != self.orig_summary_text: |
73 | return False |
74 | #compare review text |
75 | if self.review_buffer.get_text( |
76 | self.review_buffer.get_start_iter(), |
77 | self.review_buffer.get_end_iter(), |
78 | - include_hidden_chars=False) != self.orig_review_text: |
79 | + include_hidden_chars=False).decode('utf-8') != self.orig_review_text: |
80 | return False |
81 | return True |
82 | |
83 | @@ -822,9 +798,7 @@ |
84 | markup = '<span fgcolor="#%s">%s</span>' |
85 | if curr > cmax: |
86 | return markup % (empty_col, str(cmax-curr)) |
87 | - elif curr < cmin: |
88 | - return markup % (full_col, str(cmax-curr)) |
89 | - elif cmax == cmin: #saves division by 0 later if same value was passed as min and max |
90 | + elif curr <= cmin: #saves division by 0 later if same value was passed as min and max |
91 | return markup % (full_col, str(cmax-curr)) |
92 | else: |
93 | #distance between min and max values to fade colours |
94 | |
95 | === renamed file 'test/test_reviews.py' => 'test/gtk3/test_reviews.py' |
96 | --- test/test_reviews.py 2012-01-19 09:23:54 +0000 |
97 | +++ test/gtk3/test_reviews.py 2012-01-23 13:29:23 +0000 |
98 | @@ -8,18 +8,22 @@ |
99 | from testutils import setup_test_env |
100 | setup_test_env() |
101 | from gettext import gettext as _ |
102 | +from mock import Mock, patch |
103 | |
104 | +from softwarecenter.backend.piston.rnrclient_pristine import ReviewDetails |
105 | from softwarecenter.backend.reviews.rnr import ( |
106 | ReviewLoaderSpawningRNRClient as ReviewLoader) |
107 | from softwarecenter.testutils import get_test_pkg_info, get_test_db |
108 | -from softwarecenter.ui.gtk3.review_gui_helper import SubmitReviewsApp |
109 | - |
110 | +from softwarecenter.ui.gtk3.review_gui_helper import ( |
111 | + TRANSMIT_STATE_DONE, |
112 | + GRatingsAndReviews, |
113 | + SubmitReviewsApp, |
114 | + ) |
115 | +from time import sleep |
116 | |
117 | class TestReviewLoader(unittest.TestCase): |
118 | - |
119 | - def setUp(self): |
120 | - self.cache = get_test_pkg_info() |
121 | - self.db = get_test_db() |
122 | + cache = get_test_pkg_info() |
123 | + db = get_test_db() |
124 | |
125 | def _review_stats_ready_callback(self, review_stats): |
126 | self._stats_ready = True |
127 | @@ -51,6 +55,7 @@ |
128 | version=None, action='modify', review_id=10000) |
129 | # monkey patch away login to avoid that we actually login |
130 | # and the UI changes because of that |
131 | + |
132 | review_app.login = lambda: True |
133 | |
134 | # run the main app |
135 | @@ -66,12 +71,176 @@ |
136 | review_app.review_label.get_label()) |
137 | review_app.submit_window.hide() |
138 | |
139 | + def test_get_fade_colour_markup(self): |
140 | + review_app = SubmitReviewsApp(datadir="../data", app=None, |
141 | + parent_xid='', iconname='accessories-calculator', origin=None, |
142 | + version=None, action='nothing') |
143 | + cases = ( |
144 | + (('006000', '00A000', 40, 60, 50), ('008000', 10)), |
145 | + (('000000', 'FFFFFF', 40, 40, 40), ('000000', 0)), |
146 | + (('000000', '808080', 100, 400, 40), ('000000', 360)), |
147 | + (('000000', '808080', 100, 400, 1000), ('808080', -600)), |
148 | + (('123456', '5294D6', 10, 90, 70), ('427CB6', 20)), |
149 | + ) |
150 | + for args, return_value in cases: |
151 | + result = review_app._get_fade_colour_markup(*args) |
152 | + expected = '<span fgcolor="#%s">%s</span>' % return_value |
153 | + self.assertEqual(expected, result) |
154 | + |
155 | + def test_modify_review_is_the_same_supports_unicode(self): |
156 | + review_app = SubmitReviewsApp(datadir="../data", app=None, |
157 | + parent_xid='', iconname='accessories-calculator', origin=None, |
158 | + version=None, action='modify', review_id=10000) |
159 | + self.assertTrue(review_app._modify_review_is_the_same()) |
160 | + |
161 | + cases = ('', 'e', ')!') |
162 | + for case in cases: |
163 | + modified = review_app.orig_summary_text[:-1] + case |
164 | + review_app.review_summary_entry.set_text(modified) |
165 | + self.assertFalse(review_app._modify_review_is_the_same()) |
166 | + |
167 | + review_app.review_summary_entry.set_text(review_app.orig_summary_text) |
168 | + for case in cases: |
169 | + modified = review_app.orig_review_text[:-1] + case |
170 | + review_app.review_buffer.set_text(modified) |
171 | + self.assertFalse(review_app._modify_review_is_the_same()) |
172 | + |
173 | + def test_change_status(self): |
174 | + review_app = SubmitReviewsApp(datadir="../data", app=None, |
175 | + parent_xid='', iconname='accessories-calculator', origin=None, |
176 | + version=None, action='nothing') |
177 | + msg = 'Something completely different' |
178 | + cases = {'clear': (True, True, True, True, None, None), |
179 | + 'progress': (False, True, True, True, msg, None), |
180 | + 'fail': (True, False, True, True, None, msg), |
181 | + 'success': (True, True, False, True, msg, None), |
182 | + 'warning': (True, True, True, False, msg, None), |
183 | + } |
184 | + review_app.run() |
185 | + |
186 | + for status in cases: |
187 | + review_app._change_status(status, msg) |
188 | + spinner, error, success, warn, label, error_detail = cases[status] |
189 | + self.assertEqual(spinner, |
190 | + review_app.submit_spinner.get_parent() is None) |
191 | + self.assertEqual(error, |
192 | + review_app.submit_error_img.get_window() is None) |
193 | + self.assertEqual(success, |
194 | + review_app.submit_success_img.get_window() is None) |
195 | + self.assertEqual(warn, |
196 | + review_app.submit_warn_img.get_window() is None) |
197 | + if label: |
198 | + self.assertEqual(label, |
199 | + review_app.label_transmit_status.get_text()) |
200 | + if error_detail: |
201 | + buff = review_app.error_textview.get_buffer() |
202 | + self.assertEqual(error_detail, |
203 | + buff.get_text(buff.get_start_iter(), buff.get_end_iter(), |
204 | + include_hidden_chars=False)) |
205 | + review_app.detail_expander.get_visible() |
206 | |
207 | def _p(self): |
208 | main_loop = GObject.main_context_default() |
209 | while main_loop.pending(): |
210 | main_loop.iteration() |
211 | |
212 | + |
213 | +class TestGRatingsAndReviews(unittest.TestCase): |
214 | + def setUp(self): |
215 | + mock_token = {'token': 'foobar', |
216 | + 'token_secret': 'foobar', |
217 | + 'consumer_key': 'foobar', |
218 | + 'consumer_secret': 'foobar', |
219 | + } |
220 | + self.api = GRatingsAndReviews(mock_token) |
221 | + |
222 | + def tearDown(self): |
223 | + self.api.shutdown() |
224 | + |
225 | + def make_review(self): |
226 | + review = Mock() |
227 | + review.rating = 4 |
228 | + review.origin = 'ubuntu' |
229 | + review.app.appname = '' |
230 | + review.app.pkgname = 'foobar' |
231 | + review.text = 'Super awesome app!' |
232 | + review.summary = 'Cool' |
233 | + review.package_version = '1.0' |
234 | + review.date = '2012-01-22' |
235 | + review.language = 'en' |
236 | + return review |
237 | + |
238 | + def wait_for_worker(self): |
239 | + while self.api.worker_thread._transmit_state != TRANSMIT_STATE_DONE: |
240 | + sleep(0.1) |
241 | + |
242 | + @patch('softwarecenter.backend.reviews.rnr_helpers.RatingsAndReviewsAPI' |
243 | + '.submit_review') |
244 | + def test_submit_review(self, mock_submit_review): |
245 | + mock_submit_review.return_value = ReviewDetails.from_dict( |
246 | + {'foo': 'bar'}) |
247 | + review = self.make_review() |
248 | + |
249 | + self.api.submit_review(review) |
250 | + |
251 | + self.wait_for_worker() |
252 | + self.assertTrue(mock_submit_review.called) |
253 | + review_arg = mock_submit_review.call_args[1]['review'] |
254 | + self.assertEqual(review.text, review_arg.review_text) |
255 | + |
256 | + @patch('softwarecenter.backend.reviews.rnr_helpers.RatingsAndReviewsAPI' |
257 | + '.flag_review') |
258 | + def test_flag_review(self, mock_flag_review): |
259 | + mock_flag_review.return_value = 'Something' |
260 | + |
261 | + self.api.report_abuse(1234, 'Far too silly', 'Stop right now.') |
262 | + |
263 | + self.wait_for_worker() |
264 | + self.assertTrue(mock_flag_review.called) |
265 | + self.assertEqual(1234, mock_flag_review.call_args[1]['review_id']) |
266 | + |
267 | + @patch('softwarecenter.backend.reviews.rnr_helpers.RatingsAndReviewsAPI' |
268 | + '.submit_usefulness') |
269 | + def test_submit_usefulness(self, mock_submit_usefulness): |
270 | + mock_submit_usefulness.return_value = 'Something' |
271 | + |
272 | + self.api.submit_usefulness(1234, True) |
273 | + |
274 | + self.wait_for_worker() |
275 | + self.assertTrue(mock_submit_usefulness.called) |
276 | + self.assertEqual(1234, mock_submit_usefulness.call_args[1]['review_id']) |
277 | + |
278 | + @patch('softwarecenter.backend.reviews.rnr_helpers.RatingsAndReviewsAPI' |
279 | + '.modify_review') |
280 | + def test_modify_review(self, mock_modify_review): |
281 | + mock_modify_review.return_value = ReviewDetails.from_dict( |
282 | + {'foo': 'bar'}) |
283 | + review = { |
284 | + 'summary': 'Cool', |
285 | + 'review_text': 'Super awesome app!', |
286 | + 'rating': 4, |
287 | + } |
288 | + |
289 | + self.api.modify_review(1234, review) |
290 | + |
291 | + self.wait_for_worker() |
292 | + self.assertTrue(mock_modify_review.called) |
293 | + self.assertEqual(1234, mock_modify_review.call_args[1]['review_id']) |
294 | + self.assertEqual(4, mock_modify_review.call_args[1]['rating']) |
295 | + self.assertEqual('Cool', mock_modify_review.call_args[1]['summary']) |
296 | + |
297 | + @patch('softwarecenter.backend.reviews.rnr_helpers.RatingsAndReviewsAPI' |
298 | + '.delete_review') |
299 | + def test_delete_review(self, mock_delete_review): |
300 | + mock_delete_review.return_value = 'Something' |
301 | + |
302 | + self.api.delete_review(1234) |
303 | + |
304 | + self.wait_for_worker() |
305 | + self.assertTrue(mock_delete_review.called) |
306 | + self.assertEqual(1234, mock_delete_review.call_args[1]['review_id']) |
307 | + |
308 | + |
309 | if __name__ == "__main__": |
310 | import logging |
311 | logging.basicConfig(level=logging.DEBUG) |
14:58 < noodles775> achuni: review swap? https:/ /code.launchpad .net/~michael. nelson/ software- center/ 833982- purchased- app-not- available- 3/+merge/ 89695 (review_ app._modify_ review_ is_the_ same()) ? app.review_ summary_ entry.set_ text(original) or similar?
14:58 < achuni> noodles775: for the reviews-tests mp?
14:58 * noodles775 starts achuni's MP, no probs if you've not time for mine.
14:58 < noodles775> Yeah.
14:58 < achuni> sounds fair :)
15:01 < noodles775> achuni: completely ignorable, but if we use multi-line imports for easier merging, l 102 is an opportunity.
15:01 < achuni> noodles775: ack
15:02 * noodles775 notices achuni's test class attributes, and wonders if there's any benefit to setUpClass from his own branch?
15:03 < achuni> noodles775: test class attributes?
15:03 < noodles775> line 122
15:04 < noodles775> I guess the only difference is *when* they are run... yours will be run when the code is parsed, where as using setUpClass it's when the test case (class) is executed. Either way they're run once only?
15:07 < achuni> noodles775: indeed, setUpClass looks interesting
15:07 < achuni> noodles775: let me switch for that
15:11 < noodles775> achuni: with the test on line 155
15:12 < noodles775> why do you do line 159: self.assertTrue
15:13 < noodles775> I guess I expected to see a call to review_
15:14 < noodles775> Also, is there any significance to the cases defined on l161?
15:14 < noodles775> Ah, regarding my previous q re line 159, I guess i expected to see l167 right before line 159.
15:15 * noodles775 starts realising why doing email reviews with in line diffs is handy... when will LP get inline comments? :P
15:19 < noodles775> Again, totally ignorable, but you could replace the tearDown on line 223 with a self.addCleanup if you wanted to.
15:24 < noodles775> All good, adding a +1 vote now.