ubuntu-system-settings

Merge lp:~chipaca/ubuntu-system-settings/push-helper-to-spec into lp:ubuntu-system-settings

push-helper-to-spec
Merge into trunk

Proposed by John Lenton on 2014-09-11

Status:

Merged

Approved by:

Sebastien Bacher on 2014-09-15

Approved revision:

1034

Merged at revision:

1047

Proposed branch:

lp:~chipaca/ubuntu-system-settings/push-helper-to-spec

Merge into:

lp:ubuntu-system-settings

Diff against target:

417 lines (+264/-75)

3 files modified

CMakeLists.txt (+1/-1)
push-helper/software_updates_helper.py (+149/-64)
tests/test_push_helper.py.in (+114/-10)

To merge this branch:

bzr merge lp:~chipaca/ubuntu-system-settings/push-helper-to-spec

Low

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Approve on 2014-09-15
Sebastien Bacher (community)		2014-09-11	Approve on 2014-09-15
Ken VanDine			Needs Fixing on 2014-09-12
Roberto Alsina (community)			Approve on 2014-09-11
Review via email: mp+234311@code.launchpad.net

Commit message

Take the system settings push helper closer to implementing the spec at https://wiki.ubuntu.com/SoftwareUpdates#Prompting

Description of the change

This brings the push helper for system settings closer to implementing https://wiki.ubuntu.com/SoftwareUpdates#Prompting

Missing still is the bit about click package updates counter. Also the emblem doesn't appear everywhere the icon for system settings is, so the fallback to a notification in the software centre exists instead.

Revision history for this message

Roberto Alsina (ralsina) on 2014-09-11:

review: Approve

Revision history for this message

Roberto Alsina (ralsina) wrote on 2014-09-11:

To test:

* Downgrade to an older image
* Configure for automatic update
* gdbus call -e -d com.ubuntu.Postal -o /com/ubuntu/Postal/_ -m com.ubuntu.Postal.Post _ubuntu-system-settings '"null"'
* Image will download automatically
* Notification on notification centre about update will appear

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-11:

FAILED: Continuous integration, rev:1031
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~chipaca/ubuntu-system-settings/push-helper-to-spec/+merge/234311/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/1454/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4726/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3476
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/646
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4497/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5978
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5978/artifact/work/output/*zip*/output.zip
    None: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/13067/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/2877
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3781
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3781/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1454/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-11:

FAILED: Continuous integration, rev:1033
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~chipaca/ubuntu-system-settings/push-helper-to-spec/+merge/234311/+edit-commit-message

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1459/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Sebastien Bacher (seb128) wrote on 2014-09-11:

Thanks, does that address bug #1363972 (notification should replace previous ones)? if not could that be done in the same changeset?

could you also summarize what should change compared to the previous version, from an user perspective, so we can confirm that the update works as it should?

review: Needs Information

Revision history for this message

Sebastien Bacher (seb128) wrote on 2014-09-11:

oh, also the title is confusing since it suggest you can tap anywhere on the entry where it's not the case, see bug #1366288 (not a new issue but I want to point it out while we refactor/update to match design)

Revision history for this message

John Lenton (chipaca) wrote on 2014-09-11:

> Thanks, does that address bug #1363972 (notification should replace previous
> ones)? if not could that be done in the same changeset?

yes, it fixes that bug. I'll tie the bug to the branch now.

> could you also summarize what should change compared to the previous version,
> from an user perspective, so we can confirm that the update works as it
> should?

yes. Before, when a broadcast notification came in (or if you faked it with gdbus as per roberto's suggestion) you'd get a notification immediately every time, and they'd stack up.

Now, you only get a notification immediately if you have turned off auto downloads; otherwise, you'll get a notification after it's been downloaded.

Revision history for this message

John Lenton (chipaca) wrote on 2014-09-11:

> oh, also the title is confusing since it suggest you can tap anywhere on the
> entry where it's not the case, see bug #1366288 (not a new issue but I want to
> point it out while we refactor/update to match design)

as far as i know i'm per design on this, if not please point me at it?

the tap-to-expand-then-tap-this-little-icon-to-activate thing is a bit of a pet peeve of mine that i hope to address with design at some point, tho.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-11:

FAILED: Continuous integration, rev:1034
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~chipaca/ubuntu-system-settings/push-helper-to-spec/+merge/234311/+edit-commit-message

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1464/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Ken VanDine (ken-vandine) wrote on 2014-09-12:

Looks good, it does properly supersede the notification instead of stacking with normal use. You can trigger a race if you make several dbus calls to simulate the notification, it'll still give you multiple notifications. But that's a corner case, the postal service shouldn't do that.

review: Approve

Revision history for this message

Ted Gould (ted) wrote on 2014-09-12:

This relies on a bug in lifecycle confinement that will be fixed shortly:

https://bugs.launchpad.net/ubuntu-app-launch/+bug/1358753

At that point all children will also have the same lifecycle as their parents.

Revision history for this message

Ken VanDine (ken-vandine) wrote on 2014-09-12:

I dropped the top approval based on ted's comment, I'd rather not just end up with this broken next week and have to scramble to get a fix.

review: Needs Fixing

Revision history for this message

John Lenton (chipaca) wrote on 2014-09-12:

The system settings push helper is not an untrusted helper.

Revision history for this message

John Lenton (chipaca) wrote on 2014-09-12:

That is: the system settings push helper is not an untrusted helper, so it does not run confined. It is not launched by ubuntu-app-launch. That bug and its fix does not apply; you can't have untrusted helpers for "legacy" applications, so for legacy applications (which are trusted) we just do an exec "old style".

Revision history for this message

Ted Gould (ted) wrote on 2014-09-13:

Legacy applications can have untrusted helpers you just need to have an AppArmor profile or set them explicitly to unconfined. I'd recommend that they have an AppArmor profile as there are dpkg tools that make that relatively easy to maintain.

I'm really surprised that there's two separate mechanisms to run helpers, but I guess that does make this MR safe, so I retract my objection.

Revision history for this message

John Lenton (chipaca) wrote on 2014-09-13:

> Legacy applications can have untrusted helpers you just need to have an
> AppArmor profile or set them explicitly to unconfined. I'd recommend that they
> have an AppArmor profile as there are dpkg tools that make that relatively
> easy to maintain.

Hm! I'm pretty sure this failed in interesting ways when I tested it. I've just tested it again and it fails in a sane way that looks suspiciously like a missing apparmor profile. I guess we can revisit this split then (not now though).

I'll catch you on IRC to go over whether it'll possible to implement the design for update notifications without escaping the lifecycle nor special-casing system settings.

> I'm really surprised that there's two separate mechanisms to run helpers,

yeah, i wasn't too happy having to write that code either. Not having to maintain it long term would be good.

> but
> I guess that does make this MR safe, so I retract my objection.

Revision history for this message

Sebastien Bacher (seb128) wrote on 2014-09-15:

since the issue got resolved setting back to approved, we can land it ;-)

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-15:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1482/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

John Lenton

Ubuntu Touch System Settings

 === modified file 'CMakeLists.txt'
 --- CMakeLists.txt	2014-08-05 10:12:14 +0000
 +++ CMakeLists.txt	2014-09-11 20:53:25 +0000
@@ -76,7 +76,7 @@
  install(FILES ${CMAKE_CURRENT_BINARY_DIR}/ubuntu-system-settings.desktop DESTINATION share/applications)
  install(FILES ubuntu-system-settings.url-dispatcher DESTINATION share/url-dispatcher/urls)
  install(FILES screenshot.png DESTINATION ${SETTINGS_SHARE_DIR})
--install(PROGRAMS push-helper/software-updates-helper.py DESTINATION ${PUSH_HELPER_DIR} RENAME ubuntu-system-settings)
++install(PROGRAMS push-helper/software_updates_helper.py DESTINATION ${PUSH_HELPER_DIR} RENAME ubuntu-system-settings)
  if(cmake_build_type_lower MATCHES coverage)
    ENABLE_COVERAGE_REPORT(TARGETS system-settings FILTER /usr/include ${CMAKE_SOURCE_DIR}/tests/* ${CMAKE_BINARY_DIR}/*)
 === renamed file 'push-helper/software-updates-helper.py' => 'push-helper/software_updates_helper.py'
 --- push-helper/software-updates-helper.py	2014-08-28 00:45:29 +0000
 +++ push-helper/software_updates_helper.py	2014-09-11 20:53:25 +0000
@@ -16,74 +16,159 @@
  # 2. yes, this is rather convoluted. Most push helpers don't have to deal with
  #    this stuff.
++import os
  import json
  import sys
  import time
  import gettext
--if len(sys.argv) != 3:
--    print("File in and out expected via argv", file=sys.stderr)
--    sys.exit(1)
--
--f1, f2 = sys.argv[1:3]
  _ = gettext.translation('ubuntu-system-settings', fallback=True).gettext
--# here you should look at the input (the contents of the file whose
--# name is in f1, which are guaranteed to be json). If it's a broadcast
--# it will be the most recent we've received, and will have passed a
--# minimum amount of sanity checking, but you can probably do more. As
--# per the design on https://wiki.ubuntu.com/SoftwareUpdates#Prompting
--# if things are set to auto-download you should go download them (in a
--# child process -- this helper process itself has 4 more seconds to
--# live).
--#
--# the broadcast payload will be a single json object looking like
--#   { image-channel/device-model": [build-number, channel-alias]}
--#
--# e.g.,
--#
--#   {"ubuntu-touch/utopic-proposed/hammerhead":[265,""]}
--#
--#
--# When the click server starts sending notifications of packages a user can
--# update, you should probably describe that payload here. What to do with it
--# is described in some detail in the wiki page above.
--#
--#
--# Once you've downloaded things and need to actually notify the user, you'd
--# send a notification as below, over dbus to Post. That's the third payload
--# this script will be called with; for that case you'd just pass the payload
--# through.
--#
--# For cases when you don't want to notify the user (yet), the correct
--# output (to be written to a file whose name is f2) is “{}”, i.e. an
--# empty json object.
--#
--# For now, this script assumes everything that comes in is a valid
--# broadcast notification, and notifies the user directly:
--
--
--icon = "/usr/share/ubuntu/settings/system/icons/settings-system-update.svg"
--obj = {
--    "notification": {
--        "emblem-counter": {
--            "count": 1,
--            "visible": True,
--        },
--        "vibrate": {
--            "pattern": [50, 150],
--            "repeat": 3,
--        },
--        "card": {
--            "summary": _("There's an updated system image."),
--            "body": _("Tap to open the system updater."),
--            "actions": ["settings:///system/system-update"],
--            "icon": icon,
--            "timestamp": int(time.time()),
--            "persist": True,
--            "popup": True,
--        },
--    },
--}
--
--json.dump(obj, open(f2, "w"))
++SYS_UPDATE = "system-image-update"
++
++
++class SystemImage:
++    def __init__(self):
++        self.loop = None
++        self.sysimg = None
++        self.postal = None
++        self.notify = False
++
++    def setup(self):
++        import dbus
++        from dbus.mainloop.glib import DBusGMainLoop
++        from gi.repository import GLib
++
++        gloop = DBusGMainLoop()
++        sys_bus = dbus.SystemBus(mainloop=gloop)
++        ses_bus = dbus.SessionBus(mainloop=gloop)
++        self.loop = GLib.MainLoop()
++        self.sysimg = dbus.Interface(
++            sys_bus.get_object("com.canonical.SystemImage", "/Service"),
++            dbus_interface="com.canonical.SystemImage")
++        self.postal = dbus.Interface(
++            ses_bus.get_object("com.ubuntu.Postal", "/com/ubuntu/Postal/_"),
++            dbus_interface="com.ubuntu.Postal")
++        self.notify = False
++
++    def quit(self):
++        if self.notify:
++            # remove any older notifications about this
++            self.postal.ClearPersistent("_ubuntu-system-settings", SYS_UPDATE)
++            # send ours. This will of course come back to this same script.
++            self.postal.Post("_ubuntu-system-settings", json.dumps(SYS_UPDATE))
++        self.loop.quit()
++
++    def available_cb(self, available, downloading, *ignored):
++        if available:
++            if downloading:
++                # handled in the UpdateDownloaded or UpdateFailed handlers
++                return
++            # available and not downloading: auto downloads are turned
++            # off; notify the user right now.
++            self.notify = True
++        else:
++            # if not available, we were called spuriously. No notification.
++            self.notify = False
++        self.quit()
++
++    def downloaded_cb(self):
++        self.notify = True
++        self.quit()
++
++    def failed_cb(self, *ignored):
++        # give up
++        self.notify = False
++        self.quit()
++
++    def run(self):
++        self.sysimg.connect_to_signal("UpdateAvailableStatus",
++                                      self.available_cb)
++        self.sysimg.connect_to_signal("UpdateDownloaded", self.downloaded_cb)
++        self.sysimg.connect_to_signal("UpdateFailed", self.failed_cb)
++        self.sysimg.CheckForUpdate()
++        self.loop.run()
++
++
++def main():
++    if len(sys.argv) != 3:
++        print("File in and out expected via argv", file=sys.stderr)
++        sys.exit(1)
++
++    f1, f2 = sys.argv[1:3]
++
++    # here you should look at the input (the contents of the file whose
++    # name is in f1, which are guaranteed to be json). If it's a broadcast
++    # it will be the most recent we've received, and will have passed a
++    # minimum amount of sanity checking, but you can probably do more. As
++    # per the design on https://wiki.ubuntu.com/SoftwareUpdates#Prompting
++    # if things are set to auto-download you should go download them (in a
++    # child process -- this helper process itself has 4 more seconds to
++    # live).
++    #
++    # the broadcast payload will be a single json object looking like
++    #   { image-channel/device-model": [build-number, channel-alias]}
++    #
++    # e.g.,
++    #
++    #   {"ubuntu-touch/utopic-proposed/hammerhead":[265,""]}
++    #
++    #
++    # When the click server starts sending notifications of packages a user
++    # can update, you should probably describe that payload here. What to
++    # do with it is described in some detail in the wiki page above.
++    #
++    #
++    # Once you've downloaded things and need to actually notify the user,
++    # you'd send a notification as below, over dbus to Post. That's the
++    # third payload this script will be called with; for that case you'd
++    # just pass the payload through.
++    #
++    # For cases when you don't want to notify the user (yet), the correct
++    # output (to be written to a file whose name is f2) is “{}”, i.e. an
++    # empty json object.
++
++    with open(f1) as f:
++        arg = json.load(f)
++
++    obj = {}
++    if arg == "system-image-update":
++        icon = "/usr/share/ubuntu/settings/system/icons/" + \
++               "settings-system-update.svg"
++        obj = {
++            "notification": {
++                "tag": SYS_UPDATE,
++                "emblem-counter": {
++                    "count": 1,
++                    "visible": True,
++                },
++                "vibrate": {
++                    "pattern": [50, 150],
++                    "repeat": 3,
++                },
++                "card": {
++                    "summary": _("There's an updated system image."),
++                    "body": _("Tap to open the system updater."),
++                    "actions": ["settings:///system/system-update"],
++                    "icon": icon,
++                    "timestamp": int(time.time()),
++                    "persist": True,
++                },
++            },
++        }
++    elif arg == "testing":
++        # for tests
++        obj = {"testing": True}
++    else:
++        # assume it's a broadcast
++        if os.fork() == 0:
++            os.setsid()
++            s = SystemImage()
++            s.setup()
++            s.run()
++            return
++
++    json.dump(obj, open(f2, "w"))
++
++if __name__ == '__main__':
++    main()
 === modified file 'tests/test_push_helper.py.in'
 --- tests/test_push_helper.py.in	2014-09-03 14:12:52 +0000
 +++ tests/test_push_helper.py.in	2014-09-11 20:53:25 +0000
@@ -1,5 +1,4 @@
--#!/usr/bin/python
--
++#!/usr/bin/python3
  # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
  # Copyright 2014 Canonical
+ #
@@ -14,6 +13,16 @@
  import sys
  import tempfile
  import unittest
++from unittest import mock
++
++HELPER_DIR = '@CMAKE_CURRENT_SOURCE_DIR@/../push-helper/'
++sys.path.append(HELPER_DIR)
++import software_updates_helper
++
++
++class TestingSystemImage(software_updates_helper.SystemImage):
++    def setup(self):
++        pass
  class PushHelperTests(unittest.TestCase):
@@ -22,15 +31,14 @@
      def setUp(self):
          super(PushHelperTests, self).setUp()
          self.tmp_dir = tempfile.mkdtemp(suffix='push-helper', prefix='tests')
--        self.helper_path = '@CMAKE_CURRENT_SOURCE_DIR@/../push-helper/' + \
--            'software-updates-helper.py'
++        self.helper_path = HELPER_DIR + 'software_updates_helper.py'
      def tearDown(self):
          super(PushHelperTests, self).tearDown()
          shutil.rmtree(self.tmp_dir)
      def run_push_helper(self, input_fname, output_fname):
--        subprocess.call([self.helper_path, input_fname, output_fname],
++        subprocess.call(["python3", self.helper_path, input_fname, output_fname],
                          stdout=subprocess.PIPE)
      def create_input_file(self, filename, content):
@@ -49,7 +57,7 @@
          self.assertEqual(card['actions'], ['settings:///system/system-update'])
          self.assertEqual(card['persist'], True)
          self.assertEqual(card['body'], 'Tap to open the system updater.')
--        self.assertEqual(card['popup'], True)
++        self.assertEqual(card.get('popup', False), False)
          emblem_counter = notif['notification']['emblem-counter']
          self.assertEqual(emblem_counter, {'visible': True, 'count': 1})
          vibrate = notif['notification']['vibrate']
@@ -58,7 +66,7 @@
      def test_update_broadcast(self):
          """Default system-update broadcast."""
          input_f = self.create_input_file('bcast_in',
--                                         '{"daily/mako": [200, ""]}')
++                                         '"system-image-update"')
          output_f = os.path.join(self.tmp_dir, 'bcast_out')
          self.run_push_helper(input_f, output_f)
          with open(output_f, 'r') as fd:
@@ -67,13 +75,109 @@
      def test_valid_json(self):
          """Handle a valid json input."""
--        input_f = self.create_input_file('valid_json_in', '"null"')
++        input_f = self.create_input_file('valid_json_in', '"testing"')
          output_f = os.path.join(self.tmp_dir, 'valid_json_out')
          self.run_push_helper(input_f, output_f)
          with open(output_f, 'r') as fd:
              output = json.load(fd)
--        self.assertSystemUpdateNotification(output)
--
++        self.assertEqual(output, {"testing": True})
++
++    def test_system_image_run(self):
++        """Check that run looks sane"""
++        s = TestingSystemImage()
++        s.sysimg = mock.Mock(name="sysimg")
++        s.loop = mock.Mock(name="loop")
++        s.run()
++        # check the main loop was run
++        s.loop.run.assert_called_once_with()
++        # check CheckForUpdate was called
++        s.sysimg.CheckForUpdate.assert_called_once_with()
++        # and connect_to_signal
++        s.sysimg.connect_to_signal.assert_any_call("UpdateDownloaded",
++                                                   s.downloaded_cb)
++        s.sysimg.connect_to_signal.assert_any_call("UpdateFailed",
++                                                   s.failed_cb)
++        s.sysimg.connect_to_signal.assert_any_call("UpdateAvailableStatus",
++                                                   s.available_cb)
++        self.assertEqual(s.notify, False)
++
++    def test_available_and_downloading(self):
++        """check that available_cb when available and d'loading just returns"""
++        s = TestingSystemImage()
++        s.quit = mock.Mock(name="quit")
++
++        self.assertEqual(s.notify, False)
++        # available and downloading; returns without calling quit
++        s.available_cb(True, True)
++        self.assertEqual(s.notify, False)
++        self.assertEqual(s.quit.called, False)
++
++    def test_available_not_downloading(self):
++        """check that available_cb when available and not downloading
++        sets notify and quits"""
++        s = TestingSystemImage()
++        s.quit = mock.Mock(name="quit")
++
++        self.assertEqual(s.notify, False)
++        # available and not downloading; quits with notification
++        s.available_cb(True, False)
++        self.assertEqual(s.notify, True)
++        s.quit.assert_called_once_with()
++
++    def test_not_available(self):
++        """check that available_cb quits when not available"""
++        s = TestingSystemImage()
++        s.quit = mock.Mock(name="quit")
++
++        self.assertEqual(s.notify, False)
++        # not available; quits without notifying
++        s.available_cb(False, False)
++        self.assertEqual(s.notify, False)
++        s.quit.assert_called_once_with()
++
++    def test_downloaded_cb(self):
++        """check that on download, notify is set to True and quit is called"""
++        s = TestingSystemImage()
++        s.quit = mock.Mock(name="quit")
++
++        self.assertEqual(s.notify, False)
++        s.downloaded_cb()
++        self.assertEqual(s.notify, True)
++        s.quit.assert_called_once_with()
++
++    def test_failed_cb(self):
++        """check that on failure, notify is set to False and quit is called"""
++        s = TestingSystemImage()
++        s.quit = mock.Mock(name="quit")
++
++        self.assertEqual(s.notify, False)
++        s.failed_cb()
++        self.assertEqual(s.notify, False)
++        s.quit.assert_called_once_with()
++
++    def test_quit_no_notify(self):
++        """Check that quit withlooks sane"""
++        s = TestingSystemImage()
++        s.postal = mock.Mock(name="sysimg")
++        s.loop = mock.Mock(name="loop")
++        s.notify = False
++        s.quit()
++        self.assertEqual(s.postal.Post.called, False)
++        self.assertEqual(s.postal.ClearPersistent.called, False)
++        s.loop.quit.assert_called_once_with()
++
++    def test_quit_with_notify(self):
++        """Check that quit withlooks sane"""
++        s = TestingSystemImage()
++        s.postal = mock.Mock(name="sysimg")
++        s.loop = mock.Mock(name="loop")
++        s.notify = True
++        s.quit()
++        s.postal.Post.assert_called_once_with("_ubuntu-system-settings",
++                                              '"system-image-update"')
++        s.postal.ClearPersistent.assert_called_once_with(
++            "_ubuntu-system-settings", "system-image-update")
++        s.loop.quit.assert_called_once_with()
  if __name__ == '__main__':
      unittest.main(