> On Fri, Sep 14, 2012 at 10:53:23PM -0000, Gary Lasker wrote:
> > Gary Lasker has proposed merging lp:~gary-lasker/software-center/handle-
> trans-cancel-lp1027209 into lp:software-center.
> [..]
> > Follow the below steps to see the issue:
> > #. Open software-center
> > #. Click on install on a big application like wesnoth
> > #. Wait until the "In Progress" appears in the toplevel toolbar of s-c
> > #. Click on the "In Progress" button in the toolbar
> > #. Click on the "cancel" button of the install of the application
> >
> > I've also included a new unit test for this bug to insure we don't regress
> on it.
> 
> Thanks for the branch and for adding a unittest (and the adding of
> docstrings, always nice).
> 
> It looks fine, the only question I have is what happens if the
> transaction errors, e.g. if the network gets lost during a download of
> a application. Will that work as expected, i.e. not add anything to
> the launcher or do we need to connect the transaction-stopped signal
> for that?

Thanks! I've added a unit test for this case and and shows that the launcher call does not occur. I took the opportunity to consolodate some (now duplicated) code.


> 
> [..]
> > === modified file 'tests/gtk3/test_unity_launcher_integration.py'
> > --- tests/gtk3/test_unity_launcher_integration.py     2012-08-30 07:45:31
> +0000
> > +++ tests/gtk3/test_unity_launcher_integration.py     2012-09-14 22:52:21
> +0000
> > @@ -60,6 +60,20 @@
> >          self.available_pane.backend.emit("transaction-finished",
> >                                      mock_result)
> >          do_events_with_sleep()
> > +
> > +    def _simulate_install_cancellation(self, app):
> > +        # pretend we started an install
> > +        self.available_pane.backend.emit("transaction-started",
> > +                                    app.pkgname, app.appname,
> > +                                    "testid101",
> > +                                    TransactionTypes.INSTALL)
> > +        do_events_with_sleep()
> 
>       self.assertEqual(
>             self.available_pane.unity_launcher_transaction_queue.keys(),
>             ["software-center"])
> 
> > +        # send the signal to cancel the install
> > +        mock_result = Mock()
> > +        mock_result.pkgname = app.pkgname
> > +        self.available_pane.backend.emit("transaction-cancelled",
> > +                                    mock_result)
> > +        do_events_with_sleep()
> 
>         self.assertEqual(
>             self.available_pane.unity_launcher_transaction_queue, {})
> 
> 
> [..]
> 
> I like the test in general, the above lines may make it even
> better. To also check that the queue behaves in the right way
> (i.e. stuff is added and taken out again).

This diff is a little confusing to me as it appears to add code that is already in the test (in the method _simulate_install_cancellation). But I get the gist which is to check that the queue is maintained correcty. This may not be strictly necessary, however, but I'll think about it a bit more and will add this test enhancement in a separate follow-on branch in the interest of getting the current branch merged (we are close to the wire for b2 freeze).
> 
> Cheers,
>  Michael