Merge lp:~canonical-platform-qa/webbrowser-app/autopkgtest into lp:webbrowser-app

Proposed by Leo Arias
Status: Rejected
Rejected by: Olivier Tilloy
Proposed branch: lp:~canonical-platform-qa/webbrowser-app/autopkgtest
Merge into: lp:webbrowser-app
Diff against target: 163 lines (+118/-3)
3 files modified
README (+57/-3)
debian/tests/control (+26/-0)
debian/tests/touch-session-autopilot (+35/-0)
To merge this branch: bzr merge lp:~canonical-platform-qa/webbrowser-app/autopkgtest
Reviewer Review Type Date Requested Status
Olivier Tilloy Disapprove
PS Jenkins bot continuous-integration Needs Fixing
Martin Pitt Needs Fixing
Review via email: mp+256858@code.launchpad.net

Commit message

Added the dep8 tests and instructions on the README to run them in qemu or on the phone.

Description of the change

This introduces dep8 tests that can be run against a local qemu instance (optionally configured against -proposed) as well as a local phone.

It runs the two autopilot suites one dep8 test by autopilot suite (see README for adt-run runes).

To post a comment you must log in.
Revision history for this message
Leo Arias (elopio) wrote :

Not yet ready for review because if we land this and the tests are run when the package is being upgraded in the archive, they will fail because the infrastructure is not ready yet to pass the --setup-command.

Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

Small comment about where the docs go

Revision history for this message
Olivier Tilloy (osomon) wrote :

Also note that this source package contains two autopilot test suites: webbrowser_app and webapp_container. I guess both should be run.

Revision history for this message
Leo Arias (elopio) wrote :

Thanks for the review Brendan. Reply inline. Please let me know what do you think.

Revision history for this message
Leo Arias (elopio) wrote :

> Also note that this source package contains two autopilot test suites:
> webbrowser_app and webapp_container. I guess both should be run.

I'm giving that a try...

Revision history for this message
Vincent Ladeuil (vila) wrote :

This now passes the tests against the current archive, -proposed and the phone (krillin devel-proposed 196).

@Leo: Feedback welcome, there may still be some rough edges and bits we'd like to be ready for reuse in other projects but I think that can wait.

Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

Small suggestion

Revision history for this message
Martin Pitt (pitti) wrote :

Nice work! This should get some cleanup (see inline comments), but by and large LGTM.

This is a good prototype for seeing what we need to fix in our infrastructure to eventually not make this an eyebrow-riser, so once we can massage these tests to be simple again we can declare victory :-)

Revision history for this message
Martin Pitt (pitti) wrote :

Also, I don't think we want to have all that README spread out/duplicated into all of our packages, but rather centrally in README.running-tests and/or the wiki?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
978. By Vincent Ladeuil

From pitti's review: remove wrappers, use Test-Command instead. Import ubuntu-touch-session env in a non-barbaric way.

979. By Vincent Ladeuil

Revert the needs-recommends tweak, it's too greedy and on the phone it trigger importing python3-xlib which in turn triggers a bug in autopilot automatic platform detection.

Revision history for this message
Vincent Ladeuil (vila) wrote :

Thanks a lot for the reviews pitti/brendand.

I've made a some modifications and pushed them.

Discussed on IRC:
- ADT_NORMAL_USER is not available for tests so keep using $USER
- ubuntu-touch-session doesn't have the x bit so keep using sh -e

I tried needs-recommends instead of explicitly specifying python3-evdev but it broke on the phone so I reverted that part. My gut feeling is that python3-xlib is installed on the phone and confuses autopilot.
We may want to fix autopilot though.

Revision history for this message
Vincent Ladeuil (vila) wrote :
Revision history for this message
Vincent Ladeuil (vila) wrote :

> Also, I don't think we want to have all that README spread out/duplicated into
> all of our packages, but rather centrally in README.running-tests and/or the
> wiki?

Sorry, I missed that comment earlier.

I'm -0 here, until everybody knows how to use adt-run, I'm not against some doc duplication.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Vincent Ladeuil (vila) wrote :

> > Also, I don't think we want to have all that README spread out/duplicated
> into
> > all of our packages, but rather centrally in README.running-tests and/or the
> > wiki?
>
> Sorry, I missed that comment earlier.
>
> I'm -0 here, until everybody knows how to use adt-run, I'm not against some
> doc duplication.

Meh, upon reflection I think we should indeed centralize the doc AND point to it from the README. What's the URL I should use ?

Revision history for this message
Federico Gimenez (fgimenez) wrote :

Looks really great! One inline comment about test dependencies.

Don't know if it's relevant at this stage but I'm getting this [1] following the README for qemu and the current archive, there's an error for the first test:

  bash: line 8: /tmp/tmp.bEKuLEBs9e/upstart/sessions/9325.session: No such file or directory

Thanks!

[1] http://paste.ubuntu.com/10877550/

Revision history for this message
Federico Gimenez (fgimenez) wrote :

Looks really great! One inline comment about test dependencies.

Don't know if it's relevant at this stage but I'm getting this [1] following the README, there's an error for the first test:

  bash: line 8: /tmp/tmp.bEKuLEBs9e/upstart/sessions/9325.session: No such file or directory

Thanks!

[1] http://paste.ubuntu.com/10877550/

Revision history for this message
Federico Gimenez (fgimenez) wrote :

Looks really great! One inline comment about test dependencies.

Don't know if it's relevant at this stage but I'm getting this [1] following the README for qemu and the current archive, there's an error for the first test:

  bash: line 8: /tmp/tmp.bEKuLEBs9e/upstart/sessions/9325.session: No such file or directory

Thanks!

[1] http://paste.ubuntu.com/10877550/

Revision history for this message
Federico Gimenez (fgimenez) wrote :

Looks really great! One inline comment about test dependencies.

Don't know if it's relevant at this stage but I'm getting this [1] following the README for qemu and the current archive, there's an error for the first test:

  bash: line 8: /tmp/tmp.bEKuLEBs9e/upstart/sessions/9325.session: No such file or directory

Thanks!

[1] http://paste.ubuntu.com/10877550/

Revision history for this message
Federico Gimenez (fgimenez) wrote :

Looks really great! One inline comment about test dependencies.

Don't know if it's relevant at this stage but I'm getting this [1] following the README for qemu and the current archive, there's an error in the first test:

  bash: line 8: /tmp/tmp.bEKuLEBs9e/upstart/sessions/9325.session: No such file or directory

Thanks!

[1] http://paste.ubuntu.com/10877550/

980. By Vincent Ladeuil

Clarify why autopkgtest is a test dependency.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Martin Pitt (pitti) wrote :

Thanks Vincent for the updates! One important bug (sudo -A) left, the others are nitpicks.

review: Needs Fixing
981. By Vincent Ladeuil

Mention the prefered targets for the documentation about running dep8 tests.

982. By Vincent Ladeuil

XS-Testsuite isn't necessary at all any more, dpkg-source adds this automatically now. (from pitti's review).

983. By Vincent Ladeuil

Remove the FIXME, explicitly Depend'ing on python3-evdev is fine. Added 'Classes: ubuntu-touch' to document the target test class (currently a no-op).

984. By Vincent Ladeuil

Use -A for sudo or the test can hang if sudo asks for a password.

Revision history for this message
Vincent Ladeuil (vila) wrote :

Thanks for the review !

All fixed as mentioned in the inline comments.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

Any update on:

+.. FIXME: This shouldn't be part of a specific package but instead be part
+ of the wiki
+ https://wiki.ubuntu.com/Touch/Testing#Running_tests_with_autopkgtest or
+ of autopkgtest README.running-tests -- vila 2015-05-05

?

Revision history for this message
Olivier Tilloy (osomon) wrote :

As far as the changes themselves, they look ok to me (I’m not really familiar with autopkgtests so I’ll trust Martin’s thorough review). I’ve tested the instructions and got errors while running the actual tests, but the runners seemed to be doing their job.

review: Approve
Revision history for this message
Leo Arias (elopio) wrote :

Thanks for looking at this Olivier.
But it's really important that all the tests are passing, otherwise you won't be able to land an update to the archive.

We need to either fix them or select a subset of tests to begin with. Can you please paste the errors you are getting?

985. By Vincent Ladeuil

Remove the FIXME about doc by refering to the official autopkgtest documentation and dropping the proposed-migration example as it's not defined by the project.

Revision history for this message
Vincent Ladeuil (vila) wrote :

> Any update on:
>
> +.. FIXME: This shouldn't be part of a specific package but instead be part

Sorry about the delay, I somehow never seen that comment :-/

Fixed now.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Vincent Ladeuil (vila) wrote :

> Thanks for looking at this Olivier.
> But it's really important that all the tests are passing, otherwise you won't
> be able to land an update to the archive.

A bit of clarification here, proposed-migration will not block for tests that always failed in the past.

Since this is the first dep8 test, the landing won't be blocked.

Once the test is passing though, yes, new failures will block the landing.

>
> We need to either fix them or select a subset of tests to begin with. Can you
> please paste the errors you are getting?

From http://s-jenkins.ubuntu-ci:8080/job/generic-deb-autopilot-runner-vivid-mako/2461/console

The following packages have unmet dependencies:
 webbrowser-app : Depends: liboxideqt-qmlplugin (>= 1.7) but 1.5.5-0ubuntu1 is to be installed
E: Unable to correct problems, you have held broken packages.
ADB_RC=100

So,

1) it's failing on mako (which won't happen in proposed-migration (we're still far from running tests on phones in the proposed-migration)

2) I can use some help understanding the failure there as I don't quite get where liboxideqt-qmlplugin is coming from (or should come from).

Revision history for this message
Olivier Tilloy (osomon) wrote :

> I can use some help understanding the failure there as I don't quite
> get where liboxideqt-qmlplugin is coming from (or should come from).

The release pocket of vivid has oxide 1.5.5-0ubuntu1.
The updates and security pockets have oxide 1.6.6-0ubuntu0.15.04.1.
The vivid overlay PPA has oxide 1.7.7-0ubuntu0.15.04.1~ppa1.

Revision history for this message
Vincent Ladeuil (vila) wrote :

> > I can use some help understanding the failure there as I don't quite
> > get where liboxideqt-qmlplugin is coming from (or should come from).
>
> The release pocket of vivid has oxide 1.5.5-0ubuntu1.
> The updates and security pockets have oxide 1.6.6-0ubuntu0.15.04.1.
> The vivid overlay PPA has oxide 1.7.7-0ubuntu0.15.04.1~ppa1.

It's not in https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/stable-phone-overlay/+index?field.series_filter=vivid

am I looking at the wrong place ?

Revision history for this message
Vincent Ladeuil (vila) wrote :

> It's not in https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu
> /stable-phone-overlay/+index?field.series_filter=vivid
>
> am I looking at the wrong place ?

Yes and no ;) It's part of oxide-qt so available there, the -ci job mentions adding the overlay ppa too.

But apt-get doesn't find it...

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Vincent Ladeuil (vila) wrote :

Ok, so, the issue is that -ci use ubuntu/devel-proposed for mako which is wily and the overlay ppa does not have packages for wily.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Vincent Ladeuil (vila) wrote :

.... and:

$ rmadison liboxideqt-qmlplugin

...

 liboxideqt-qmlplugin | 1.7.7-0ubuntu0.15.04.1~ppa1 | wily-proposed | amd64, armhf, i386

@Olivier: Do you know why 1.7 is only in wily-proposed ? What's the way out of this situation ?

Revision history for this message
Olivier Tilloy (osomon) wrote :

@vila: migration of oxide 1.7.7 from proposed to release was blocked on some autopkgtest failure, it has now been resolved.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :
review: Disapprove

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'README'
--- README 2015-04-01 05:57:42 +0000
+++ README 2015-05-18 21:41:52 +0000
@@ -40,11 +40,11 @@
4040
41 $ ctest41 $ ctest
4242
43
44= Automated UI tests =43= Automated UI tests =
4544
46webbrowser-app uses autopilot (https://launchpad.net/autopilot) to test its UI.45webbrowser-app uses autopilot (https://launchpad.net/autopilot) to test its UI.
47To run the tests, you will need to install python3-autopilot and autopilot-qt5.46To run the tests locally, you will need to install python3-autopilot and
47autopilot-qt5.
48Then do the following:48Then do the following:
4949
50 $ cd tests/autopilot/50 $ cd tests/autopilot/
@@ -54,6 +54,8 @@
5454
55 $ autopilot3 list webbrowser_app55 $ autopilot3 list webbrowser_app
5656
57In order to run the tests in a virtual machine with an environment closer to
58what a user will get in Ubuntu Touch, see the Dep8 tests section.
5759
58= Code coverage =60= Code coverage =
5961
@@ -68,6 +70,59 @@
68This will generate a coverage report in XML format (coverage.xml) and an70This will generate a coverage report in XML format (coverage.xml) and an
69interactive human-readable report in HTML format (coveragereport/index.html).71interactive human-readable report in HTML format (coveragereport/index.html).
7072
73= Dep8 tests =
74
75Dep8 tests excercise the package "as-installed".
76
77Currently, the webbrowser-app has one suite of dep8 tests that uses autopilot
78(https://launchpad.net/autopilot) to test from the point of view of the user.
79
80To run the tests you will need autopkgtest:
81
82 $ sudo apt-get install autopkgtest
83
84You can use multiple test beds to execute the tests. Below you will find
85instructions to run them in a virtual machine
86You can find more information with:
87
88 $ man adt-run
89
90== Run dep8 tests ==
91
92To run the tests in a qemu virtual machine, you will first have to create it
93(see /usr/share/doc/autopkgtest/README.running-tests.rst.gz). We output the
94image to ~/ rather than the current directory, so it will be in a safer
95place to avoid rebuilding images every time. You can store it in any
96directory you wish. This image is better consumed "fresh", building it daily
97will avoid long updates/upgrades when running the tests.
98
99 $ adt-buildvm-ubuntu-cloud -r vivid -a amd64 -o ~/
100
101Then run the tests using adt-run with the qemu virtualization host against
102the current archive.
103
104 $ adt-run -B -U --unbuilt-tree . \
105 -o ~/adt-browser-test/$(date +%Y-%m-%d-%H-%M) \
106 --- qemu ~/adt-vivid-amd64-cloud.img
107
108The tests can also be run on a local phone.
109
110 $ adt-run -B -U --unbuilt-tree . \
111 -o ~/adt-browser-test/$(date +%Y-%m-%d-%H-%M) \
112 --- ssh -s adb -- -p <password> --serial <ADB_SERIAL>
113
114== Examine the dep8 autopilot results ==
115
116To examine the test results, which are in subunit format, additional tools are
117required.
118
119 $ sudo add-apt-repository ppa:thomir/trv
120 $ sudo apt-get update
121 $ sudo apt-get install trv
122 $ trv
123
124You can find the results file in the directory
125~/adt-browser-test/$(date +%Y-%m-%d-%H-%M)/artifacts.
71126
72= Settings =127= Settings =
73128
@@ -93,4 +148,3 @@
93148
94 - restoreSession: whether to restore the previous browsing session at startup149 - restoreSession: whether to restore the previous browsing session at startup
95 (defaults to true)150 (defaults to true)
96
97151
=== added directory 'debian/tests'
=== added file 'debian/tests/control'
--- debian/tests/control 1970-01-01 00:00:00 +0000
+++ debian/tests/control 2015-05-18 21:41:52 +0000
@@ -0,0 +1,26 @@
1# Copyright 2015 Canonical
2#
3# This program is free software: you can redistribute it and/or modify it
4# under the terms of the GNU General Public License version 3, as published
5# by the Free Software Foundation.
6#
7# This program is distributed in the hope that it will be useful,
8# but WITHOUT ANY WARRANTY; without even the implied warranty of
9# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
10# GNU General Public License for more details.
11#
12# You should have received a copy of the GNU General Public License
13# along with this program. If not, see <http://www.gnu.org/licenses/>.
14
15# autopkgtest is a test dependency so we don't need to copy
16# ubuntu-touch-session to the testbed but can use it from autopkgtest instead
17
18Test-Command: debian/tests/touch-session-autopilot webbrowser_app
19Restrictions: allow-stderr
20Classes: ubuntu-touch
21Depends: webbrowser-app-autopilot, autopkgtest, python3-evdev
22
23Test-Command: debian/tests/touch-session-autopilot webapp_container
24Restrictions: allow-stderr
25Classes: ubuntu-touch
26Depends: webapp-container-autopilot, autopkgtest, python3-evdev
027
=== added file 'debian/tests/touch-session-autopilot'
--- debian/tests/touch-session-autopilot 1970-01-01 00:00:00 +0000
+++ debian/tests/touch-session-autopilot 2015-05-18 21:41:52 +0000
@@ -0,0 +1,35 @@
1#!/bin/sh
2
3# Copyright 2015 Canonical
4#
5# This program is free software: you can redistribute it and/or modify it
6# under the terms of the GNU General Public License version 3, as published
7# by the Free Software Foundation.
8#
9# This program is distributed in the hope that it will be useful,
10# but WITHOUT ANY WARRANTY; without even the implied warranty of
11# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12# GNU General Public License for more details.
13#
14# You should have received a copy of the GNU General Public License
15# along with this program. If not, see <http://www.gnu.org/licenses/>.
16
17# This runs the $1 autopilot suite ensuring that the testbed is configured
18# properly
19
20SUITE=$1
21
22set -e
23
24if ! pgrep -f unity-system-compositor ; then
25 # We're not using Mir, setup enough of an X stack
26
27 # FIXME: We won't need an X stack once Mir provides mirvfb or any other
28 # mean to test consistently on all platforms -- vila 2015-04-22
29 sudo -A ADT_NORMAL_USER=${USER} sh -e /usr/share/autopkgtest/setup-commands/ubuntu-touch-session
30 # Import the environment produced above in the current shell
31 set -a
32 . /etc/environment
33fi
34
35autopilot3 run -v -f subunit -o ${ADT_ARTIFACTS} ${SUITE}

Subscribers

People subscribed via source and target branches

to status/vote changes: