Merge lp:~barry/ubuntu-ota-tests/check-for-update-2 into lp:ubuntu-ota-tests

[Barry Warsaw]

Add check for update test and related services.

Take 2

[Christopher Lee (veebers)]

(as mentioned in inline comments) I think we should have a better method for having support scripts and functionality, I've suggested something in my branch[1] (as well as posed the question there re: packaging the scripts).

[1] https://code.launchpad.net/~canonical-platform-qa/ubuntu-ota-tests/check-for-running-service/+merge/252057

review: Needs Information

[Barry Warsaw (barry)]

On Mar 06, 2015, at 04:44 AM, Christopher Lee wrote:

>(as mentioned in inline comments) I think we should have a better method for
>having support scripts and functionality, I've suggested something in my
>branch[1] (as well as posed the question there re: packaging the scripts).

Yep, commented there. TL;DR: I like create a package for shared
functionality, and debian/tests/ubuntu_ota_tests is fine for now, but later
they should be refactored out into a normal Python 3 package.

>> === added directory 'debian'
>> === renamed directory 'debian' => 'debian.moved'
>> === added file 'debian/changelog'
>> --- debian/changelog 1970-01-01 00:00:00 +0000
>> +++ debian/changelog 2015-03-06 00:20:09 +0000
>> @@ -0,0 +1,5 @@
>> +ubuntu-ota-tests (1.0-1) UNRELEASED; urgency=medium
>> +
>> + * Initial release.
>> +
>> + -- Christopher Lee <email address hidden> Tue, 03 Mar 2015 14:32:30 +1300
>
>This seems odd to be in the diff, it should be in trunk. Either something has
>gone wrong here or trunk needs to be fixed.

Nope, the branch was submitted against a prerequisite branch because at the
time, trunk didn't have the necessary infrastructure. It does now, so I'll
resubmit this branch against trunk, after taking into consideration all the
comments here.

>> +class CheckingReactor(Reactor):
>> + def __init__(self, bus):
>> + super().__init__(bus)
>> + self.signals = []
>> +
>> + def _do_UpdateAvailableStatus(self, signal, path, *args):
>> + self.signals.append(UASRecord(*args))
>> + self.quit()
>> +
>> + def run(self, timeout=None):
>> + super().run(timeout)
>> + if self.timed_out:
>> + raise TimeoutError
>
>These are cool :-) Will they be useful in other tests/scripts? If so it will
>be worth moving them into a support module (as per other comment).

Yep! Will do.

>> +
>> +
>> +def check_for_update():
>> + # Since we'll be receiving signals, we have to first define the default
>> + # D-Bus main loop.
>> + DBusGMainLoop(set_as_default=True)
>> + # Boilerplate for making calls to the system service.
>> + system_bus = dbus.SystemBus()
>> + service = system_bus.get_object('com.canonical.SystemImage', '/Service')
>> + iface = dbus.Interface(service, 'com.canonical.SystemImage')
>
>I imagine the boilerplate code will be used a lot so should live somewhere
>communal, I've comment in the big comment box re: how I've copied this code
>of yours into support modules :-)
>(https://code.launchpad.net/~canonical-platform-qa/ubuntu-ota-tests/check-for-running-service/+merge/252057)

Yep, definitely should be shared. See my comments in your mp. :)

[lp:~barry/ubuntu-ota-tests/check-for-update-2]

4. By Barry Warsaw on 2015-03-09

Whitespace normalization.

5. By Barry Warsaw on 2015-03-09

Whitespace normalization.

6. By Barry Warsaw on 2015-03-12

Trunk merge.

7. By Barry Warsaw on 2015-03-12

Add the reactors back.

8. By Barry Warsaw on 2015-03-12

Add the check_for_update test.

[Barry Warsaw (barry)]

Trunk merged, ready for follow up reviews.

[Federico Gimenez (fgimenez)]

The reactor module is really great, I'm already using it the apply-update branch and works flawlessly :)

Perhaps you could add a check_for_update method in the services module and move the debian/tests/check_for_update code to the test_services selftest, although I don't know how you plan to use this check. Anyway, not a blocker.

Thanks!

review: Approve

[lp:~barry/ubuntu-ota-tests/check-for-update-2]

9. By Barry Warsaw on 2015-03-12

Add test dependencies.

10. By Barry Warsaw on 2015-03-12

* Move the guts of the check for update into a service function.
* Add a selftest for check_for_update.
* Refactor test main.

[Barry Warsaw (barry)]

On Mar 12, 2015, at 03:28 PM, Federico Gimenez wrote:

>The reactor module is really great, I'm already using it the apply-update
>branch and works flawlessly :)

\o/

>Perhaps you could add a check_for_update method in the services module and
>move the debian/tests/check_for_update code to the test_services selftest,
>although I don't know how you plan to use this check. Anyway, not a blocker.

Great ideas. I've now done this.

[lp:~barry/ubuntu-ota-tests/check-for-update-2]

11. By Barry Warsaw on 2015-03-12

Trunk merge, resolve conflicts.

[Christopher Lee (veebers)]

Nice, I do have one inline query re: utc time but otherwise this is an approve :-)

review: Needs Information

[lp:~barry/ubuntu-ota-tests/check-for-update-2]

12. By Barry Warsaw on 2015-03-16

Trunk merge

13. By Barry Warsaw on 2015-03-16

* Actually use UTC.
* Run the selftests in verbose mode so they're easier to verify.

[Barry Warsaw (barry)]

On Mar 13, 2015, at 08:02 AM, Christopher Lee wrote:

>> + def test_check_for_update(self):
>> + status = services.check_for_update()
>> + utcnow = datetime.now().replace(microsecond=0).isoformat(' ')
>
>This wouldn't actually be utc would it? datetime.now uses localtime if no tz
>is passed in. For utc there is datetime.utcnow

Good catch! Yes, this should have been utcnow(). I pushed a fix.

I also changed the test suite to run the selftests in verbose mode so that
it's easier to verify that all the tests are running.

[Leo Arias (elopio)]

This is nice. When merged with federico's branch, we can remove debian/tests/check_for_update and instead fail the upgrade test when there is no newer version available.

review: Approve

[Christopher Lee (veebers)]

Awesome, looks good to me.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== added file 'debian/tests/check_for_update'
--- debian/tests/check_for_update	1970-01-01 00:00:00 +0000
+++ debian/tests/check_for_update	2015-03-16 14:25:21 +0000
@@ -0,0 +1,45 @@
+#!/usr/bin/python3
+
+# Ubuntu OTA Tests
+# Copyright (C) 2015 Canonical
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Check for whether an update is available or not."""
+
+import sys
+
+from ubuntu_ota_tests.services import check_for_update, ensure_loop_ready
+
+
+def main():
+    ensure_loop_ready()
+    status = check_for_update()
+    # This bit may or may not survive.  For now, proof that the test works is
+    # by printing the results to stdout.
+    available = bool(int(status.is_available))
+    print('update available?', available)
+    print('downloading?', bool(int(status.downloading)))
+    print('available_version:', status.available_version or 'n/a')
+    print('update size:', status.update_size)
+    print('last update date:', status.last_update_date)
+    if status.error_reason:
+        print('An error occurred:', status.error_reason)
+    # We want to return to the shell exit code 0 (success) when there is an
+    # update available, and 1 (failure) when there isn't.
+    return int(not available)
+
+
+if __name__ == '__main__':
+    sys.exit(main())

=== modified file 'debian/tests/control'
--- debian/tests/control	2015-03-12 15:37:04 +0000
+++ debian/tests/control	2015-03-16 14:25:21 +0000
@@ -9,4 +9,11 @@
 Restrictions: needs-root allow-stderr
 Depends: python3-autopilot,
          python3-psutil,
-         system-image-cli
\ No newline at end of file
+         system-image-cli
+
+Tests: check_for_update
+Depends: system-image-common,
+         system-image-dbus,
+         ubuntu-download-manager,
+         dbus, dbus-x11,
+         python3-psutil, python3-xdg, python3-setuptools, python3-nose2

=== modified file 'debian/tests/ota_selftests'
--- debian/tests/ota_selftests	2015-03-11 17:56:51 +0000
+++ debian/tests/ota_selftests	2015-03-16 14:25:21 +0000
@@ -22,7 +22,7 @@
 
 cd debian/tests
 
-python3-coverage run --include=./ubuntu_ota_tests/* --omit=./ubuntu_ota_tests/selftests/*,./ubuntu_ota_tests/__init__.py -m unittest discover ubuntu_ota_tests.selftests
+python3-coverage run --include=./ubuntu_ota_tests/* --omit=./ubuntu_ota_tests/selftests/*,./ubuntu_ota_tests/__init__.py -m unittest discover -v ubuntu_ota_tests.selftests
 
 mkdir -p $ADT_ARTIFACTS/coverage && python3-coverage report -m > $ADT_ARTIFACTS/coverage/report.txt
 mv .coverage $ADT_ARTIFACTS/coverage

=== added file 'debian/tests/ubuntu_ota_tests/reactors.py'
--- debian/tests/ubuntu_ota_tests/reactors.py	1970-01-01 00:00:00 +0000
+++ debian/tests/ubuntu_ota_tests/reactors.py	2015-03-16 14:25:21 +0000
@@ -0,0 +1,54 @@
+# Ubuntu OTA Tests
+# Copyright (C) 2015 Canonical
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Convenient reactors used for asynchronous method calls."""
+
+__all__ = [
+    'CheckForUpdateReactor',
+    ]
+
+
+import dbus
+
+from collections import namedtuple
+from systemimage.reactor import Reactor
+
+
+# Use a namedtuple for more convenient argument unpacking.
+UASRecord = namedtuple('UASRecord',
+    'is_available downloading available_version update_size '
+    'last_update_date error_reason')
+
+
+class CheckForUpdateReactor(Reactor):
+    def __init__(self, iface):
+        super().__init__(dbus.SystemBus())
+        self.iface = iface
+        self.signals = []
+        self.react_to('UpdateAvailableStatus')
+
+    def _do_UpdateAvailableStatus(self, signal, path, *args):
+        self.signals.append(UASRecord(*args))
+        self.quit()
+
+    def run(self, timeout=None):
+        self.schedule(self.iface.CheckForUpdate)
+        super().run(timeout)
+        if self.timed_out:
+            raise TimeoutError
+        assert len(self.signals) == 1, (
+            'Unexpected signal count: {}'.format(len(self.signals)))
+        return self.signals[0]

=== modified file 'debian/tests/ubuntu_ota_tests/selftests/test_services.py'
--- debian/tests/ubuntu_ota_tests/selftests/test_services.py	2015-03-12 14:57:16 +0000
+++ debian/tests/ubuntu_ota_tests/selftests/test_services.py	2015-03-16 14:25:21 +0000
@@ -18,6 +18,7 @@
 
 import unittest
 
+from datetime import datetime
 from ubuntu_ota_tests import services
 
 
@@ -46,3 +47,8 @@
     def test_set_auto_download_on(self):
         services.set_auto_download(True)
         self.assertEqual(True, services.get_auto_download())
+
+    def test_check_for_update(self):
+        status = services.check_for_update()
+        utcnow = datetime.utcnow().replace(microsecond=0).isoformat(' ')
+        self.assertLessEqual(status.last_update_date, utcnow)

=== modified file 'debian/tests/ubuntu_ota_tests/services.py'
--- debian/tests/ubuntu_ota_tests/services.py	2015-03-12 14:57:16 +0000
+++ debian/tests/ubuntu_ota_tests/services.py	2015-03-16 14:25:21 +0000
@@ -24,6 +24,7 @@
 from testtools import matchers
 
 from ubuntu_ota_tests import system
+from ubuntu_ota_tests.reactors import CheckForUpdateReactor
 
 
 logger = logging.getLogger(__name__)
@@ -92,23 +93,44 @@
     info = sys_image_iface.Information()
     return int(info['current_build_number'])
 
+
 def set_auto_download(on):
     """Sets the value of auto_download in the system image client.
 
     :param on: Whether auto download should be on or off
     """
     # The system image client only accepts strings for SetSetting,
-    # with '1' representing On and '0' Off   
+    # with '1' representing On and '0' Off
     auto_download = '1' if on else '0'
     sys_image_iface = get_system_image_interface()
     sys_image_iface.SetSetting('auto_download', auto_download)
 
+
 def get_auto_download():
     """Gets the current value of auto_download in the system image client
-    
+
     :returns: True if on, False if off
     """
     sys_image_iface = get_system_image_interface()
     # We get back either '1' or '0' (strings) so have to convert first to
     # Integer then Boolean
     return bool(int(sys_image_iface.GetSetting('auto_download')))
+
+
+def check_for_update():
+    """Check for whether updates are available.
+
+    :return: namedtuple containing the following attributes:
+        - is_available: flag indicating whether an update is available
+        - downloading: flag indicating whether a download is in progress
+        - available_version: string specifying update target candidate
+          version, or 'n/a' if no update is available
+        - update_size: integer providing total size in bytes for the update
+        - last_update_date: ISO 8601 format UTC date string that the last
+          update was applied to this device
+        - error_reason: a string containing the reason the check or download
+          failed, or the empty string on success
+    """
+    iface = get_system_image_interface()
+    reactor = CheckForUpdateReactor(iface)
+    return reactor.run()