Merge lp:~elachuni/software-center/reviews-tests into lp:software-center

[Anthony Lenton]

This branch includes several tests for code in the softwarecenter.ui.gtk3.review_gui_helper module.

[lp:~elachuni/software-center/reviews-tests]

2686. By Anthony Lenton on 2012-01-23

Merged in latest changes from trunk.

[Michael Nelson (michael.nelson)]

14:58 < noodles775> achuni: review swap? https://code.launchpad.net/~michael.nelson/software-center/833982-purchased-app-not-available-3/+merge/89695
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(review_app._modify_review_is_the_same())?
15:13 < noodles775> I guess I expected to see a call to review_app.review_summary_entry.set_text(original) or similar?
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.

review: Approve

[Gary Lasker (gary-lasker)]

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_reviews.py'). I'm not quite sure what's causing this hang, so if you could take a look I'd appreciate it very much. I found that commenting out the first test in the TestReviewLoader class prevented this hang, so I did that and merged the branch.

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.

=== modified file 'softwarecenter/ui/gtk3/review_gui_helper.py'
--- softwarecenter/ui/gtk3/review_gui_helper.py	2012-01-19 09:23:54 +0000
+++ softwarecenter/ui/gtk3/review_gui_helper.py	2012-01-23 13:29:23 +0000
@@ -200,12 +200,6 @@
                 self.pending_delete.empty()):
                 return
 
-    # usefulness
-    def queue_usefulness(self, usefulness):
-        """ queue a new usefulness report for sending to LP """
-        logging.debug("queue_usefulness %s %s %s" % usefulness)
-        self.pending_usefulness.put(usefulness)
-
     def _submit_usefulness_if_pending(self):
         """ the actual usefulness function """
         while not self.pending_usefulness.empty():
@@ -224,12 +218,6 @@
                 self._write_exception_html_log_if_needed(e)
                 self._transmit_state = TRANSMIT_STATE_ERROR
             self.pending_usefulness.task_done()
-    
-    #modify
-    def queue_modify(self, modification):
-        """ queue a new review modification request for sending to LP """
-        logging.debug("queue_modify %s %s" % modification)
-        self.pending_modify.put(modification)
 
     def _submit_modify_if_pending(self):
         """ the actual modify function """
@@ -254,13 +242,7 @@
                 self._transmit_state = TRANSMIT_STATE_ERROR
                 self._transmit_error_str = err_str
             self.pending_modify.task_done() 
-                       
-    #delete
-    def queue_delete(self, deletion):
-        """ queue a new deletion request for sending to LP """
-        logging.debug("queue_delete review id: %s" % deletion)
-        self.pending_delete.put(deletion)
-    
+
     def _submit_delete_if_pending(self):
         """ the actual deletion """
         while not self.pending_delete.empty():
@@ -278,12 +260,6 @@
                 self._transmit_state = TRANSMIT_STATE_ERROR
             self.pending_delete.task_done()
 
-    # reports
-    def queue_report(self, report):
-        """ queue a new report for sending to LP """
-        logging.debug("queue_report %s %s %s" % report)
-        self.pending_reports.put(report)
-
     def _submit_reports_if_pending(self):
         """ the actual report function """
         while not self.pending_reports.empty():
@@ -532,7 +508,7 @@
             
     def _change_status(self, type,  message):
         """method to separate the updating of status icon/spinner and message in the submit review window,
-         takes a type (progress, fail, success) as a string and a message string then updates status area accordingly"""
+         takes a type (progress, fail, success, clear, warning) as a string and a message string then updates status area accordingly"""
         self._clear_status_imagery()
         self.label_transmit_status.set_text("")
         
@@ -781,13 +757,13 @@
         if self.star_rating.get_rating() != self.orig_star_rating:
             return False
         #compare summary text
-        if self.review_summary_entry.get_text() != self.orig_summary_text:
+        if self.review_summary_entry.get_text().decode('utf-8') != self.orig_summary_text:
             return False
         #compare review text
         if self.review_buffer.get_text(
             self.review_buffer.get_start_iter(),
             self.review_buffer.get_end_iter(),
-            include_hidden_chars=False) != self.orig_review_text:
+            include_hidden_chars=False).decode('utf-8') != self.orig_review_text:
             return False
         return True
 
@@ -822,9 +798,7 @@
         markup = '<span fgcolor="#%s">%s</span>'
         if curr > cmax:
             return markup % (empty_col, str(cmax-curr))
-        elif curr < cmin:
-            return markup % (full_col, str(cmax-curr))
-        elif cmax == cmin:  #saves division by 0 later if same value was passed as min and max
+        elif curr <= cmin:  #saves division by 0 later if same value was passed as min and max
             return markup % (full_col, str(cmax-curr))
         else:
             #distance between min and max values to fade colours

=== renamed file 'test/test_reviews.py' => 'test/gtk3/test_reviews.py'
--- test/test_reviews.py	2012-01-19 09:23:54 +0000
+++ test/gtk3/test_reviews.py	2012-01-23 13:29:23 +0000
@@ -8,18 +8,22 @@
 from testutils import setup_test_env
 setup_test_env()
 from gettext import gettext as _
+from mock import Mock, patch
 
+from softwarecenter.backend.piston.rnrclient_pristine import ReviewDetails
 from softwarecenter.backend.reviews.rnr import (
     ReviewLoaderSpawningRNRClient as ReviewLoader)
 from softwarecenter.testutils import get_test_pkg_info, get_test_db
-from softwarecenter.ui.gtk3.review_gui_helper import SubmitReviewsApp
-
+from softwarecenter.ui.gtk3.review_gui_helper import (
+    TRANSMIT_STATE_DONE,
+    GRatingsAndReviews,
+    SubmitReviewsApp,
+    )
+from time import sleep
 
 class TestReviewLoader(unittest.TestCase):
-
-    def setUp(self):
-        self.cache = get_test_pkg_info()
-        self.db = get_test_db()
+    cache = get_test_pkg_info()
+    db = get_test_db()
 
     def _review_stats_ready_callback(self, review_stats):
         self._stats_ready = True
@@ -51,6 +55,7 @@
             version=None, action='modify', review_id=10000)
         # monkey patch away login to avoid that we actually login
         # and the UI changes because of that
+
         review_app.login = lambda: True
 
         # run the main app
@@ -66,12 +71,176 @@
             review_app.review_label.get_label())
         review_app.submit_window.hide()
 
+    def test_get_fade_colour_markup(self):
+        review_app = SubmitReviewsApp(datadir="../data", app=None,
+            parent_xid='', iconname='accessories-calculator', origin=None,
+            version=None, action='nothing')
+        cases = (
+            (('006000', '00A000', 40, 60, 50), ('008000', 10)),
+            (('000000', 'FFFFFF', 40, 40, 40), ('000000', 0)),
+            (('000000', '808080', 100, 400, 40), ('000000', 360)),
+            (('000000', '808080', 100, 400, 1000), ('808080', -600)),
+            (('123456', '5294D6', 10, 90, 70), ('427CB6', 20)),
+            )
+        for args, return_value in cases:
+            result = review_app._get_fade_colour_markup(*args)
+            expected = '<span fgcolor="#%s">%s</span>' % return_value
+            self.assertEqual(expected, result)
+
+    def test_modify_review_is_the_same_supports_unicode(self):
+        review_app = SubmitReviewsApp(datadir="../data", app=None,
+            parent_xid='', iconname='accessories-calculator', origin=None,
+            version=None, action='modify', review_id=10000)
+        self.assertTrue(review_app._modify_review_is_the_same())
+
+        cases = ('', 'e', ')!')
+        for case in cases:
+            modified = review_app.orig_summary_text[:-1] + case
+            review_app.review_summary_entry.set_text(modified)
+            self.assertFalse(review_app._modify_review_is_the_same())
+
+        review_app.review_summary_entry.set_text(review_app.orig_summary_text)
+        for case in cases:
+            modified = review_app.orig_review_text[:-1] + case
+            review_app.review_buffer.set_text(modified)
+            self.assertFalse(review_app._modify_review_is_the_same())
+
+    def test_change_status(self):
+        review_app = SubmitReviewsApp(datadir="../data", app=None,
+            parent_xid='', iconname='accessories-calculator', origin=None,
+            version=None, action='nothing')
+        msg = 'Something completely different'
+        cases = {'clear': (True, True, True, True, None, None),
+                 'progress': (False, True, True, True, msg, None),
+                 'fail': (True, False, True, True, None, msg),
+                 'success': (True, True, False, True, msg, None),
+                 'warning': (True, True, True, False, msg, None),
+                }
+        review_app.run()
+
+        for status in cases:
+            review_app._change_status(status, msg)
+            spinner, error, success, warn, label, error_detail = cases[status]
+            self.assertEqual(spinner,
+                review_app.submit_spinner.get_parent() is None)
+            self.assertEqual(error,
+                review_app.submit_error_img.get_window() is None)
+            self.assertEqual(success,
+                review_app.submit_success_img.get_window() is None)
+            self.assertEqual(warn,
+                review_app.submit_warn_img.get_window() is None)
+            if label:
+                self.assertEqual(label,
+                    review_app.label_transmit_status.get_text())
+            if error_detail:
+                buff = review_app.error_textview.get_buffer()
+                self.assertEqual(error_detail,
+                    buff.get_text(buff.get_start_iter(), buff.get_end_iter(),
+                    include_hidden_chars=False))
+                review_app.detail_expander.get_visible()
 
     def _p(self):
         main_loop = GObject.main_context_default()
         while main_loop.pending():
             main_loop.iteration()
 
+
+class TestGRatingsAndReviews(unittest.TestCase):
+    def setUp(self):
+        mock_token = {'token': 'foobar', 
+                      'token_secret': 'foobar',
+                      'consumer_key': 'foobar',
+                      'consumer_secret': 'foobar',
+                     }
+        self.api = GRatingsAndReviews(mock_token)
+
+    def tearDown(self):
+        self.api.shutdown()
+
+    def make_review(self):
+        review = Mock()
+        review.rating = 4
+        review.origin = 'ubuntu'
+        review.app.appname = ''
+        review.app.pkgname = 'foobar'
+        review.text = 'Super awesome app!'
+        review.summary = 'Cool'
+        review.package_version = '1.0'
+        review.date = '2012-01-22'
+        review.language = 'en'
+        return review        
+
+    def wait_for_worker(self):
+        while self.api.worker_thread._transmit_state != TRANSMIT_STATE_DONE:
+            sleep(0.1)
+
+    @patch('softwarecenter.backend.reviews.rnr_helpers.RatingsAndReviewsAPI'
+        '.submit_review')
+    def test_submit_review(self, mock_submit_review):
+        mock_submit_review.return_value = ReviewDetails.from_dict(
+            {'foo': 'bar'})
+        review = self.make_review()
+
+        self.api.submit_review(review)
+
+        self.wait_for_worker()
+        self.assertTrue(mock_submit_review.called)
+        review_arg = mock_submit_review.call_args[1]['review']
+        self.assertEqual(review.text, review_arg.review_text)
+
+    @patch('softwarecenter.backend.reviews.rnr_helpers.RatingsAndReviewsAPI'
+        '.flag_review')
+    def test_flag_review(self, mock_flag_review):
+        mock_flag_review.return_value = 'Something'
+
+        self.api.report_abuse(1234, 'Far too silly', 'Stop right now.')
+
+        self.wait_for_worker()
+        self.assertTrue(mock_flag_review.called)
+        self.assertEqual(1234, mock_flag_review.call_args[1]['review_id'])
+
+    @patch('softwarecenter.backend.reviews.rnr_helpers.RatingsAndReviewsAPI'
+        '.submit_usefulness')
+    def test_submit_usefulness(self, mock_submit_usefulness):
+        mock_submit_usefulness.return_value = 'Something'
+
+        self.api.submit_usefulness(1234, True)
+
+        self.wait_for_worker()
+        self.assertTrue(mock_submit_usefulness.called)
+        self.assertEqual(1234, mock_submit_usefulness.call_args[1]['review_id'])
+
+    @patch('softwarecenter.backend.reviews.rnr_helpers.RatingsAndReviewsAPI'
+        '.modify_review')
+    def test_modify_review(self, mock_modify_review):
+        mock_modify_review.return_value = ReviewDetails.from_dict(
+            {'foo': 'bar'})
+        review = {
+            'summary': 'Cool',
+            'review_text': 'Super awesome app!',
+            'rating': 4,
+        }
+
+        self.api.modify_review(1234, review)
+
+        self.wait_for_worker()
+        self.assertTrue(mock_modify_review.called)
+        self.assertEqual(1234, mock_modify_review.call_args[1]['review_id'])
+        self.assertEqual(4, mock_modify_review.call_args[1]['rating'])
+        self.assertEqual('Cool', mock_modify_review.call_args[1]['summary'])
+
+    @patch('softwarecenter.backend.reviews.rnr_helpers.RatingsAndReviewsAPI'
+        '.delete_review')
+    def test_delete_review(self, mock_delete_review):
+        mock_delete_review.return_value = 'Something'
+
+        self.api.delete_review(1234)
+
+        self.wait_for_worker()
+        self.assertTrue(mock_delete_review.called)
+        self.assertEqual(1234, mock_delete_review.call_args[1]['review_id'])
+
+
 if __name__ == "__main__":
     import logging
     logging.basicConfig(level=logging.DEBUG)