Merge lp:~canonical-platform-qa/ubuntu-ota-tests/current-version-number into lp:ubuntu-ota-tests

[Federico Gimenez]

Getting the current version number through the D-Bus API

For executing the tests:

  $ adt-run -B --unbuilt-tree=. --- ssh -s adb

[Brendan Donegan (brendan-donegan)]

Why does the current_version script need to call adb? Tests shouldn't have to call adb that should be a matter for the runner - I would remove that if it doesn't have any purpose

review: Needs Information

[Federico Gimenez (fgimenez)]

You are right, it was a proof of concept. I've modified it to run from a test with a bash function that could be reused.

[Barry Warsaw (barry)]

Thoughts, suggestions, questions.

[Brendan Donegan (brendan-donegan)]

review: Needs Fixing

[Federico Gimenez (fgimenez)]

Some inline comments, thanks!

[Federico Gimenez (fgimenez)]

I've taken the opportunity to add some self tests for the current code. The get_systemimage_iface test reflects that the interaction with the dbus api is a bit complicated, Demeter must be turning in her grave (if she has one) :)

[Brendan Donegan (brendan-donegan)]

Thanks for the tests, but I'm confused by some of the refactoring you did.

review: Needs Information

[Federico Gimenez (fgimenez)]

@brendand finally I've reverted the initial approach, if we need to add more methods to interact with the api the iface would be instantiated one time for each one of them with the default value of the function as I had defined it.

[Brendan Donegan (brendan-donegan)]

Okay looks good to me. Try and get one more review

review: Approve

[Brendan Donegan (brendan-donegan)]

Some questions about debian/tests/control actually

review: Needs Information

[Federico Gimenez (fgimenez)]

Yes, ota_basic was a shell-script based test used in the first spikes, see for example this recent branch from Chris [1] where it is already removed.

Regarding the Tests-Directory configuration option I've leave it there as a remainder to myself (we can remove it of course), it is what Martin suggested in order to execute the autopkgtest scripts from the root of the project and being able to import the python modules placed there as well. I think that we still need to decide how are we going to handle this.

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

[Brendan Donegan (brendan-donegan)]

> Yes, ota_basic was a shell-script based test used in the first spikes, see for
> example this recent branch from Chris [1] where it is already removed.
>
> Regarding the Tests-Directory configuration option I've leave it there as a
> remainder to myself (we can remove it of course), it is what Martin suggested

If it's to serve as a reminder than put an actual reminder, something like '# We might be able to use Tests-Directory here instead of hacking the path in the scripts'

> in order to execute the autopkgtest scripts from the root of the project and
> being able to import the python modules placed there as well. I think that we
> still need to decide how are we going to handle this.
>
> [1] https://code.launchpad.net/~canonical-platform-qa/ubuntu-ota-tests/check-
> for-running-service/+merge/252057

[Federico Gimenez (fgimenez)]

Well, it was just a remainder for me :) I've removed it, let's see if we finally use it or not

[Brendan Donegan (brendan-donegan)]

Okay, approve

review: Approve

[Leo Arias (elopio)]

This branch has some small flake8 errors. Please fix them before merging. Other than that, looks good to me.

review: Needs Fixing

[Federico Gimenez (fgimenez)]

Ok, fixed. We could add a minimal setup.py file for executing flake8 (and perhaps coverage), don't know if it's worth the effort.

Thanks!

[Richard Huddie (rhuddie)]

Checked there were no flake8 errors and ran with adt-run, which gave the correct build number. Looks good, so approving.

review: Approve

[Leo Arias (elopio)]

135 +++ ubuntu_ota_tests/tests/test_client.py

This file is missing the copyright header. After that, +1 from me.

[Federico Gimenez (fgimenez)]

Ok, added the copyright; there's also a notice in the readme about how to run the unit tests

Thanks!

[Federico Gimenez (fgimenez)]

Merged [1] into this one, I've put this one back to in progress until that branch lands.

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

[lp:~canonical-platform-qa/ubuntu-ota-tests/current-version-number]

24. By Federico Gimenez on 2015-03-11

flake8 fixes; reverted ota_basic

[Brendan Donegan (brendan-donegan)]

Already approved by me, I don't see anything else to be done

[Leo Arias (elopio)]

looks good. I'd just like the coverage command in a script.

review: Needs Fixing

[lp:~canonical-platform-qa/ubuntu-ota-tests/current-version-number]

25. By Federico Gimenez on 2015-03-11

returning an actual int

26. By Federico Gimenez on 2015-03-11

test instead of test command; moved coverage reports to ADT_ARTIFACTS; setup + addcleanup instead of teardown

[Federico Gimenez (fgimenez)]

Ok, I think that all the issues have been addressed... except the ota_basic one, it's a mistery for me why it is reported like that

Thanks!

[Leo Arias (elopio)]

Great, thanks Federico.
I think we should set -e to the ota_selftests script to be on the safe side.

I'll do it as I think you already EOD.

[lp:~canonical-platform-qa/ubuntu-ota-tests/current-version-number]

27. By Leo Arias on 2015-03-11

Added set -e to the selftests script.

[Leo Arias (elopio)]

85% coverage, not bad. Top-approving.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'debian/tests/control'
--- debian/tests/control	2015-03-10 06:37:58 +0000
+++ debian/tests/control	2015-03-11 17:57:10 +0000
@@ -1,4 +1,6 @@
-Test-Command: cd debian/tests; python3 -m unittest discover ubuntu_ota_tests.selftests
+Tests: ota_selftests
 Restrictions: allow-stderr
 Depends: python3-autopilot,
+         python3-coverage,
          python3-psutil,
+

=== added file 'debian/tests/ota_basic'
--- debian/tests/ota_basic	1970-01-01 00:00:00 +0000
+++ debian/tests/ota_basic	2015-03-11 17:57:10 +0000
@@ -0,0 +1,29 @@
+#!/bin/bash
+
+#
+# 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/>.
+#
+
+# Steps:
+#  - Flash device with revision x
+#  - Check device flashed correctly
+#  - Trigger upgrade to revision x+1
+#  - (Wait for upgrade)
+#  - Reboot device
+#  - Check the upgrade worked as expected.
+
+exit 0

=== removed file 'debian/tests/ota_basic'
--- debian/tests/ota_basic	2015-03-03 02:58:26 +0000
+++ debian/tests/ota_basic	1970-01-01 00:00:00 +0000
@@ -1,29 +0,0 @@
-#!/bin/bash
-
-#
-# 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/>.
-#
-
-# Steps:
-#  - Flash device with revision x
-#  - Check device flashed correctly
-#  - Trigger upgrade to revision x+1
-#  - (Wait for upgrade)
-#  - Reboot device
-#  - Check the upgrade worked as expected.
-
-exit 0

=== added file 'debian/tests/ota_selftests'
--- debian/tests/ota_selftests	1970-01-01 00:00:00 +0000
+++ debian/tests/ota_selftests	2015-03-11 17:57:10 +0000
@@ -0,0 +1,30 @@
+#!/bin/bash
+
+#
+# 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/>.
+#
+
+set -e
+
+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
+
+mkdir -p $ADT_ARTIFACTS/coverage && python3-coverage report -m > $ADT_ARTIFACTS/coverage/report.txt
+mv .coverage $ADT_ARTIFACTS/coverage
+
+exit 0

=== modified file 'debian/tests/ubuntu_ota_tests/selftests/test_services.py'
--- debian/tests/ubuntu_ota_tests/selftests/test_services.py	2015-03-10 05:55:55 +0000
+++ debian/tests/ubuntu_ota_tests/selftests/test_services.py	2015-03-11 17:57:10 +0000
@@ -23,6 +23,9 @@
 
 class ServicesTestCase(unittest.TestCase):
 
+    def setUp(self):
+        self.addCleanup(services.ensure_system_image_dbus_not_running)
+
     def test_ensure_system_image_dbus_not_runnig_must_stop_process(self):
         # start the dbus process if it isn't already
         services.get_system_image_interface()
@@ -30,3 +33,8 @@
 
         services.ensure_system_image_dbus_not_running()
         self.assertFalse(services._is_system_image_process_running())
+
+    def test_get_current_version_number(self):
+        current_build_number = services.get_current_build_number()
+        self.assertGreater(current_build_number, 0,
+                           'The current build number should be greater than 0')

=== modified file 'debian/tests/ubuntu_ota_tests/services.py'
--- debian/tests/ubuntu_ota_tests/services.py	2015-03-10 03:31:34 +0000
+++ debian/tests/ubuntu_ota_tests/services.py	2015-03-11 17:57:10 +0000
@@ -84,3 +84,10 @@
         return system.get_process_by_name(SYS_IMG_PROC_NAME) is not None
     except OSError:
         return False
+
+
+def get_current_build_number():
+    """Returns the build number of the current image"""
+    sys_image_iface = get_system_image_interface()
+    info = sys_image_iface.Information()
+    return int(info['current_build_number'])